Skip to content

Add support for IfDifferent #51826

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for IfDifferent #51826

wants to merge 3 commits into from
+126 −7

Conversation

@JanKrivanek
Copy link
Member

@JanKrivanek JanKrivanek commented Nov 20, 2025

Context

dotnet/msbuild#11052 Added support for IfDifferent value of CopyToOutputDirectory metadata.
The building/publishing of static web assets was not honoring this. It should be supported, so that the new copy action can be added to Project System copy action dropdown without risk of causing issues.

FYI @baronfel @drewnoakes

Copilot AI review requested due to automatic review settings November 20, 2025 14:22
@github-actions github-actions bot added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Nov 20, 2025
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Nov 20, 2025

Thanks for your PR, @@JanKrivanek.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

Copilot finished reviewing on behalf of JanKrivanek November 20, 2025 14:24
Copilot AI reviewed Nov 20, 2025
View reviewed changes
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 adds support for the IfDifferent value of CopyToOutputDirectory and CopyToPublishDirectory metadata for static web assets, aligning with the new capability added to MSBuild in dotnet/msbuild#11052. This enables Project System to offer the new copy action in its dropdown without causing issues with static web asset handling.

  • Adds filtering logic to split static web assets with IfDifferent copy semantics into dedicated item groups
  • Creates new copy targets that use SkipUnchangedFiles="true" to implement the IfDifferent behavior
  • Updates target dependencies to ensure the new IfDifferent targets are executed during build and publish

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/StaticWebAssetsSdk/Targets/Microsoft.NET.Sdk.StaticWebAssets.targets Adds _BuildStaticWebAssetsIfDifferent item group and _BuildCopyStaticWebAssetsIfDifferent target for build scenarios, updates WriteStaticWebAssetsUpToDateCheck dependencies
src/StaticWebAssetsSdk/Targets/Microsoft.NET.Sdk.StaticWebAssets.Publish.targets Adds _PublishStaticWebAssetsIfDifferent item group and _PublishCopyStaticWebAssetsIfDifferent target for publish scenarios, adds condition guards to existing PreserveNewest and Always targets

@baronfel
Copy link
Member

baronfel commented Nov 20, 2025

I suspect we'll also need to extend the copies done as part of CopyFilesToPublishDirectory:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

Lines 267 to 271 in b64c645

<Target Name="CopyFilesToPublishDirectory"
DependsOnTargets="_IncrementalCleanPublishDirectory;
_CopyResolvedFilesToPublishPreserveNewest;
_CopyResolvedFilesToPublishAlways;
_HandleFileConflictsForPublish" />

The build-time Copy usage was updated in the MSBuild Targets, but Publish is more of an SDK concept.

@baronfel
Copy link
Member

baronfel commented Nov 20, 2025

We'll likely want to schedule a follow-up item to find usages of CopyToOutputDirectory or CopyToPublishDirectory metadata where PreserveNewest is set and evaluate using IfDifferent there instead - this would prevent content-based copies from occurring, though I suspect that IfDifferent doesn't play well with MSBuild Item Incrementality on Input/Output declarations. pinging @rainersigwald / @YuliiaKovalova for thoughts on that, we might need diff-aware Item incrementality before we can really take that step.

@JanKrivanek JanKrivanek mentioned this pull request Nov 20, 2025
Open
2 tasks
@baronfel
Copy link
Member

baronfel commented Nov 20, 2025

@JanKrivanek I think this point from your linked issue

Revisit all instances of logic that splits items based on CopyToOutputDirectory/CopyToPublishDirectory/Copy... metadata values - to account for the 'IfDifferent' value

needs to happen with this PR - otherwise all normal publishes are broken (even if staticwebassets works). The audit of the rest of the SDK I agree is a separate unit of work, but the CopyToOutputDirectory.. Targets needs to happen here.

@JanKrivanek JanKrivanek requested review from a team as code owners November 21, 2025 14:37
@JanKrivanek
Copy link
Member Author

JanKrivanek commented Nov 21, 2025

@JanKrivanek I think this point from your linked issue

Revisit all instances of logic that splits items based on CopyToOutputDirectory/CopyToPublishDirectory/Copy... metadata values - to account for the 'IfDifferent' value

needs to happen with this PR - otherwise all normal publishes are broken (even if staticwebassets works). The audit of the rest of the SDK I agree is a separate unit of work, but the CopyToOutputDirectory.. Targets needs to happen here.

Fair point.
Added the support in all the publishing targets that batch by that metadata (basically I string searched where in the whole repo the Always or PreserveNewest metadata is being queried and updated those. I have not queried nor updated locations where those are being used - that's that separate work and safer to be made separately)

@JanKrivanek
Copy link
Member Author

JanKrivanek commented Nov 21, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 21, 2025

Commenter does not have sufficient privileges for PR 51826 in repo dotnet/sdk

baronfel
baronfel approved these changes Nov 21, 2025
View reviewed changes
Copy link
Member

@baronfel baronfel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Excellent - these are the publish-related things I was worried about. I'd love to get these backported to 10.0.2xx once this merges!

@build-analysis build-analysis bot mentioned this pull request Nov 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@baronfel baronfel baronfel approved these changes

Assignees

No one assigned

Labels

Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JanKrivanek @baronfel