Repository navigation
Project v2 build - #19678
Project v2 build#19678
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19678Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19678" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds a run-only build coordinator that builds .NET project resources together before startup.
Changes:
- Generates and builds a content-addressed
.slnx. - Launches projects with
--no-buildand gates mixed file-based apps. - Adds tests, documentation, and simplified playground examples.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
Directory.Packages.props |
Adds SolutionPersistence version. |
src/Aspire.Hosting.Dotnet/Aspire.Hosting.Dotnet.csproj |
References SolutionPersistence. |
src/Aspire.Hosting.Dotnet/DotnetProjectBuildCoordinator.cs |
Coordinates build lifecycle. |
src/Aspire.Hosting.Dotnet/DotnetProjectBuildResource.cs |
Generates and builds the solution. |
src/Aspire.Hosting.Dotnet/DotnetProjectHostingExtensions.cs |
Wires build coordination into projects. |
src/Aspire.Hosting.Dotnet/DotnetProjectMetadata.cs |
Tracks build suppression. |
src/Aspire.Hosting.Dotnet/README.md |
Documents coordinated builds. |
tests/Aspire.Hosting.Dotnet.Tests/DotnetProjectBuildCoordinatorTests.cs |
Tests coordination and failures. |
tests/Aspire.Hosting.Dotnet.Tests/DotnetProjectResourceTests.cs |
Updates launch-argument expectations. |
tests/Aspire.Hosting.Dotnet.Tests/Snapshots/DotnetProjectBuildCoordinatorTests.GeneratedSolutionContainsOnlyUniqueProjectsInModelOrder.verified.slnx |
Verifies generated solution content. |
playground/DotnetProject/DotnetProject.AppHost/AppHost.cs |
Removes manual build ordering. |
playground/DotnetProject/TypeScriptAppHost/apphost.mts |
Removes TypeScript wait chains. |
docs/plans/project-v2-csharpprogram-watch.md |
Records completed build design. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Code review — 8 findingsReviewed at head
1. New E2E spec has no CI shard — the shard-matrix guard test will fail and the spec never runs
Fix: add a matrix row with 2.
|
This comment has been minimized.
This comment has been minimized.
|
Replying to the eight findings in #19678 (comment). Changes are in 1. New E2E spec has no CI shardFixed. Added the blocking Linux 2.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 51 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
src/Aspire.Hosting/ApplicationModel/ExecutableLaunchConfiguration.cs:134
BuildConfigurationis a new public member onProjectLaunchConfiguration(andSuppressBuildbelow is another), but the PR checklist says no public API was added. Update the PR metadata and route these additions through the required API review before merging.
[JsonPropertyName("build_configuration")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public string? BuildConfiguration { get; set; }
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Code review of the coordinated Project v2 build. 4 issues found:
- Correctness / behavior change (2): the coordinated
.slnxbuild injects MSBuild solution global properties that individual project builds never set;suppress_buildis now emitted for every classicAddProjectresource, which removes the VS Code extension's recovery build for the existing v1 user base. - Performance (1):
PathNormalizer.ResolveToFilesystemPathregressed from a filtered, Windows-only lookup to a full directory enumeration per path segment on all platforms. - Test reliability (1): two new tests depend on POSIX permission enforcement and fail when the test process runs as root (anchored on both affected files).
Already-resolved threads (IDE suppress-build, case-sensitivity dedup, symlink dedup, public API checklist) were excluded.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2ab6dd5a-828a-4f5a-8ab8-3adfa778ae61
|
Replying to the suppressed finding in Copilot review 5117291723:
Verdict: valid. Fixed in 7baff9f. The |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🔵 Needs a closer look
The large cross-component change introduces build orchestration, IDE integration, and filesystem concurrency behavior that warrants final human review.
Review details
Files not reviewed (1)
- src/Aspire.Cli/Resources/TemplatingStrings.Designer.cs: Generated file
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Aspire.Hosting.Dotnet/DotnetProjectHostingExtensions.cs:338
- The PR description says coordinated file apps continue with
dotnet run --file ... --no-cache, but this path launches them with--no-build; the new README and coordinator test also assert--no-build. Please update the PR description to match the implemented behavior, since suppressing this second build is essential to the race fix.
- Files reviewed: 97/99 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1ad11430-337c-4c2e-81d8-8ae1e8bf6faf
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
The claimed atomic, symlink-safe .gitignore update remains a direct symlink-following write.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 68/69 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
VS Code incorrectly rejects valid custom external run commands when their project has no TargetPath artifact.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
extension/src/debugger/languages/dotnet.ts:1335
- External-build launches reject a missing
TargetPathbefore honoring the resolvedRunCommand. A valid project can setRunCommandto an unrelated launcher and intentionally produce no target artifact; the DCP path launches that command directly, but F5 fails here with “prebuilt output missing.” Only requireTargetPath/the apphost when the resolved command actually uses the project output (for example, whencanDebugResolvedProjectRunCommandreturns true); otherwise proceed with the custom command.
- Files reviewed: 67/68 changed files
- Comments generated: 0 new
- Review effort level: Balanced
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedChange Categories
The current head implements coordinated builds for path-based .NET project resources, The PR description is stale for Test Scenarios ExecutedScenario 1: PR artifact identityObjective: Verify that all product scenarios use the current PR artifact. Coverage Type: Artifact validation Status: Passed Evidence:
Observations:
Scenario 2: Shared project graph coordinated buildObjective: Verify that two services sharing a project reference use one coordinated build, Coverage Type: Happy path Status: Passed Steps:
Evidence:
Observations:
Scenario 3: Build-environment isolation and direct launchObjective: Verify that Coverage Type: Happy path and boundary Status: Passed Steps:
Evidence:
Observations:
Scenario 4: Mixed project and file-based appObjective: Verify that a file-based Coverage Type: Happy path and boundary Status: Passed Evidence:
Observations:
Scenario 5: Coordinated-build failure safetyObjective: Verify that a deterministic compile failure retains build diagnostics and blocks Coverage Type: Unhappy path Status: Passed Expected Outcome: The build fails safely, both project resources remain failed, logs identify Evidence:
Observations:
Scenario 6:
|
| Job | Result | Classification |
|---|---|---|
Aspire.Cli.Tests Windows |
Passed | Initial PR-caused assertion failure resolved by 95207f804a |
Aspire.Cli.Tests Linux |
Passed | Initial PR-caused assertion failure resolved by 95207f804a |
Aspire.Cli.Tests macOS |
Passed | Initial PR-caused assertion failure resolved by 95207f804a |
VS Code E2E Windows apphost-lifecycle-tools |
Passed | Initial timing failure did not reproduce |
Aspire.Hosting.Azure.Kubernetes.Tests Windows |
Passed | Initial timeout did not reproduce |
| VS Code extension unit tests Windows | Passed | Local cache-test failures were environment-specific |
VS Code E2E Windows dynamic-debug-configuration |
Failed | New unrelated teardown-only file-lock failure |
VS Code lifecycle failure evidence
- Current run passed 7/8 tests.
- The only failure timed out after 10 seconds waiting for
.e2e-cli-wrappers\run-aspire-cli-timeout\timeout-child.pid. - The same shard passed 8/8 in prior run
33916911179at PR head7baff9fa. - The failed test and its
runAspireClipath did not change between7baff9faand70c89ce5. - The PR's new Linux and Windows coordinated-build shards both passed.
- The updated run passed the Windows lifecycle shard:
https://github.com/microsoft/aspire/actions/runs/33931645917/job/101215264080.
Kubernetes failure evidence
- The only failed test was
DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent. - It failed with
System.TimeoutException. - The same Windows job passed in prior run
33916911179. - No Azure Kubernetes or shared pipeline file changed between PR heads
7baff9faand70c89ce5. - The updated run passed the Windows Kubernetes job:
https://github.com/microsoft/aspire/actions/runs/33931645917/job/101213710250.
New dynamic-debug teardown failure
- The actual test
re-resolves the selected AppHost by dynamic configuration name in duplicate-alias workspaces
passed. - Its
after eachcleanup failed withEBUSYwhile removing
.e2e-dynamic-debug\second. - The same Windows shard passed in both earlier PR runs
33916911179and33928693484. - The only change in
95207f804awas the CLI unit-test correction, so this cleanup lock is unrelated
to the PR fix.
Summary
| Scenario | Status | Notes |
|---|---|---|
| Artifact identity | Passed | Refreshed PR CLI matched 95207f804a |
| Shared coordinated build | Passed | Shared dependency built once |
| Build environment and launch output | Passed | Build/runtime values remained isolated |
| Mixed project/file app | Passed | Builds serialized; file app used --no-build |
| Build failure safety | Passed | Logs retained; forced start blocked |
| Init behavior | Passed with stale-description caveat | Current head preserves existing ignore content |
| Extension build and coordinated E2E | Passed | Focused Extension Host run passed 3/3 |
| CI workflow matrix | Passed | 68/68 contract tests; both new CI shards passed |
| Failed-scenario recheck | Passed | Every previously failed job passed |
| Full PR CI | Failed | New unrelated dynamic-debug teardown lock |
Overall Result
ISSUES REMAIN
The coordinated-build product behavior and the new extension E2E coverage worked as intended.
The deterministic CLI test failure was corrected, and every previously failed scenario passed on
the updated head. The latest CI run is red only because a different dynamic-debug E2E test passed
its assertion and then hit a Windows EBUSY cleanup lock.
A separate valid review concern remains: the PR description claims atomic and symlink-safe
.gitignore behavior that is not present at final head. Testing confirms the current templates and
initialization paths do not add .aspire/.
Recommendations
- Resolve the remaining
.gitignorereview concern or update the PR description and intended
behavior explicitly. - Retry the Windows
dynamic-debug-configurationshard; its functional test passed and only temp
directory cleanup failed. - No additional coordinated-build product correction is indicated by these test results.
Artifact Locations
- Test root:
C:\Users\karolz\AppData\Local\Temp\aspire-pr-test-19678-8f7546b4 - Report:
C:\Users\karolz\AppData\Local\Temp\aspire-pr-test-19678-8f7546b4\pr-19678-testing-report.md - Recheck logs:
recheck-cli-test.log,recheck-extension-unit.log,
recheck-install.log, andrecheck-version.txt - Downloaded CI diagnostics:
C:\Users\karolz\AppData\Local\Temp\aspire-pr-test-19678-8f7546b4\ci-failures
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector53 / 99 PR test projects · 5 PR jobs · 2 advisory-only targets, from 66 changed files. Selected PR test projects (53 / 99)
Selected PR jobs (5)
Advisory workflow impact (2)
How these were chosen — grouped by what changed
🧪 show 31
🔧 🔧 🧪 📦 affected project 📦 affected project 📄 📄 📄 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
🔵 Needs a closer look
The broad lifecycle, cross-process artifact, build, and IDE-launch changes warrant final human validation despite extensive coverage.
Review details
- Files reviewed: 67/68 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Description
DotnetProjectResourceservices currently perform separate implicit builds when they start. When several services reference the same projects, those builds can write to sharedbinandobjdirectories concurrently, causing nondeterministic failures. This change gives Project v2 one coordinated initial build phase so every service starts from a consistent set of outputs.Coordinated build
Compatible
.csprojresources are collected into generated MSBuild traversal projects under the AppHost's intermediate output (Aspire:Store:Path/.aspire/build). Projects within a traversal build can run in parallel and MSBuild deduplicates shared references; build groups run serially because different SDK selections, configurations, or build environments can still target overlapping outputs.Resources using
WithBuildEnvironment(...)receive serialized direct builds. This keeps project-specific MSBuild inputs isolated while allowing runtime-only values—including service-discovery and connection-string variables fromWithReference(...)—to remain outside the build grouping decision.File-based
.csapplications cannot participate in traversal projects and continue usingdotnet run --file ... --no-cache. In mixed applications, they wait for the coordinated build because#:projectreferences may share outputs with project-based resources.Starting the built output
After the coordinated build, Aspire resolves each project's
RunCommand,RunArguments, and working directory from the same build context and launches that output directly. This avoids a laterdotnet runevaluation using runtime environment variables, selecting a different output, or starting another build.Missing project files remain on the ordinary
dotnet run --project <path>path so the resulting SDK error is associated with the affected resource rather than failing the shared build. Coordinated-build failures retain their logs and block both normal and forced resource starts.IDE and F5 launches
The substantial changes in
extension/src/debugger/languages/dotnet.tsteach the VS Code extension about outputs produced by an external coordinated build. The extension now honors the AppHost configuration, build environment, SDK-discovery working directory, and build-suppression state instead of independently selecting or rebuilding the default Debug output and recreating the original race.The extension also resolves
RunCommandandRunArgumentsin that same context. For file-based applications, Configuration is supplied as a global MSBuild property so an in-source#:property Configuration=...directive cannot redirect the IDE to a different output.The Hosting launch metadata and pipeline APIs are generalized so
Aspire.Hosting.Dotnetcan carry this contract to IDEs without depending on productionInternalsVisibleToaccess or duplicating core launch behavior.Generated artifact safety
Traversal projects need stable paths under the AppHost's intermediate output and can be shared by concurrent AppHost processes. They are therefore content-addressed, published atomically, protected by held-file leases, and removed only after they are no longer in use. Filesystem path canonicalization prevents casing and symlink aliases from creating conflicting build identities.
Because these generated files live under
Aspire:Store:Path/.aspire/buildinstead of the source tree, project templates andaspire initdo not need to modify.gitignore.The shared held-file lease primitive is also used by bundle-version cleanup, allowing both cleanup paths to conservatively retain data when lease state cannot be determined.
User-facing usage
C# AppHost
TypeScript AppHost
WithBuildEnvironment(...)is intended for values that affect MSBuild evaluation. These values are not automatically added to the launched process; useWithEnvironment(...)separately when the same value is also required at runtime. Build environments should not contain secrets because their values may appear in build diagnostics.The playground no longer needs manual
WaitFor(...)chains to serialize service builds.Validation
./build.sh --build --pack /p:SkipNativeBuild=trueAspire.Hosting.Dotnet.Tests: 96 passedPathNormalizerTests: 7 passedChecklist
<remarks />and<code />elements on your triple slash comments?Fixes
#19037