Skip to content

[NativeAOT] Update ILCompiler paths to handle PublishAotUsingRuntimePack=true #85996

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

Closed
wants to merge 1 commit into from
Closed

[NativeAOT] Update ILCompiler paths to handle PublishAotUsingRuntimePack=true #85996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@
Text="Add a PackageReference for '$(_hostPackageName)' to allow cross-compilation for $(_targetArchitecture)" />

<!-- NativeAOT runtime pack assemblies need to be defined to avoid the default CoreCLR implementations being set as compiler inputs -->
<Error Condition="'@(PrivateSdkAssemblies)' == ''" Text="The PrivateSdkAssemblies ItemGroup is required for _ComputeAssembliesToCompileToNative" />
<Error Condition="'@(FrameworkAssemblies)' == ''" Text="The FrameworkAssemblies ItemGroup is required for _ComputeAssembliesToCompileToNative" />
<Error Condition="'@(PrivateSdkAssemblies)' == '' and '$(PublishAotUsingRuntimePack)' != 'true'" Text="The PrivateSdkAssemblies ItemGroup is required for _ComputeAssembliesToCompileToNative" />
<Error Condition="'@(FrameworkAssemblies)' == '' and '$(PublishAotUsingRuntimePack)' != 'true'" Text="The FrameworkAssemblies ItemGroup is required for _ComputeAssembliesToCompileToNative" />

<ComputeManagedAssembliesToCompileToNative
Assemblies="@(_ResolvedCopyLocalPublishAssets)"
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ The .NET Foundation licenses this file to you under the MIT license.

<ItemGroup>
<NativeLibrary Condition="'$(IlcMultiModule)' == 'true'" Include="$(SharedLibrary)" />
<NativeLibrary Condition="'$(NativeLib)' == '' and '$(CustomNativeMain)' != 'true'" Include="$(IlcSdkPath)libbootstrapper.a" />
<NativeLibrary Condition="'$(NativeLib)' != '' or '$(CustomNativeMain)' == 'true'" Include="$(IlcSdkPath)libbootstrapperdll.a" />
<NativeLibrary Include="$(IlcSdkPath)$(FullRuntimeName).a" />
<NativeLibrary Include="$(IlcSdkPath)$(EventPipeName)$(LibFileExt)" />
<NativeLibrary Condition="'$(LinkStandardCPlusPlusLibrary)' != 'true' and '$(StaticICULinking)' != 'true'" Include="$(IlcSdkPath)libstdc++compat.a" />
<NativeLibrary Condition="'$(NativeLib)' == '' and '$(CustomNativeMain)' != 'true'" Include="$(IlcFrameworkSdkPath)libbootstrapper.a" />
<NativeLibrary Condition="'$(NativeLib)' != '' or '$(CustomNativeMain)' == 'true'" Include="$(IlcFrameworkSdkPath)libbootstrapperdll.a" />
<NativeLibrary Include="$(IlcFrameworkSdkPath)$(FullRuntimeName).a" />
<NativeLibrary Include="$(IlcFrameworkSdkPath)$(EventPipeName)$(LibFileExt)" />
<NativeLibrary Condition="'$(LinkStandardCPlusPlusLibrary)' != 'true' and '$(StaticICULinking)' != 'true'" Include="$(IlcFrameworkSdkPath)libstdc++compat.a" />
</ItemGroup>

<ItemGroup>
Expand Down
27 changes: 21 additions & 6 deletions src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,26 +126,41 @@ The .NET Foundation licenses this file to you under the MIT license.

<!-- The properties below need to be defined only after we've found the correct runtime package reference -->
<Target Name="SetupProperties" DependsOnTargets="$(IlcSetupPropertiesDependsOn)" BeforeTargets="Publish">
<ItemGroup>
<_NETCoreAppFrameworkReference Include="@(ResolvedFrameworkReference)" Condition="'%(ResolvedFrameworkReference.RuntimePackName)' == 'Microsoft.NETCore.App.Runtime.NativeAOT.$(RuntimeIdentifier)'" />
</ItemGroup>

<PropertyGroup>
<!-- Define paths used in build targets to point to the runtime-specific ILCompiler implementation -->
<IlcToolsPath Condition="'$(IlcToolsPath)' == ''">$(IlcHostPackagePath)\tools\</IlcToolsPath>
<IlcSdkPath Condition="'$(IlcSdkPath)' == ''">$(RuntimePackagePath)\sdk\</IlcSdkPath>
<IlcFrameworkPath Condition="'$(IlcFrameworkPath)' == ''">$(RuntimePackagePath)\framework\</IlcFrameworkPath>
<IlcFrameworkNativePath Condition="'$(IlcFrameworkNativePath)' == ''">$(RuntimePackagePath)\framework\</IlcFrameworkNativePath>
<IlcFrameworkPath Condition="'$(IlcFrameworkPath)' == '' and '$(PublishAotUsingRuntimePack)' != 'true'">$(RuntimePackagePath)\framework\</IlcFrameworkPath>
<_NETCoreAppRuntimePackPath>%(_NETCoreAppFrameworkReference.RuntimePackPath)/runtimes/$(RuntimeIdentifier)/</_NETCoreAppRuntimePackPath>
<IlcFrameworkNativePath Condition="'$(IlcFrameworkNativePath)' == '' and '$(PublishAotUsingRuntimePack)' == 'true'">$(_NETCoreAppRuntimePackPath)\native\</IlcFrameworkNativePath>
<IlcFrameworkNativePath Condition="'$(IlcFrameworkNativePath)' == '' and '$(PublishAotUsingRuntimePack)' != 'true'">$(RuntimePackagePath)\framework\</IlcFrameworkNativePath>
<IlcFrameworkSdkPath Condition="'$(IlcFrameworkSdkPath)' == '' and '$(PublishAotUsingRuntimePack)' == 'true'">$(IlcFrameworkNativePath)</IlcFrameworkSdkPath>
<IlcFrameworkSdkPath Condition="'$(IlcFrameworkSdkPath)' == '' and '$(PublishAotUsingRuntimePack)' != 'true'">$(IlcSdkPath)</IlcFrameworkSdkPath>
<IlcMibcPath Condition="'$(IlcMibcPath)' == ''">$(RuntimePackagePath)\mibc\</IlcMibcPath>
</PropertyGroup>

<ItemGroup>
<PrivateSdkAssemblies Include="$(IlcSdkPath)*.dll" />
<ItemGroup Condition="'$(PublishAotUsingRuntimePack)' == 'true'">
<PrivateSdkAssemblies Include="$(IlcFrameworkSdkPath)*.dll"/>
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to name this IlcPrivateSdkPath since that's where PrivateSdkAssemblies come from?

Suggested change
<PrivateSdkAssemblies Include="$(IlcFrameworkSdkPath)*.dll"/>
<PrivateSdkAssemblies Include="$(IlcPrivateSdkPath)*.dll"/>

<FrameworkAssemblies Include="@(RuntimePackAsset)" Condition="'%(Extension)' == '.dll'" />
<DefaultFrameworkAssemblies Include="@(FrameworkAssemblies)" />
Copy link
Member

Choose a reason for hiding this comment

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

Why aren't PrivateSdkAssemblies included in DefaultFrameworkAssemblies in this case?

Copy link
Member Author

@filipnavara filipnavara May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's not exactly intuitive but RuntimePackAsset already contains the files that end up in PrivateSdkAssemblies, so FrameworkAssemblies already contains the union of the two sets.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, FrameworkAssemblies itself is not used beyond this point, so the additional assemblies included in it have no effect.

Copy link
Member

Choose a reason for hiding this comment

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

Also, FrameworkAssemblies itself is not used beyond this point, so the additional assemblies included in it have no effect.

Sorry, I got confused with this.
Aren't FrameworkAssemblies used through DefaultFrameworkAssemblies to populate IlcReference:

runtime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets

Line 155 in ea94440

<IlcReference Include="@(DefaultFrameworkAssemblies)" />

Copy link
Member Author

@filipnavara filipnavara May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

DefaultFrameworkAssemblies is used beyond the point and it's expected to contain the union of lib/net8.0/*.dll and native/*.dll from the runtime pack.

In the case of PublishAotUsingRuntimePack=false these come from two different source directories and get merged into one DefaultFrameworkAssemblies item group.

In the case of PublishAotUsingRuntimePack=true all the files in the DefaultFrameworkAssemblies item group come from the RuntimePackAsset item group which already contains both the lib/net8.0/*.dll files and the native/*.dll files.

I was just trying to clarify that FrameworkAssemblies in the PublishAotUsingRuntimePack=true case will also include PrivateSdkAssemblies (native/*.dll), while in the PublishAotUsingRuntimePack=false case it won't include those files. However, FrameworkAssemblies itself is not used beyond this point, so the distinction doesn't matter.

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 can just rewrite it as to make it clearer:

-      <FrameworkAssemblies Include="@(RuntimePackAsset)" Condition="'%(Extension)' == '.dll'" />
-      <DefaultFrameworkAssemblies Include="@(FrameworkAssemblies)" />
+      <DefaultFrameworkAssemblies Include="@(RuntimePackAsset)" Condition="'%(Extension)' == '.dll'" />

Copy link
Member

Choose a reason for hiding this comment

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

Thank you very much for the clarification!

Regarding your last comment:

I can just rewrite it as to make it clearer:

Would it make sense to keep having a single ItemGroup and just distinguish what gets included in DefaultFrameworkAssemblies based on PublishAotUsingRuntimePack value?

</ItemGroup>

<ItemGroup Condition="'$(PublishAotUsingRuntimePack)' != 'true'">
<PrivateSdkAssemblies Include="$(IlcFrameworkSdkPath)*.dll"/>
<!-- Exclude unmanaged dlls -->
<FrameworkAssemblies Include="$(IlcFrameworkPath)*.dll" Exclude="$(IlcFrameworkPath)*.Native.dll;$(IlcFrameworkPath)msquic.dll" />

<MibcFile Include="$(IlcMibcPath)*.mibc" Condition="'$(IlcPgoOptimize)' == 'true'" />

<DefaultFrameworkAssemblies Include="@(FrameworkAssemblies)" />
<DefaultFrameworkAssemblies Include="@(PrivateSdkAssemblies)" />
</ItemGroup>

<ItemGroup>
<MibcFile Include="$(IlcMibcPath)*.mibc" Condition="'$(IlcPgoOptimize)' == 'true'" />
</ItemGroup>
</Target>

<Target Name="ComputeIlcCompileInputs" DependsOnTargets="$(IlcDynamicBuildPropertyDependencies)" BeforeTargets="Publish">
Expand Down