[release/13.5] Ensure Aspire CLI bundle resolution - #19364
[release/13.5] Ensure Aspire CLI bundle resolution#19364Jose Perez Rodriguez (joperezr) merged 10 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19364Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19364" |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR strengthens Aspire CLI bundle resolution across MSBuild and the CLI by requiring a valid DCP executable in bundle layouts and adding an automated “setup” fallback path when a usable bundle isn’t present.
Changes:
- Added an MSBuild task (
RunAspireCliCommand) and updated targets to runaspire setup(or DNX-based setup) to prepare the CLI bundle during real builds, while skipping design-time builds. - Tightened bundle/layout validation to require the DCP executable (not just the directory) across CLI discovery and bundle service verification.
- Updated acquisition scripts to plan/execute bundle setup after install, and expanded tests to cover new resolution/setup behaviors and error reporting.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/MSBuildTests.cs | Adds coverage for MSBuild inspection JSON behavior and new bundle setup flows. |
| tests/Aspire.Hosting.Tests/Dcp/DcpCliArgsTests.cs | Updates validation tests to reflect run vs publish mode requirements. |
| tests/Aspire.Hosting.Sdk.Tests/Aspire.Hosting.Sdk.Tests.csproj | Exposes net472 task assembly path for full-framework MSBuild tests. |
| tests/Aspire.Hosting.Sdk.Tests/AppHostSdkTargetsTests.cs | Adds extensive tests for bundle resolution/setup, DNX selection, escaping, and timeout/process-tree behavior. |
| tests/Aspire.Hosting.Sdk.Tests.FakeCommand/Program.cs | Extends fake command host to support hang + setup bundle layout creation. |
| tests/Aspire.Cli.Tests/Projects/DotNetAppHostProjectTests.cs | Validates when CLI bundle is/waswo isn’t acquired/injected; ensures env-path precedence/overrides behave correctly. |
| tests/Aspire.Cli.Tests/Projects/AppHostInfoResolverTests.cs | Tightens assertions around MSBuild target/property probing shape. |
| tests/Aspire.Cli.Tests/Layout/LayoutDiscoveryReparsePointTests.cs | Adds DCP executable to layout fixtures and tests rejecting layouts missing it. |
| tests/Aspire.Cli.Tests/Caching/AppHostInfoDiskCacheTests.cs | Verifies disk cache payload stays limited to inspection metadata via JSON inspection. |
| tests/Aspire.Cli.Tests/BundleServiceTests.cs | Updates validity requirements to include DCP executable. |
| tests/Aspire.Cli.Tests/BundleServiceIntegrationTests.cs | Adds extraction verification for missing DCP executable in payload and updates fake payload builder. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptShellTests.cs | Ensures dry-run plans include bundle setup and occur in the correct order. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPowerShellTests.cs | Ensures WhatIf plans include bundle setup and occur in the correct order. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPSFunctionTests.cs | Adds function-level tests for PowerShell bundle setup behavior and failure propagation. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptFunctionTests.cs | Adds function-level tests for bash bundle setup behavior and failure propagation. |
| src/Aspire.Hosting/Dcp/DcpOptions.cs | Makes DCP/dashboard path validation conditional on run mode via execution context. |
| src/Aspire.Hosting.Tasks/RunAspireCliCommand.cs | Introduces a bounded-execution task with structured args and shim-safe invocation. |
| src/Aspire.Hosting.Tasks/ResolveAspireCliInvocation.cs | Adds itemized DNX host argument outputs to avoid cmd.exe re-parsing issues. |
| src/Aspire.Hosting.Tasks/ResolveAspireCliBundle.cs | Adds toggles for PATH search/warnings and outputs default Aspire home directory used for resolution. |
| src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets | Integrates bundle setup fallback (native and DNX), skips design-time, and improves ASPIRE009 diagnostics. |
| src/Aspire.Cli/Projects/DotNetAppHostProject.cs | Avoids bundle acquisition during validation/inspection; refines env injection to preserve/replace based on usability. |
| src/Aspire.Cli/Projects/AppHostInfoResolver.cs | Comment updates aligning with new inspection behavior expectations. |
| src/Aspire.Cli/Layout/LayoutDiscovery.cs | Requires DCP executable for bundle/legacy layouts and improves logging message. |
| src/Aspire.Cli/Bundles/BundleService.cs | Tightens versioned layout validity to require DCP executable presence. |
| eng/scripts/get-aspire-cli.sh | Runs/plans aspire setup post-install (host-architecture gated), including dry-run output. |
| eng/scripts/get-aspire-cli.ps1 | Runs/plans setup post-install (host-architecture gated) with ShouldProcess support. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
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. |
|
I have a fix for the build break in the works... UPDATE: should be fixed |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs: Generated file
Suppressed comments (2)
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:275
- Interpolating the executable path inside a single-quoted MSBuild condition makes valid Unix paths containing an apostrophe (for example
/home/o'connor/.aspire/bin/aspire) produce a condition parse error before setup runs. The same path is also interpolated into later conditions. Avoid putting path values directly in condition expressions; derive boolean availability/sentinel properties in the resolver task and condition on those instead.
<_AspireCliSetupExecutable Condition="'$(AspireCliBundlePath)' == '' and Exists('$(_AspireCliSetupPath)')">$(_AspireCliSetupPath)</_AspireCliSetupExecutable>
<_AspireCliSetupExecutable Condition="'$(AspireCliBundlePath)' == '' and '$(AspireCliPath)' == '' and '$(_AspireResolvedCliInvocationMode)' == 'Dnx' and Exists('$(_AspireResolvedDnxHostPath)')">$(_AspireResolvedDnxHostPath)</_AspireCliSetupExecutable>
src/Aspire.Hosting.AppHost/build/Aspire.Hosting.AppHost.in.targets:243
- This condition directly single-quotes
AspireCliPath, so an explicitly configured executable path containing an apostrophe is parsed as malformed MSBuild syntax instead of being validated. Use a quote-safe boolean output from the resolver (or consistently escape the value before condition evaluation) rather than embedding the raw path in the expression.
This issue also appears on line 274 of the same file.
<_AspireCliBundleWarnOnInvalidPaths Condition="'$(AspireCliBundlePath)' != '' or ('$(AspireCliPath)' != '' and !Exists('$(AspireCliPath)'))">true</_AspireCliBundleWarnOnInvalidPaths>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 52 / 100 test projects · 7 jobs, from 27 changed files. Selected test projects (52 / 100)
Selected jobs (7)
How these were chosen — grouped by what changed
🔧 show 41
🔧 📄 📄 📦 affected project 📦 affected project 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Jose Perez Rodriguez (joperezr)
left a comment
Jose Perez Rodriguez (joperezr)
left a comment
There was a problem hiding this comment.
Important fix for new templates as the bundle is the default there. Approved.
26d2c9a
into
release/13.5
|
✅ No documentation update needed. Step 5 branch taken: This PR is a backport (Backport of #19251 to release/13.5) with a [release/13.5] title prefix and a backport head branch, targeting the release/13.5 base. Per workflow policy, backport PRs are out of scope for docs generation because the user-facing documentation is authored against the original forward PR (#19251) on the default branch; drafting a second docs PR here would be duplicate noise. Triggered signals (1): new_public_type (evidence: src/Aspire.Hosting.Tasks/RunAspireCliCommand.cs adds No docs PR was drafted. If documentation is needed for the underlying change, it should be tracked against the original PR #19251. |
Backport of #19251 to release/13.5
/cc Karol Zadora-Przylecki (@karolz-ms)
Customer Impact
Testing
Risk
Regression?