Skip to content

Clean up AOT publish process #73416

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

Merged
LakshanF merged 29 commits into from
Aug 15, 2022
Merged

Clean up AOT publish process #73416

Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b9d18e1
Clean up AOT publish process
LakshanF Aug 4, 2022
62a302a
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 5, 2022
b5f37f0
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 5, 2022
3b86b66
Merge branch 'main' of https://github.com/dotnet/runtime into CleanPu…
LakshanF Aug 5, 2022
e992c9b
FB and working around running native aot tests
LakshanF Aug 7, 2022
dcf710c
update crossgen2 reference
LakshanF Aug 8, 2022
da529fd
moving package version change out of a target as per FB
LakshanF Aug 8, 2022
3cb58e6
Fix nativeaot test take 2
LakshanF Aug 9, 2022
5696f2d
disabling a failing test
LakshanF Aug 10, 2022
941d558
Clean up AOT publish process
LakshanF Aug 4, 2022
363fd28
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 5, 2022
48b0f74
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 5, 2022
c1fb84e
FB and working around running native aot tests
LakshanF Aug 7, 2022
eb13dc8
update crossgen2 reference
LakshanF Aug 8, 2022
294c463
moving package version change out of a target as per FB
LakshanF Aug 8, 2022
2a07fcc
Fix nativeaot test take 2
LakshanF Aug 9, 2022
8038c58
disabling a failing test
LakshanF Aug 10, 2022
45c21cb
Remove TrimMode workaround
LakshanF Aug 10, 2022
7867291
Merge branch 'CleanPublishAot' of https://github.com/LakshanF/runtime…
LakshanF Aug 11, 2022
5907416
work around a test issue
LakshanF Aug 11, 2022
b12795b
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 11, 2022
80cedc4
Update src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILComp…
LakshanF Aug 11, 2022
a52fcdd
Merge branch 'CleanPublishAot' of https://github.com/LakshanF/runtime…
LakshanF Aug 11, 2022
80faf94
update the version check for package
LakshanF Aug 12, 2022
141c61f
Cross target support, requires changes from SDK
LakshanF Aug 14, 2022
de74e2b
Change ResolvedTargetILCompilerPack to match SDK changes
LakshanF Aug 15, 2022
3d48f00
Merge branch 'main' into CleanPublishAot
LakshanF Aug 15, 2022
62ce9bd
FB
LakshanF Aug 15, 2022
ff574f8
Trigger Build to fix interop break
LakshanF Aug 15, 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
2 changes: 1 addition & 1 deletion eng/testing/tests.singlefile.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<SelfContained>true</SelfContained>
</PropertyGroup>

<Import Project="$(CoreCLRBuildIntegrationDir)Microsoft.DotNet.ILCompiler.targets" Condition="'$(TestNativeAot)' == 'true'" />
<Import Project="$(CoreCLRBuildIntegrationDir)Microsoft.DotNet.ILCompiler.SingleEntry.targets" Condition="'$(TestNativeAot)' == 'true'" />

<ItemGroup Condition="'$(TestNativeAot)' == 'true'">
<RdXmlFile Include="$(MSBuildThisFileDirectory)default.rd.xml" />
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/nativeaot/BuildIntegration/BuildFrameworkNativeObjects.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<DebugSymbols Condition="'$(DebugSymbols)' == ''">true</DebugSymbols>
</PropertyGroup>

<Import Project="$(MSBuildThisFileDirectory)\Microsoft.DotNet.ILCompiler.targets" Condition="'$(IlcCalledViaPackage)' == 'true'" />
<Import Project="$(MSBuildThisFileDirectory)\Microsoft.DotNet.ILCompiler.SingleEntry.targets" Condition="'$(IlcCalledViaPackage)' == 'true'" />
<Import Project="Microsoft.NETCore.Native.targets" Condition="'$(IlcCalledViaPackage)' == ''" />

<Target Name="BuildAllFrameworkLibraries"
Expand Down
37 changes: 16 additions & 21 deletions ...ation/Microsoft.DotNet.ILCompiler.targets → ...oft.DotNet.ILCompiler.SingleEntry.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup Condition="'$(RuntimeIdentifier)' != ''">

<!-- Define the name of the runtime specific compiler package to import -->
<OSIdentifier Condition="$(RuntimeIdentifier.StartsWith('win'))">win</OSIdentifier>
<OSIdentifier Condition="$(RuntimeIdentifier.StartsWith('osx'))">osx</OSIdentifier>
Expand All @@ -26,38 +25,34 @@

<IlcHostArch Condition="'$(IlcHostArch)' == ''">$(OSHostArch)</IlcHostArch>
<IlcHostPackageName>runtime.$(OSIdentifier)-$(IlcHostArch).Microsoft.DotNet.ILCompiler</IlcHostPackageName>

<IlcCalledViaPackage>true</IlcCalledViaPackage>

</PropertyGroup>

<PropertyGroup>

<!-- If the NativeAOT toolchain is being consumed via package, runtime-specific properties must be set before compilation can proceed -->
<ImportRuntimeIlcPackageTargetDependsOn>RunResolvePackageDependencies</ImportRuntimeIlcPackageTargetDependsOn>
<IlcSetupPropertiesDependsOn>ImportRuntimeIlcPackageTarget</IlcSetupPropertiesDependsOn>
<IlcDynamicBuildPropertyDependencies>SetupProperties</IlcDynamicBuildPropertyDependencies>

</PropertyGroup>

<!-- Locate the runtime package according to the current target runtime -->
<Target Name="ImportRuntimeIlcPackageTarget" Condition="'$(BuildingFrameworkLibrary)' != 'true' AND $(IlcCalledViaPackage) == 'true'" DependsOnTargets="$(ImportRuntimeIlcPackageTargetDependsOn)" BeforeTargets="Publish">
<!-- This targets file is imported by the SDK when the AotRuntimePackageLoadedViaSDK property is set. SDK resolves runtime package paths differently-->
<Error Condition="'@(ResolvedILCompilerPack)' == '' AND '$(AotRuntimePackageLoadedViaSDK)' == 'true'" Text="The ResolvedILCompilerPack ItemGroup is required for target ImportRuntimeIlcPackageTarget" />
<!-- NativeAOT runtime pack assemblies need to be defined to avoid the default CoreCLR implementations being set as compiler inputs -->
<Error Condition="'@(PackageDefinitions)' == '' AND '$(AotRuntimePackageLoadedViaSDK)' != 'true'" Text="The PackageDefinitions ItemGroup is required for target ImportRuntimeIlcPackageTarget" />

<ItemGroup>
<!-- If called via package instead of the SDK, update the runtime package version to match the build package -->
<_PackageReferenceExceptILCompiler Include="@(PackageReference)" Exclude="Microsoft.DotNet.ILCompiler" />
<_ILCompilerPackageReference Include="@(PackageReference)" Exclude="@(_PackageReferenceExceptILCompiler)" />
<KnownILCompilerPack Update="Microsoft.DotNet.ILCompiler" Condition="@(_ILCompilerPackageReference->'%(Identity)')=='Microsoft.DotNet.ILCompiler'">
<ILCompilerPackVersion>@(_ILCompilerPackageReference->'%(Version)')</ILCompilerPackVersion>
</KnownILCompilerPack>
</ItemGroup>

<!-- This targets file is imported by the SDK when the AotRuntimePackageLoadedViaSDK property is set. Use the SDK runtime package resolve property to set down stream properties -->
<PropertyGroup Condition="'$(AotRuntimePackageLoadedViaSDK)' == 'true'">
<RuntimePackagePath>@(ResolvedILCompilerPack->'%(PackageDirectory)')</RuntimePackagePath>
<IlcHostPackagePath>@(ResolvedILCompilerPack->'%(PackageDirectory)')</IlcHostPackagePath>
<IlcPath>>@(ResolvedILCompilerPack->'%(PackageDirectory)')</IlcPath>
</PropertyGroup>
<!-- Locate the runtime package according to the current target runtime -->
<Target Name="ImportRuntimeIlcPackageTarget" Condition="'$(BuildingFrameworkLibrary)' != 'true' and '$(PublishAot)' == 'true' and $(IlcCalledViaPackage) == 'true'" DependsOnTargets="$(ImportRuntimeIlcPackageTargetDependsOn)" BeforeTargets="Publish">
<Error Condition="'@(ResolvedILCompilerPack)' == ''" Text="The ResolvedILCompilerPack ItemGroup is required for target ImportRuntimeIlcPackageTarget" />

<!-- Use the non-SDK runtime package resolve property to set down stream properties if there is an explicit reference in the project -->
<PropertyGroup Condition="'$(AotRuntimePackageLoadedViaSDK)' != 'true'">
<RuntimePackagePath Condition="'%(PackageDefinitions.Name)' == '$(RuntimeIlcPackageName)'">%(PackageDefinitions.ResolvedPath)</RuntimePackagePath>
<IlcHostPackagePath Condition="'%(PackageDefinitions.Name)' == '$(IlcHostPackageName)'">%(PackageDefinitions.ResolvedPath)</IlcHostPackagePath>
<PropertyGroup>
<IlcHostPackagePath Condition="'@(ResolvedILCompilerPack)' == '$(IlcHostPackageName)'">@(ResolvedILCompilerPack->'%(PackageDirectory)')</IlcHostPackagePath>
<RuntimePackagePath Condition="'@(ResolvedTargetILCompilerPack)' == '$(RuntimeIlcPackageName)'">@(ResolvedTargetILCompilerPack->'%(PackageDirectory)')</RuntimePackagePath>
<RuntimePackagePath Condition="'@(ResolvedTargetILCompilerPack)' != '$(RuntimeIlcPackageName)'">@(ResolvedILCompilerPack->'%(PackageDirectory)')</RuntimePackagePath>
</PropertyGroup>

</Target>
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILCompiler.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ Copyright (c) .NET Foundation. All rights reserved.
***********************************************************************************************
-->
<Project>

<PropertyGroup>
<!-- Set the publishAot property to true if not set-->
<PublishAot Condition="'$(PublishAot)' == ''">true</PublishAot>
Copy link
Member

@MichalStrehovsky MichalStrehovsky Aug 17, 2022

Choose a reason for hiding this comment

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

If I understand it correctly, this was needed because we do the ".targets included from a .props file" antipattern - I think we discussed this in one of the past pull request - to do the real fix, we likely need to break up Microsoft.DotNet.ILCompiler.targets into Microsoft.DotNet.ILCompiler.targets and Microsoft.DotNet.ILCompiler.props as needed.

(The convention is that .props are included before the user project, .targets get included after the user provided stuff - we would see PublishAot being set there if the user wants it.)

Copy link
Member

@MichalStrehovsky MichalStrehovsky Aug 17, 2022

Choose a reason for hiding this comment

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

Most of the stuff that is already in .targets should be in .targets. We wouldn't probably have much (if anything?) in the props.

Copy link
Member

@sbomer sbomer Aug 17, 2022

Choose a reason for hiding this comment

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

I don't think it's because we are importing targets from props (if we are, that should probably be fixed) - this was needed to preserve the OOB package behavior where the packagereference was all that was needed to turn on AOT publish (you didn't need to set PublishAot explicitly).

Copy link
Member

@MichalStrehovsky MichalStrehovsky Aug 17, 2022

Choose a reason for hiding this comment

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

Ah, you're right. I saw .targets in a .props and I thought it's an Import, but it's just storing it in a property for later.

<!-- N.B. The ILCompilerTargetsPath is used as a sentinel to indicate a version of this file has already been imported. It will also be the path
used to import the targets later in the SDK. -->
<ILCompilerTargetsPath>$(MSBuildThisFileDirectory)Microsoft.DotNet.ILCompiler.targets</ILCompilerTargetsPath>
<ILCompilerTargetsPath>$(MSBuildThisFileDirectory)Microsoft.DotNet.ILCompiler.SingleEntry.targets</ILCompilerTargetsPath>
</PropertyGroup>
</Project>
2 changes: 1 addition & 1 deletion src/coreclr/tools/aot/crossgen2/crossgen2.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</PropertyGroup>

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

<Target Name="RewriteRuntimePackDir"
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/tests.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@

<!-- https://github.com/dotnet/runtime/issues/72908 -->
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Reflection.MetadataLoadContext\tests\System.Reflection.MetadataLoadContext.Tests.csproj" />

<!-- Test needs to copy .so file: https://github.com/dotnet/runtime/issues/72987 -->
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.IO.Ports\tests\System.IO.Ports.Tests.csproj"
Condition="'$(TargetOS)' != 'windows'" />
Expand Down