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

Publish crossgen as AOT if supported by the target platform #65948

Merged
merged 35 commits into from
Apr 1, 2022
Merged

Publish crossgen as AOT if supported by the target platform #65948

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1c237a9
Typo
agocke Feb 26, 2022
b3435c2
Publish crossgen as AOT if supported, and self-contained otherwise
agocke Feb 28, 2022
800b2d3
Remove debugging property
agocke Feb 28, 2022
2509e2f
Use CoreCLRArtifactsPath from liveBuild
agocke Mar 2, 2022
6a1c6ff
Fold crossgen_publish into crossgen
agocke Mar 3, 2022
bdc0042
Try to refactor targets
agocke Mar 3, 2022
b4b7721
Remove special PublishToDisk impl
agocke Mar 4, 2022
593dc27
Call restore in installer
agocke Mar 4, 2022
449fbf4
Hook more properties for installer partition
agocke Mar 4, 2022
c3b6fba
Use diasymreader copy from artifacts when publishing
agocke Mar 4, 2022
e210695
Fix Linux packaging
agocke Mar 4, 2022
69ae018
Include netfx facades in the crossgen toolpack
agocke Mar 5, 2022
a52f4ca
Disable crossgen package building on freebsd
agocke Mar 11, 2022
3fe8a1d
Adjust R2R logic and skip verification of Windows assemblies on non-w…
agocke Mar 12, 2022
4c3f31a
Move conditional exclusion just to WindowsDesktop libraries
agocke Mar 13, 2022
07c2f6c
Also include windows desktop
agocke Mar 13, 2022
f2f45e6
Always build pre-test for source build as well
agocke Mar 13, 2022
35fa89a
Use the coreclr centos version for sourcebuild since it has NativeAOT…
agocke Mar 15, 2022
5a8df96
Revert "Use the coreclr centos version for sourcebuild since it has N…
agocke Mar 15, 2022
497836e
Add -lssl -lcrypto to NativeAOT linking on Unix
agocke Mar 15, 2022
9897beb
Merge remote-tracking branch 'upstream/main' into crossgen-publish
agocke Mar 17, 2022
acb5fd5
Add testing step after publish
agocke Mar 16, 2022
9290f87
Include extension in check
agocke Mar 18, 2022
94c4545
Disable NativeAOT use in source build
agocke Mar 18, 2022
349dccc
Use OS and arch check instead of CrossBuild flag
agocke Mar 23, 2022
6333ee4
Respond to PR comments and skip verifying closure
agocke Mar 25, 2022
a8e28d0
Respond to PR comments
agocke Mar 29, 2022
4445b93
Set Configuration in sfxproj
agocke Mar 30, 2022
ff7c339
Remove Configuration set from sfxproj
agocke Mar 30, 2022
b897d68
Merge remote-tracking branch 'upstream/main' into crossgen-publish
agocke Mar 30, 2022
1e111f5
Disable NativeAOT for MacOS
agocke Mar 31, 2022
8553fee
Work around MacOS problem with single-file
agocke Mar 31, 2022
d8eeb9e
re-enable single-file
agocke Mar 31, 2022
f682eae
Copy over the S.P.C from the coreclr partition to ensure matching bits
agocke Mar 31, 2022
202eee4
Merge remote-tracking branch 'upstream/main' into crossgen-publish
agocke Apr 1, 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
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>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary because the RuntimeList.xml file is only created when running the libs.pretest subset.

Copy link
Member

Choose a reason for hiding this comment

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

Why was the DotNetBuildFromSource condition added initially? Is removing it going to break source build?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested it and couldn't find a problem. Not sure if there's any other testing I should be doing.

Copy link
Member

Choose a reason for hiding this comment

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

DotNetBuildFromSource skips this because it doesn't need a layout as it doesn't run tests. Shouldn't we get the xml manifests from the installer partition instead of relying on the test layouts?


<DefaultHostSubsets>host.native+host.tools</DefaultHostSubsets>
<DefaultHostSubsets Condition="'$(DotNetBuildFromSource)' != 'true'">$(DefaultHostSubsets)+host.pkg+host.tests</DefaultHostSubsets>
Expand Down
15 changes: 15 additions & 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>
agocke marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<Target Name="ResolveRuntimeFilesFromLocalBuild">
Expand Down Expand Up @@ -223,4 +224,18 @@
<PropertyGroup>
<BundledRuntimeIdentifierGraphFile>$(RuntimeIdGraphDefinitionFile)</BundledRuntimeIdentifierGraphFile>
</PropertyGroup>

<Target Name="RewriteRuntimePackDir"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not thrilled with this approach -- I think we should move to a mechanism to have a "well-known" runtime pack that can be used and preferred over all others. This might require new SDK functionality, I'm not sure.

Also, I'll have to update crossgen to build for net7.0 in that case. Not a big deal since we have a public preview out, but it's an interesting problem for net8.0, where we will have a net8.0 runtime pack, but no bootstrap SDK that would contain 8.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jkoritzinsky would appreciate your feedback on this in particular

Copy link
Member

Choose a reason for hiding this comment

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

Honestly, this approach (re-pointing to the live runtime pack for the current RID) looks fine to me.

Personally, I'd prefer that we require consuming projects to manually hook this target, especially since it's only used in once place.

When we start work on .NET 8, this should still be handleable without much issue as our work to update framework references .

Maybe we should refactor this into another .targets file like eng/runtimepacks.targets that can be manually included in projects that will use it (similar to the eng/targetingpacks.targets) file.

Condition="'$(RunningPublish)' == 'true'"
DependsOnTargets="ResolveRuntimeFilesFromLocalBuild"
BeforeTargets="ResolveRuntimePackAssets">
<ItemGroup>
<!-- Remove AspNetCore runtime pack since we don't build it locally -->
<ResolvedRuntimePack Remove="Microsoft.AspNetCore.App.Runtime.$(RuntimeIdentifier)" />

<ResolvedRuntimePack Update="Microsoft.NETCore.App.Runtime.$(RuntimeIdentifier)">
<PackageDirectory>$(MicrosoftNetCoreAppRuntimePackDir)</PackageDirectory>
</ResolvedRuntimePack>
</ItemGroup>
</Target>
</Project>
6 changes: 3 additions & 3 deletions src/coreclr/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
<IsNativeAotProject Condition="$(MSBuildProjectDirectory.Contains('nativeaot'))">true</IsNativeAotProject>
<BaseIntermediateOutputPath Condition="'$(IsNativeAotProject)' != 'true'">$([MSBuild]::NormalizeDirectory('$(ArtifactsObjDir)', 'coreclr', '$(MSBuildProjectName)'))</BaseIntermediateOutputPath>
<BaseIntermediateOutputPath Condition="'$(IsNativeAotProject)' == 'true'">$([MSBuild]::NormalizeDirectory('$(ArtifactsObjDir)', 'coreclr', 'nativeaot', '$(MSBuildProjectName)'))</BaseIntermediateOutputPath>
<IntermediateOutputPath Condition="'$(PlatformName)' == 'AnyCPU'">$(BaseIntermediateOutputPath)$(Configuration)\</IntermediateOutputPath>
<IntermediateOutputPath Condition="'$(PlatformName)' != 'AnyCPU'">$(BaseIntermediateOutputPath)$(PlatformName)\$(Configuration)\</IntermediateOutputPath>
<IntermediateOutputPath Condition="'$(PlatformName)' == 'AnyCPU'">$(BaseIntermediateOutputPath)$(RuntimeConfiguration)\</IntermediateOutputPath>
Copy link
Member

Choose a reason for hiding this comment

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

Why was this change needed? We don't appear to be using RuntimeConfiguration anywhere under src/coreclr - Configuration is used instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is because we're now building crossgen from the packaging project, which doesn't necessarily set the same variables as the coreclr partition. I think this is more robust, although it may not be complete. Hopefully any problems should be easy to trace down, though, as the files will simply not appear in the right directories.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the CoreCLR partition expects someone already made it so that Configuration is populated from RuntimeConfiguration. Would it be more in line with the project file expectations to set Configuration=$(RuntimeConfiguration) as an AdditionalProperty in Microsoft.NETCore.App.Crossgen2.sfxproj?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it, although I'm not sure this is robust going forward. I think it would make more sense for each partition to set the appropriate properties it needs to build, so it can be referenced transparently from anywhere, rather than have it set from the outside.

Probably a broader change though.

Copy link
Member

Choose a reason for hiding this comment

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

Probably a broader change though

Yep, we would also have to make sure it keeps working when opening/building the CoreCLR projects with VS (Configuration property has a meaning to VS).

<IntermediateOutputPath Condition="'$(PlatformName)' != 'AnyCPU'">$(BaseIntermediateOutputPath)$(PlatformName)\$(RuntimeConfiguration)\</IntermediateOutputPath>
<ProjectDir>$(MSBuildThisFileDirectory)</ProjectDir>
<RuntimeBinDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'coreclr', '$(TargetOS).$(TargetArchitecture).$(Configuration)'))</RuntimeBinDir>
<RuntimeBinDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'coreclr', '$(TargetOS).$(TargetArchitecture).$(RuntimeConfiguration)'))</RuntimeBinDir>

<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<SignAssembly Condition="'$(UsingMicrosoftNETSdk)' != 'true'">false</SignAssembly>
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<LinkerArg Include="-static" Condition="'$(StaticallyLinked)' == 'true'" />
<LinkerArg Include="-lgssapi_krb5" Condition="'$(TargetOS)' != 'OSX' and '$(StaticallyLinked)' != 'true'" />
<LinkerArg Include="-lrt" Condition="'$(TargetOS)' != 'OSX'" />
<LinkerArg Include="-lssl -lcrypto" Condition="'$(TargetOS)' != 'OSX'" />
agocke marked this conversation as resolved.
Show resolved Hide resolved
<LinkerArg Include="-licucore" Condition="'$(TargetOS)' == 'OSX'" />
<LinkerArg Include="-dynamiclib" Condition="'$(TargetOS)' == 'OSX' and '$(NativeLib)' == 'Shared'" />
<LinkerArg Include="-shared" Condition="'$(TargetOS)' != 'OSX' and '$(NativeLib)' == 'Shared'" />
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 @@ -389,7 +389,7 @@ public sealed record SymUnmanagedReaderRcw(IntPtr Inst) : ISymUnmanagedReader
private bool _disposed = false;

// This is not actually called in any code path right now. Rather than implement this
// without testing, it's been lefted throwing an exception. If this code path is ever
// without testing, it's been left throwing an exception. If this code path is ever
// reached, it can be implemented and tested.
public int GetMethod(int methodToken, out ISymUnmanagedMethod method) => throw new NotImplementedException();

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
45 changes: 40 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,44 @@
<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>
<OutputPath>$(RuntimeBinDir)crossgen2</OutputPath>
<!-- Trimming is not currently working, but set the appropriate feature flags for NativeAOT -->
<PublishTrimmed Condition="'$(NativeAotSupported)' == 'true'">true</PublishTrimmed>
<RuntimeIdentifiers Condition="'$(RunningPublish)' != '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>
</PropertyGroup>

<Import Project="crossgen2.props" />

<PropertyGroup Condition="'$(NativeAotSupported)' != 'true'">
<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>

<!-- Forced by ILLink targets; we should fix the SDK -->
<SelfContained Condition="'$(RunningPublish)' == 'true'">true</SelfContained>
</PropertyGroup>

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

</Project>
9 changes: 6 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,17 @@
<TargetSpec>$(TargetOSComponent)_$(TargetArchitectureForLocalJitBuild)_$(TargetArchitectureForSharedLibraries)</TargetSpec>

<JitInterfaceLibraryName>$(LibPrefix)jitinterface_$(TargetArchitectureForSharedLibraries)$(LibSuffix)</JitInterfaceLibraryName>
<CoreCLRArtifactsPath Condition="'$(CoreCLRArtifactsPath)' == ''">$(RuntimeBinDir)$(CrossHostArch)</CoreCLRArtifactsPath>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: could you add the same comment as in ILCompiler.props? (<!-- This will be provided when using the liveBuild, and unset otherwise. -->)

</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 +105,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