Fix NativeAOT publish including satellite assemblies despite embedding them#124192
Fix NativeAOT publish including satellite assemblies despite embedding them#124192
Conversation
… test Co-authored-by: dsplaisted <145043+dsplaisted@users.noreply.github.com>
|
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
| string exePath = Environment.ProcessPath ?? Assembly.GetExecutingAssembly().Location; | ||
| string publishDir = Path.GetDirectoryName(exePath); | ||
|
|
||
| if (string.IsNullOrEmpty(publishDir)) | ||
| { | ||
| Console.WriteLine("ERROR: Could not determine publish directory"); | ||
| return 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
We do this kind of testing in the SDK repo, not here. For example: https://github.com/dotnet/sdk/blob/fb940e86643e35e8d5fd400ac50e1987337bbc13/test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs#L153-L161
There was a problem hiding this comment.
Removed the test in commit 3feb4a7. Testing will be handled in the SDK repo as you suggested.
| <ItemGroup> | ||
| <IntermediateAssembly Remove="@(IntermediateAssembly)" /> | ||
| <IntermediateAssembly Include="$(NativeBinary)" /> | ||
| <IntermediateSatelliteAssembliesWithTargetPath Remove="@(IntermediateSatelliteAssembliesWithTargetPath)" /> |
There was a problem hiding this comment.
@copilot where is the logic that filters out satellite assemblies from NuGet references? I can see that we correctly don't preserve satellite assemblies from NuGet references. The issue is isolated to satellite assemblies from current assembly.
There was a problem hiding this comment.
The logic for filtering NuGet satellite assemblies is in the ComputeManagedAssembliesToCompileToNative task (lines 74-85), which outputs _AssembliesToSkipPublish. Line 25 then removes those from _ResolvedCopyLocalPublishAssets.
The fix in line 33 addresses the project's own satellite assemblies by removing IntermediateSatelliteAssembliesWithTargetPath, which the SDK would otherwise unconditionally add back to the publish list.
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Description
NativeAOT embeds satellite assemblies into the native binary but still copies them to the publish folder. This occurs because
ComputeLinkedFilesToPublishremovesIntermediateAssemblyand_DebugSymbolsIntermediatePathto prevent redundant publishing, but doesn't removeIntermediateSatelliteAssembliesWithTargetPath.Fix: Add removal of
IntermediateSatelliteAssembliesWithTargetPathinComputeLinkedFilesToPublishtarget:This follows the same pattern already used for
IntermediateAssemblyand_DebugSymbolsIntermediatePath.Customer Impact
Testing
Testing will be added in the SDK repo per review feedback. The fix can be validated by publishing a NativeAOT app with satellite assemblies and verifying that:
Risk
Minimal. One-line change following established pattern. Satellite assemblies remain embedded and functional; only removes redundant disk copies.
Original prompt
This section details on the original issue you should resolve
<issue_title>AOT publish includes satellite assemblies in publish folder</issue_title>
<issue_description>### Describe the bug
Publishing an AOT app will include satellite resource assemblies (for example
<language>\AppName.resources.dllin the output folder. It appears that these are also embedded in the NativeAOT executable, the app can still show localized strings even if these are deleted.Ideally if these satellite assemblies are not needed, they should not be copied to the publish folder. As it is, it's confusing and makes it look like they need to be deployed with the app.
To Reproduce
PublishAotto true in the .csproj fileExpected: No language subfolders and satellite assemblies in the publish folder
Actual: Language subfolders with satellite assemblies are present in the publish folder
Repro project
Binlog
Further technical details
.NET SDK version: 10.0.102</issue_description>
<agent_instructions>Fix this bug. Pay attention to the analysis from @baronfel about how to fix it.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@baronfel I gave this problem to Copilot CLI equipped with the viktorhofer/dotnet-skills plugin, and here was its analysis after publishing + inspecting the binlog:Root Cause Analysis
The satellite assemblies (e.g.
es\AppName.resources.dll) end up in the NativeAOT publish output because the NativeAOT build integration correctly embeds them into the native binary but fails to remove them from the publish file list, so the SDK's generic publish pipeline copies them to the output directory anyway.How satellite assemblies flow through the pipeline
Step 1: NativeAOT collects satellite assemblies for embedding
In
Microsoft.NETCore.Native.Publish.targets(dotnet/runtime),_ComputeIlcCompileInputsgathers satellite assemblies from two sources:_SatelliteAssembliesToPublish= satellite assemblies from package/project references (extracted from_ResolvedCopyLocalPublishAssetsbyComputeManagedAssembliesToCompileToNative)IntermediateSatelliteAssembliesWithTargetPath= the project's own satellite assemblies (e.g.es\52913-resx-in-nativeaot.resources.dll)These are passed to ILC via
--satellite:inMicrosoft.NETCore.Native.targets, which embeds them into the native binary. This part works correctly — the app can resolve localized strings even if the satellite DLLs are deleted from disk.Step 2:
ComputeLinkedFilesToPublishcleans up the publish list — but misses the project's own satellitesComputeLinkedFilesToPublishrunsBeforeTargets="ComputeResolvedFilesToPublishList"and modifies the publish list:This successfully removes package-reference satellite assemblies (via
_AssembliesToSkipPublish) and replaces the managed assembly with the native binary. But it does NOT removeIntermediateSatelliteAssembliesWithTargetPath.Step 3: The SDK unconditionally re-adds the project's satellite assemblies to publish
In
Microsoft.NET.Publish.targets(dotnet/sdk),ComputeResolvedFilesToPublishListunconditionally includes: