-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix embedded resources missing in PublishAot + NoBuild binaries #12791
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
Adds PrepareResources dependency to _GenerateCompileDependencyCache and implements fallback to populate _CoreCompileResourceInputs when _GenerateCompileInputs is skipped Co-authored-by: JanProvaznik <[email protected]>
| <Target Name="_GenerateCompileDependencyCache" Condition="'$(DesignTimeBuild)' != 'true' and '$(BuildingProject)' == 'true'" DependsOnTargets="ResolveAssemblyReferences;_GenerateCompileInputs"> | ||
| <Target Name="_GenerateCompileDependencyCache" Condition="'$(DesignTimeBuild)' != 'true' and '$(BuildingProject)' == 'true'" DependsOnTargets="ResolveAssemblyReferences;PrepareResources;_GenerateCompileInputs"> | ||
|
|
||
| <!-- If _GenerateCompileInputs was skipped and _CoreCompileResourceInputs is empty, populate it here. | ||
| This handles the NoBuild publish scenario where _GenerateCompileInputs runs once but its items | ||
| are not available when CoreCompile needs them later. PrepareResources dependency ensures that | ||
| resource metadata (OutputResource) is set before we populate the items. --> | ||
| <ItemGroup Condition="'@(_CoreCompileResourceInputs)' == ''"> | ||
| <_CoreCompileResourceInputs Include="@(EmbeddedResource->'%(OutputResource)')" Condition="'%(EmbeddedResource.WithCulture)' == 'false' and '%(EmbeddedResource.Type)' == 'Resx'" /> | ||
| <_CoreCompileResourceInputs Include="@(EmbeddedResource)" Condition="'%(EmbeddedResource.WithCulture)' == 'false' and '%(EmbeddedResource.Type)' == 'Non-Resx' " /> | ||
| <_CoreCompileResourceInputs Include="@(ManifestResourceWithNoCulture)" Condition="'%(ManifestResourceWithNoCulture.EmittedForCompatibilityOnly)'==''"> | ||
| <Type>Resx</Type> | ||
| <WithCulture>false</WithCulture> | ||
| </_CoreCompileResourceInputs> | ||
| <_CoreCompileResourceInputs Include="@(ManifestNonResxWithNoCultureOnDisk)" Condition="'%(ManifestNonResxWithNoCultureOnDisk.EmittedForCompatibilityOnly)'==''"> | ||
| <Type>Non-Resx</Type> | ||
| <WithCulture>false</WithCulture> | ||
| </_CoreCompileResourceInputs> | ||
| </ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this is poorly maintainable adding a lot of code for a specific scenario, can it be done cleaner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Refactored in f61713e to make _GenerateCompileInputs always run (via Outputs="") and clear items before repopulating. This eliminates the code duplication and keeps a single source of truth for the resource input logic.
…ng logic Instead of duplicating resource population logic in _GenerateCompileDependencyCache, make _GenerateCompileInputs always run by adding Outputs="" and clearing items before repopulating to avoid duplicates. This is much more maintainable. Co-authored-by: JanProvaznik <[email protected]>
JanProvaznik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in depth analyze the PR which regressed this and figure out an appropriate mitigation while preserving the caching benefit
create an end to end test for it
@copilot
…scenarios Instead of forcing the target to always run with Outputs="", use CallTarget to explicitly invoke _GenerateCompileInputs from _GenerateCompileDependencyCache. This ensures _CoreCompileResourceInputs is populated even when MSBuild's target skipping logic would normally skip _GenerateCompileInputs. Also added PrepareResources dependency to ensure resource metadata is set before calling _GenerateCompileInputs. Added end-to-end test EmbeddedResourcesWithNoBuildPublish to verify embedded resources work correctly with NoBuild publish. Co-authored-by: JanProvaznik <[email protected]>
Done in 33d17a9: Analysis of PR #10928: Added Mitigation: Use Test: Added |
dd21c87 to
11f3824
Compare
Summary
Fixes issue where publishing with
PublishAot=trueandNoBuild=trueproduces native binaries without embedded resources, causingMissingManifestResourceExceptionat runtime.Customer Impact
Applications using Native AOT with NoBuild publish workflows would fail at runtime with missing resource exceptions. This affects customers using embedded resources (e.g., .resx files) in their Native AOT applications when using the NoBuild optimization during publish.
Regression?
Yes - introduced by PR #10928 which added
_GenerateCompileInputsas a dependency to_GenerateCompileDependencyCache. During NoBuild publish scenarios,PrepareResourceswasn't running early enough in the dependency chain, soEmbeddedResourceitems didn't have their metadata (especiallyOutputResource) set when_GenerateCompileInputstried to use them. This resulted in_CoreCompileResourceInputsbeing empty, causing CoreCompile to produce resource-less DLLs during RID-specific recompilation.Testing
EmbeddedResourcesWithNoBuildPublishto verify NoBuild publish with embedded resourcesEmbeddedResourcesFileCompileCachefrom PR Cache Embedded Resources during compilation task #10928 still passes (incremental build scenario)dotnet build -p:PublishAot=truefollowed bydotnet publish -p:PublishAot=true -p:NoBuild=trueproduces working binary with embedded resourcesRisk
Low - The fix adds
PrepareResourcesto the dependencies of_GenerateCompileDependencyCacheto ensure resource metadata is set before_GenerateCompileInputsruns. This is a minimal change that ensures proper target ordering. The existing dependency on_GenerateCompileInputsalready works correctly - MSBuild ensures dependencies inDependsOnTargetsrun in order within the same invocation. Preserves PR #10928's fast up-to-date check benefit while fixing NoBuild scenarios.Changes Made:
Modified
_GenerateCompileDependencyCachetarget inMicrosoft.Common.CurrentVersion.targets:PrepareResourcesto dependencies to ensure resource metadata (OutputResource) is set before_GenerateCompileInputsrunsAdded end-to-end test:
2. Created
EmbeddedResourcesWithNoBuildPublishtest inProjectCacheTests.csto verify embedded resources work correctly withNoBuild=trueduring publishOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.