Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry publish crossgen as AOT (#65948) #67636

Merged
merged 43 commits into from
Apr 26, 2022
Merged

Retry publish crossgen as AOT (#65948) #67636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
64c286c
Publish crossgen as AOT if supported by the target platform (#65948)
agocke Apr 1, 2022
66c295d
Set TargetFramework for the crossgen build to be NetCoreAppToolCurrent
agocke Apr 6, 2022
0de9ffa
Remove crossgen2packagerid property
agocke Apr 6, 2022
5a229cf
Override in csproj
agocke Apr 6, 2022
8a52522
Try deleting first
agocke Apr 7, 2022
9dd8701
Log the assets.json file
agocke Apr 7, 2022
21ebeb9
Pass down DotNetBuildFromSource
agocke Apr 7, 2022
aa7c940
Up the message importance
agocke Apr 7, 2022
e33ee7a
Try to skip restore in source build
agocke Apr 8, 2022
ecf61b1
Typo
agocke Apr 8, 2022
802acb6
Add PropertyGroup
agocke Apr 8, 2022
bf5fb36
Try depending on restore
agocke Apr 8, 2022
8d658ee
Use PackageRID instead of RuntimeIdentifier
agocke Apr 9, 2022
237fe3c
Remove restore dependency
agocke Apr 9, 2022
fbc260a
Add log of RID
agocke Apr 9, 2022
a55404e
Set RuntimeIdentifier in the project file
agocke Apr 10, 2022
8a8a8a5
Add print after restore
agocke Apr 10, 2022
7466f7f
only log while running publish
agocke Apr 10, 2022
fd8e054
Typo
agocke Apr 10, 2022
8e6d6b2
Escape characters
agocke Apr 11, 2022
ebee79f
Use cat instead of reading lines
agocke Apr 12, 2022
cf41668
Try disabling assets cache
agocke Apr 15, 2022
1471d40
Adjust RIDs
agocke Apr 15, 2022
3f39bf6
Set RID to package RID for source build
agocke Apr 15, 2022
558ea09
Typo
agocke Apr 15, 2022
0cbba11
Typo
agocke Apr 15, 2022
5e9ece4
Adjust crossgen path on source build
agocke Apr 15, 2022
e26dc43
Fix crossgen2tool location
agocke Apr 15, 2022
92d1909
Removing logging
agocke Apr 16, 2022
aaffc19
Remove target reference
agocke Apr 16, 2022
4dacc8f
Set PackageRID to include portable OS when building portable bits
agocke Apr 16, 2022
edc4682
Revert change to PackageRID and instead add musl check for _packageOS
agocke Apr 18, 2022
ddf892e
Update Directory.Build.props
agocke Apr 18, 2022
995c63f
Change to use RuntimeIdentifiers instead of RuntimeIdentifier, and av…
agocke Apr 19, 2022
6caf24a
Remove libs.pretest from sourcebuild set
agocke Apr 20, 2022
c2ae019
Move RewriteRuntimePackDir to crossgen2 project
agocke Apr 20, 2022
cf0e37a
Use live host in source build
agocke Apr 21, 2022
4d09845
Remove package assets cache disabling
agocke Apr 21, 2022
ab11405
Update src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.…
agocke Apr 22, 2022
850d708
PR comments
agocke Apr 22, 2022
c824ec9
Typo
agocke Apr 25, 2022
0c492e7
PR feedback
agocke Apr 25, 2022
3d16f41
Update src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.…
agocke Apr 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@
<_runtimeOS Condition="$(_runtimeOS.StartsWith('tizen'))">linux</_runtimeOS>
<_runtimeOS Condition="'$(PortableBuild)' == 'true'">$(_portableOS)</_runtimeOS>

<_packageOS Condition="'$(CrossBuild)' == 'true'">$(_hostOS.ToLowerInvariant)</_packageOS>
<_packageLibc Condition="$(_runtimeOS.Contains('musl'))">-musl</_packageLibc>
<_packageOS Condition="'$(CrossBuild)' == 'true'">$(_hostOS.ToLowerInvariant)$(_packageLibc)</_packageOS>
Comment on lines +156 to +157
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@am11 just to double check, does that change look right to you?

Copy link
Member

@am11 am11 Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agocke, could you tell what is this fixing?

IMO, we should run official build against the PR as we did previously so we can get some confidence from the package validation step.

ps - what I learned previously about this part of the configuration (<PropertyGroup Label="CalculateOS">) is that it is very tricky to be sure about. And when everything was green (incl. outerloop and official builds), it broke Tizen, which was reported few days later. 😁

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, the problem is that PackageRID was linux-x64 on alpine, when it should be linux-musl-x64.

<_packageOS Condition="'$(_packageOS)' == '' and '$(PortableBuild)' == 'true'">$(_portableOS)</_packageOS>
<_packageOS Condition="'$(_packageOS)' == ''">$(_runtimeOS)</_packageOS>
</PropertyGroup>
Expand Down Expand Up @@ -183,10 +184,6 @@

<PackageRID>$(_packageOS)-$(TargetArchitecture)</PackageRID>

<!-- Crossgen2 does not support armel, so we will use arm instead. -->
<Crossgen2PackageRID Condition="'$(TargetArchitecture)' == 'armel'">$(_packageOS)-arm</Crossgen2PackageRID>
<Crossgen2PackageRID Condition="'$(TargetArchitecture)' != 'armel'">$(PackageRID)</Crossgen2PackageRID>

<OutputRid Condition="'$(OutputRid)' == ''">$(PackageRID)</OutputRid>
<OutputRid Condition="'$(PortableBuild)' == 'true'">$(_portableOS)-$(TargetArchitecture)</OutputRid>
</PropertyGroup>
Expand Down
3 changes: 1 addition & 2 deletions eng/Subsets.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@
<DefaultLibrariesSubsets Condition="'$(BuildTargetFramework)' == '$(NetCoreAppCurrent)' or
'$(BuildTargetFramework)' == '' or
'$(BuildAllConfigurations)' == 'true'">libs.native+</DefaultLibrariesSubsets>
<DefaultLibrariesSubsets>$(DefaultLibrariesSubsets)libs.sfx+libs.oob</DefaultLibrariesSubsets>
<DefaultLibrariesSubsets Condition="'$(DotNetBuildFromSource)' != 'true'">$(DefaultLibrariesSubsets)+libs.pretest</DefaultLibrariesSubsets>
<DefaultLibrariesSubsets>$(DefaultLibrariesSubsets)libs.sfx+libs.oob+libs.pretest</DefaultLibrariesSubsets>
ViktorHofer marked this conversation as resolved.
Show resolved Hide resolved

<DefaultHostSubsets>host.native+host.tools</DefaultHostSubsets>
<DefaultHostSubsets Condition="'$(DotNetBuildFromSource)' != 'true'">$(DefaultHostSubsets)+host.pkg+host.tests</DefaultHostSubsets>
Expand Down
1 change: 1 addition & 0 deletions eng/liveBuilds.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
<CoreCLRCrossTargetComponentDirName Condition="'$(TargetArchitecture)' == 'arm' and '$(BuildArchitecture)' != 'arm' and '$(TargetsWindows)' == 'true'">x86</CoreCLRCrossTargetComponentDirName>
<CoreCLRCrossTargetComponentDirName Condition="'$(TargetArchitecture)' == 'arm' and '$(BuildArchitecture)' != 'arm' and '$(TargetsLinux)' == 'true'">x64</CoreCLRCrossTargetComponentDirName>
<CoreCLRCrossTargetComponentDirName Condition="'$(TargetArchitecture)' == 'armel' and '$(BuildArchitecture)' != 'armel' and '$(TargetsLinux)' == 'true'">x64</CoreCLRCrossTargetComponentDirName>
<SingleFileHostSourcePath>$(CoreCLRArtifactsPath)/corehost/singlefilehost$(ExeSuffix)</SingleFileHostSourcePath>
</PropertyGroup>

<Target Name="ResolveRuntimeFilesFromLocalBuild">
Expand Down
2 changes: 0 additions & 2 deletions eng/testing/tests.singlefile.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
<PublishSingleFile>true</PublishSingleFile>
<UseAppHost>true</UseAppHost>
<SelfContained>true</SelfContained>
<SingleFileHostSourcePath>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'coreclr', '$(TargetOS).$(TargetArchitecture).$(Configuration)', 'corehost'))/singlefilehost</SingleFileHostSourcePath>
<SingleFileHostSourcePath Condition="'$(TargetOS)' == 'windows'">$(SingleFileHostSourcePath).exe</SingleFileHostSourcePath>
</PropertyGroup>

<PropertyGroup Condition="'$(TestNativeAot)' == 'true'">
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/TypeSystem/Ecma/SymbolReader/UnmanagedPdbSymbolReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Internal.TypeSystem.Ecma
/// </summary>
public abstract class UnmanagedPdbSymbolReader : PdbSymbolReader
{
public static PdbSymbolReader TryOpenSymbolReaderForMetadataFile(string metadataFileName, string searchPath)
public static PdbSymbolReader? TryOpenSymbolReaderForMetadataFile(string metadataFileName, string searchPath)
{
return null;
}
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/tools/aot/ILCompiler/ILCompiler.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,18 @@
<TargetSpec>$(TargetOSComponent)_$(TargetArchitectureForLocalJitBuild)_$(TargetArchitectureForSharedLibraries)</TargetSpec>

<JitInterfaceLibraryName>$(LibPrefix)jitinterface_$(TargetArchitectureForSharedLibraries)$(LibSuffix)</JitInterfaceLibraryName>
<!-- This will be provided when using the liveBuild, and unset otherwise. -->
<CoreCLRArtifactsPath Condition="'$(CoreCLRArtifactsPath)' == ''">$(RuntimeBinDir)$(CrossHostArch)</CoreCLRArtifactsPath>
</PropertyGroup>

<ItemGroup>
<Content Include="$(RuntimeBinDir)$(CrossHostArch)/$(JitInterfaceLibraryName)"
<Content Include="$(CoreCLRArtifactsPath)/$(JitInterfaceLibraryName)"
CopyToOutputDirectory="PreserveNewest"
CopyToPublishDirectory="PreserveNewest"
Link="%(FileName)%(Extension)"
/>

<Content Include="$(RuntimeBinDir)$(CrossHostArch)/$(LibPrefix)clrjit_*_$(TargetArchitectureForSharedLibraries)$(LibSuffix)"
<Content Include="$(CoreCLRArtifactsPath)/$(LibPrefix)clrjit_*_$(TargetArchitectureForSharedLibraries)$(LibSuffix)"
CopyToOutputDirectory="PreserveNewest"
CopyToPublishDirectory="PreserveNewest"
Link="%(FileName)%(Extension)"
Expand Down
63 changes: 58 additions & 5 deletions src/coreclr/tools/aot/crossgen2/crossgen2.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,62 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project>

<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />

<PropertyGroup>
<OutputPath>$(RuntimeBinDir)/crossgen2</OutputPath>
<!-- The default value for macOS is false -->
<UseAppHost>true</UseAppHost>
<AppHostRuntimeIdentifier>$(Crossgen2PackageRID)</AppHostRuntimeIdentifier>
agocke marked this conversation as resolved.
Show resolved Hide resolved
<OutputPath>$(RuntimeBinDir)crossgen2</OutputPath>
<!-- Can't use NativeAOT in source build yet https://github.com/dotnet/runtime/issues/66859 and we
don't want to ship using NativeAOT on MacOS -->
<NativeAotSupported Condition="'$(DotNetBuildFromSource)' == 'true' or '$(TargetOS)' == 'osx'">false</NativeAotSupported>
<!-- Trimming is not currently working, but set the appropriate feature flags for NativeAOT -->
<PublishTrimmed Condition="'$(NativeAotSupported)' == 'true'">true</PublishTrimmed>
<RuntimeIdentifiers Condition="'$(RunningPublish)' != 'true' and '$(DotNetBuildFromSource)' != 'true'">linux-x64;linux-musl-x64;linux-arm;linux-musl-arm;linux-arm64;linux-musl-arm64;freebsd-x64;osx-x64;osx-arm64;win-x64;win-x86;win-arm64;win-arm</RuntimeIdentifiers>
<RuntimeIdentifiers Condition="'$(DotNetBuildFromSource)' == 'true'">$(PackageRID)</RuntimeIdentifiers>
<SelfContained>false</SelfContained>
<SelfContained Condition="'$(RunningPublish)' == 'true'">true</SelfContained>
</PropertyGroup>

<Import Project="crossgen2.props" />

<PropertyGroup Condition="'$(NativeAotSupported)' != 'true'">
<PublishSingleFile>true</PublishSingleFile>
<PublishReadyToRun>true</PublishReadyToRun>
<!-- Disable crossgen on NetBSD, illumos and Solaris for now. This can be revisited when we have full support. -->
<PublishReadyToRun Condition="'$(TargetOS)'=='NetBSD' Or '$(TargetOS)'=='illumos' Or '$(TargetOS)'=='Solaris'">false</PublishReadyToRun>
<!-- Disable crossgen on FreeBSD when cross building from Linux. -->
<PublishReadyToRun Condition="'$(TargetOS)'=='FreeBSD' and '$(CrossBuild)'=='true'">false</PublishReadyToRun>
<PublishReadyToRunComposite>true</PublishReadyToRunComposite>
</PropertyGroup>

<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />

<PropertyGroup Condition="'$(NativeAotSupported)' == 'true'">
<IlcToolsPath>$(CoreCLRILCompilerDir)</IlcToolsPath>
<IlcToolsPath Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)'">$(CoreCLRCrossILCompilerDir)</IlcToolsPath>
<CppCompilerAndLinker Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)' and '$(HostOS)' != 'windows'">clang-9</CppCompilerAndLinker>
<SysRoot Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)' and '$(HostOS)' != 'windows'">$(ROOTFS_DIR)</SysRoot>
<IlcBuildTasksPath>$(CoreCLRILCompilerDir)netstandard/ILCompiler.Build.Tasks.dll</IlcBuildTasksPath>
<IlcSdkPath>$(CoreCLRAotSdkDir)</IlcSdkPath>
<IlcFrameworkPath>$(MicrosoftNetCoreAppRuntimePackRidLibTfmDir)</IlcFrameworkPath>
<IlcFrameworkNativePath>$(MicrosoftNetCoreAppRuntimePackNativeDir)</IlcFrameworkNativePath>
<TrimmerSingleWarn>false</TrimmerSingleWarn>
</PropertyGroup>

<Import Project="$(R2ROverridePath)" Condition="'$(R2ROverridePath)' != ''" />
<Import Project="$(CoreCLRBuildIntegrationDir)Microsoft.DotNet.ILCompiler.targets"
Condition="'$(NativeAotSupported)' == 'true' and '$(RunningPublish)' == 'true'" />

<Target Name="RewriteRuntimePackDir"
Copy link
Member

@ViktorHofer ViktorHofer Apr 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider to add a TODO to track consolidating this logic with targetingpack.targets and/or create a tracking issue for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll file a tracking issue.

Condition="'$(RunningPublish)' == 'true'"
DependsOnTargets="ResolveRuntimeFilesFromLocalBuild"
BeforeTargets="ResolveRuntimePackAssets">
<ItemGroup>
<!-- Remove AspNetCore runtime pack since we don't build it locally -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is removing the AspNetCore runtime pack actually necessary? I never hit issue with it being available as long as no projects use it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @agocke for that question

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agocke did you see that question? :)

<ResolvedRuntimePack Remove="Microsoft.AspNetCore.App.Runtime.$(RuntimeIdentifier)" />

<ResolvedRuntimePack Update="Microsoft.NETCore.App.Runtime.$(RuntimeIdentifier)">
<PackageDirectory>$(MicrosoftNetCoreAppRuntimePackDir)</PackageDirectory>
</ResolvedRuntimePack>
</ItemGroup>
</Target>

</Project>
10 changes: 7 additions & 3 deletions src/coreclr/tools/aot/crossgen2/crossgen2.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,18 @@
<TargetSpec>$(TargetOSComponent)_$(TargetArchitectureForLocalJitBuild)_$(TargetArchitectureForSharedLibraries)</TargetSpec>

<JitInterfaceLibraryName>$(LibPrefix)jitinterface_$(TargetArchitectureForSharedLibraries)$(LibSuffix)</JitInterfaceLibraryName>
<!-- This will be provided when using the liveBuild, and unset otherwise. -->
<CoreCLRArtifactsPath Condition="'$(CoreCLRArtifactsPath)' == ''">$(RuntimeBinDir)$(CrossHostArch)</CoreCLRArtifactsPath>
</PropertyGroup>

<ItemGroup>
<Content Include="$(RuntimeBinDir)$(CrossHostArch)/$(JitInterfaceLibraryName)"
<Content Include="$(CoreCLRArtifactsPath)/$(JitInterfaceLibraryName)"
CopyToOutputDirectory="PreserveNewest"
CopyToPublishDirectory="PreserveNewest"
Link="%(FileName)%(Extension)"
/>

<Content Include="$(RuntimeBinDir)$(CrossHostArch)/$(LibPrefix)clrjit_*_$(TargetArchitectureForSharedLibraries)$(LibSuffix)"
<Content Include="$(CoreCLRArtifactsPath)/$(LibPrefix)clrjit_*_$(TargetArchitectureForSharedLibraries)$(LibSuffix)"
CopyToOutputDirectory="PreserveNewest"
CopyToPublishDirectory="PreserveNewest"
Link="%(FileName)%(Extension)"
Expand All @@ -104,7 +106,9 @@
<DiaSymReaderTargetArch>$(TargetArchitectureForSharedLibraries)</DiaSymReaderTargetArch>
<DiaSymReaderTargetArch Condition="'$(DiaSymReaderTargetArch)' == 'x64'">amd64</DiaSymReaderTargetArch>
<DiaSymReaderTargetArchFileName>Microsoft.DiaSymReader.Native.$(DiaSymReaderTargetArch).dll</DiaSymReaderTargetArchFileName>
<DiaSymReaderTargetArchPath>$(PkgMicrosoft_DiaSymReader_Native)\runtimes\win\native\$(DiaSymReaderTargetArchFileName)</DiaSymReaderTargetArchPath>
<DiaSymReaderTargetArchPath Condition="'$(PkgMicrosoft_DiaSymReader_Native)' != ''">$(PkgMicrosoft_DiaSymReader_Native)\runtimes\win\native\$(DiaSymReaderTargetArchFileName)</DiaSymReaderTargetArchPath>
<!-- When publishing we won't have the NuGet packages, so use the copy from the build artifacts directory. -->
<DiaSymReaderTargetArchPath Condition="'$(PkgMicrosoft_DiaSymReader_Native)' == ''">$(CoreCLRArtifactsPath)crossgen2/$(DiaSymReaderTargetArchFileName)</DiaSymReaderTargetArchPath>
</PropertyGroup>

<ItemGroup Condition="'$(TargetOS)' == 'windows'">
Expand Down
Loading