Place file-based app artifacts into repo's artifacts dir if used #49863
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements functionality to place file-based app artifacts into the repository's artifacts directory when the repo uses artifacts output layout, contributing to fixing issue #49826. The change allows file-based apps to respect existing repository artifact configurations rather than always using a global temp directory.
Key changes:
- Introduces
FileBasedAppArtifactsPathproperty to control artifact placement for file-based apps - Updates build logic to detect and use repository artifacts layout when available
- Adds comprehensive test coverage for the new artifact placement behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| VirtualProjectBuildingCommand.cs | Replaces direct ArtifactsPath setting with FileBasedAppArtifactsPath property |
| Microsoft.NET.DefaultArtifactsPath.props | Adds logic to use FileBasedAppArtifactsPath when set, enabling artifacts output |
| UseArtifactsOutputPath.props | Updates condition to import artifacts path configuration when FileBasedAppArtifactsPath is set |
| RunFileTests.cs | Adds test cases for artifact placement scenarios and refactors existing embedded resource test |
| dotnet-run-file.md | Updates documentation to describe new artifact placement behavior |
Comments suppressed due to low confidence (1)
test/dotnet.Tests/CommandTests/Run/RunFileTests.cs:1022
- The test uses the
-blflag (binary log) but doesn't verify the binary log output or explain why it's needed for this specific test case about embedded resources in repo artifacts.
new DotnetCommand(Log, "run", "Program.cs", "-bl")
| If [artifacts output layout][artifacts-output] is enabled, build outputs of the file-based app are placed there | ||
| (except caching markers which are placed in the global temp directory described next). |
There was a problem hiding this comment.
I admit I'm not understanding the motivation of this change here. Do we expect users to then be looking at these outputs, or is this just to leverage the existing things that exclude artifact outputs?
There was a problem hiding this comment.
Leveraging existing default item excludes is one motivation. Another is that all repo artifacts are in one place - so the same motivations should apply as to why artifacts layout exists in the first place, e.g., you can clean all repo artifacts together.
There was a problem hiding this comment.
Will the artifacts at least be placed under a runfile subfolder or some such? Otherwise I worry a bit that the artifacts directory will be polluted.
In particular, if a file is not part of a loaded project (e.g. it was not loaded yet or it was deliberately unloaded), the IDE currently will just treat it as a file-based program in many cases, it will do a design time build on it in order to get basic set of references and so on. It could be confusing if the artifacts folder contains many such directories simply due to the way the user was browsing.
There was a problem hiding this comment.
In particular, if a file is not part of a loaded project (e.g. it was not loaded yet or it was deliberately unloaded), the IDE currently will just treat it as a file-based program in many cases, it will do a design time build on it in order to get basic set of references and so on.
Hm, that's unfortunate, I haven't realized that.
But I've also noticed that artifacts layout currently seems to assume that projects have unique names. So each project gets a folder like artifacts/[ProjectName]. Script's "project name" is the script's file name. That can lead to two scripts sharing the same output location (even more likely given the IDE behavior you described). So I guess we need to create some unique names or directories for scripts.
Contributes to fixing the issue described in #49826.