[11.0.1xx] File-based apps: improve legacy artifact support - #56248
[11.0.1xx] File-based apps: improve legacy artifact support#56248jjonescz wants to merge 1 commit into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
Critical compilation and legacy-fallback issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Backports file-based app artifact compatibility for older SDK hosts.
Changes:
- Adds an SDK capability marker.
- Implements legacy artifact-path fallback logic.
- Adds compatibility regression tests.
File summaries
| File | Summary and review findings |
|---|---|
test/dotnet.Tests/CommandTests/Run/RunFileTests_CscOnlyAndApi.cs |
Adds compatibility tests. Critical (1 vote): missing System.Security import for SecurityElement. |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DefaultArtifactsPath.props |
Adds the artifact support marker. Nit (1 vote): tests override the marker instead of verifying its assignment. |
src/Cli/Microsoft.DotNet.FileBasedPrograms/VirtualProjectBuilder.cs |
Adds artifact support detection and fallback. Critical (3 votes): _useLegacyArtifactsPath is undeclared. Critical (3 votes): fallback reuses cached modern project text instead of generating legacy properties. |
Review details
Suppressed comments (1)
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DefaultArtifactsPath.props:20
- Both cases in the new test force
_SupportsFileBasedAppArtifactsPaththroughadditionalGlobalProperties; because global properties override project properties, the test never exercises this new assignment. Removing this line would still leave that test green while the current SDK is classified as legacy in real use. Add a case that evaluates without overriding the marker and verifies support is detected.
<_SupportsFileBasedAppArtifactsPath>true</_SupportsFileBasedAppArtifactsPath>
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
| if (_useLegacyArtifactsPath is null) | ||
| { | ||
| var supportsFileBasedAppArtifactsPath = await project.GetPropertyValueAsync("_SupportsFileBasedAppArtifactsPath").ConfigureAwait(false); | ||
|
|
||
| _useLegacyArtifactsPath = !string.Equals(supportsFileBasedAppArtifactsPath, bool.TrueString, StringComparison.OrdinalIgnoreCase); |
| if (_useLegacyArtifactsPath == true) | ||
| { | ||
| return await CreateProjectInstanceNoEvaluation(projectCollection, directives, additionalGlobalProperties).ConfigureAwait(false); |
| { | ||
| xml.Should() | ||
| .Contain("<IncludeProjectNameInArtifactsPaths>false</IncludeProjectNameInArtifactsPaths>") | ||
| .And.Contain($"<ArtifactsPath>{SecurityElement.Escape(artifactsPath)}</ArtifactsPath>") |
This comment has been minimized.
This comment has been minimized.
🔍 Build Failure AnalysisSummary — The build fails to compile across every leg (Linux/macOS/Windows, AOT and non-AOT) with Root cause: incomplete backport of #56160 — missing
|
| Code | Project | File:Line | Message |
|---|---|---|---|
CS0103 |
Microsoft.DotNet.ProjectTools / Microsoft.DotNet.FileBasedPrograms.Package |
VirtualProjectBuilder.cs:505 |
The name _useLegacyArtifactsPath does not exist in the current context |
CS0103 |
Microsoft.DotNet.ProjectTools / Microsoft.DotNet.FileBasedPrograms.Package |
VirtualProjectBuilder.cs:509 |
The name _useLegacyArtifactsPath does not exist in the current context |
CS0103 |
Microsoft.DotNet.ProjectTools / Microsoft.DotNet.FileBasedPrograms.Package |
VirtualProjectBuilder.cs:511 |
The name _useLegacyArtifactsPath does not exist in the current context |
🤖 Generated by the Build Failure Analysis workflow using (a href="(dev.azure.com/redacted) · commit 904a90e
🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · auto · 167 AIC · ⌖ 2.78 AIC · ⊞ 7.2K · [◷]( · ◷)
Backport of #56160.