Skip to content

Conversation

@tmat
Copy link
Member

@tmat tmat commented Jun 4, 2025

Iteration on #11893, which had to be reverted.
Adds condition to only update OutputResource if it is not set already.

Copilot AI review requested due to automatic review settings June 4, 2025 21:26
@tmat
Copy link
Member Author

tmat commented Jun 4, 2025

@rainersigwald @YuliiaKovalova ptal

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the design-time build by ensuring resource items carry an explicit output path before code generation runs.

  • Introduce a new AssignEmbeddedResourceOutputPaths target and wire it into the PrepareResourceNamesDependsOn sequence.
  • Refactor OutputResources to use the newly assigned %(OutputResource) metadata.
  • Add a dependency for _GenerateCompileInputs on PrepareResourceNames to guarantee metadata is set.
Comments suppressed due to low confidence (2)

src/Tasks/Microsoft.Common.CurrentVersion.targets:3255

  • There are no automated tests covering the new AssignEmbeddedResourceOutputPaths target. Consider adding unit or integration tests to verify that OutputResource metadata is correctly set for EmbeddedResource items.
<Target Name="AssignEmbeddedResourceOutputPaths">

src/Tasks/Microsoft.Common.CurrentVersion.targets:3253

  • [nitpick] The XML comment could be expanded to note which downstream targets rely on this metadata (e.g., _GenerateCompileInputs and GetItemTargetPaths), improving clarity for future maintainers.
Sets OutputResource metadata on EmbeddedResource items. This metadata is used in design time build without running ResGen target.

@rainersigwald
Copy link
Member

Looks ok to me but should we test it in the VMR too since it broke there?

@tmat
Copy link
Member Author

tmat commented Jun 5, 2025

@rainersigwald How do I do that?

@rainersigwald
Copy link
Member

Create a draft PR with this same content in dotnet/dotnet's src/msbuild directory.

@tmat
Copy link
Member Author

tmat commented Jun 6, 2025

Validation PR: dotnet/dotnet#987

@ghost ghost assigned MichalPavlik and YuliiaKovalova and unassigned MichalPavlik Jul 7, 2025
@YuliiaKovalova YuliiaKovalova merged commit 89e4aff into dotnet:main Jul 8, 2025
10 checks passed
@tmat
Copy link
Member Author

tmat commented Jul 8, 2025

Oh, wait ... this was still broken. I had to tweak the code in the validation PR. Let me submit a fix...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants