Skip to content

Conversation

baronfel
Copy link
Member

Fixes #49582

The SDK knows that it's opting into Artifacts Path, and so the SDK can coordinate correcting the cleanup behavior for ArtifactsPath.

@Copilot Copilot AI review requested due to automatic review settings June 28, 2025 19:01
Copy link
Contributor

@Copilot 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 updates the SDK to properly register files within artifacts folders for cleanup and provides accompanying tests.

  • Adds a test in ArtifactsOutputPathTests.cs to verify that published files (e.g., hostfxr.dll) are correctly cleaned up when building in Release configuration.
  • Updates the Microsoft.NET.DefaultOutputPaths.targets to include a new target _TrackFileWritesShareableUnderArtifactsPath ensuring file writes under both OutputPath and IntermediateOutputPath are tracked for cleanup.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/Microsoft.NET.Build.Tests/ArtifactsOutputPathTests.cs Adds tests to verify cleanup behavior for artifacts folders
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DefaultOutputPaths.targets Enhances cleanup logic by tracking file writes under artifacts output directories

@dsplaisted
Copy link
Member

Instead of duplicating some MSBuild logic, would it make sense to update MSBuild so that (with an opt-in from the SDK) it would also look under the output and intermediate output paths in addition to the project directory?

@baronfel
Copy link
Member Author

That's also very reasonable, and much more clean. I can set that up for sure.

@baronfel
Copy link
Member Author

When dotnet/msbuild#12096 is merged, I'll update this to just set the new property (and keep the existing tests).

@baronfel baronfel marked this pull request as draft July 1, 2025 20:23
@baronfel
Copy link
Member Author

The change has flowed to the VMR this morning, so the next VMR->SDK flow should let us merge this.

@baronfel baronfel force-pushed the 49582-fix-artifacts-paths-cleanups branch from a2c6fe6 to d266bc4 Compare July 17, 2025 23:07
@baronfel
Copy link
Member Author

Once #49740 merges this can be rebased and the tests should light up.

@baronfel baronfel marked this pull request as ready for review July 25, 2025 15:16
@baronfel baronfel force-pushed the 49582-fix-artifacts-paths-cleanups branch from d266bc4 to 6f71ec0 Compare July 25, 2025 15:16
@baronfel
Copy link
Member Author

The containers test failures are because Red Hat's registries are undergoing emergency maintenance - nothing actually wrong with the SDK.

image

@baronfel baronfel force-pushed the 49582-fix-artifacts-paths-cleanups branch from 6f71ec0 to d9f4a44 Compare July 25, 2025 17:15
@baronfel baronfel enabled auto-merge July 25, 2025 18:16
@baronfel baronfel merged commit d7acdf1 into dotnet:main Jul 25, 2025
27 checks passed
@baronfel baronfel deleted the 49582-fix-artifacts-paths-cleanups branch July 28, 2025 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"dotnet build" fails to produce a runnable binary if used after a self-contained publish with ArtifactsPath
3 participants