Forward-port #17528 + #17564 + #17573 from release/13.4 to main + DotNet template identity-channel fix#17597
Closed
mitchdenny wants to merge 4 commits into
Closed
Forward-port #17528 + #17564 + #17573 from release/13.4 to main + DotNet template identity-channel fix#17597mitchdenny wants to merge 4 commits into
mitchdenny wants to merge 4 commits into
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17597Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17597" |
Contributor
There was a problem hiding this comment.
Pull request overview
Forward-ports two Aspire CLI fixes from release/13.4 to main: (1) ensuring aspire new prefers templates matching the running CLI/SDK version for explicit prerelease channels, and (2) stabilizing staging-channel NuGet cache paths so temporary nuget.config cleanup doesn’t delete packages that are still referenced by the integration probe manifest.
Changes:
- Update
aspire newtemplate-version resolution to pin to the current CLI/SDK version for explicit prerelease channels (daily/staging), with corresponding channel-resolution test coverage. - Stabilize staging
globalPackagesFolderto an absolute, persistent per-feed cache under<ASPIRE_HOME>/.nugetpackages/<feed-hash>; add cache-clearing support and tests. - Extend supporting utilities (
VersionHelper,CliPathHelper,NuGetConfigMerger,TemporaryNuGetConfig) and add/adjust unit tests to lock in the new invariants.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/VersionHelperTests.cs | Adds coverage for “current CLI version match” behavior when a channel is selected vs. no channel/hives. |
| tests/Aspire.Cli.Tests/Utils/CliPathHelperTests.cs | Adds tests for staging feed cache-key hashing (normalization, determinism, length handling). |
| tests/Aspire.Cli.Tests/Projects/PrebuiltAppHostServerTests.cs | Asserts staging globalPackagesFolder is absolute, outside temp config dir, and keyed by feed-hash. |
| tests/Aspire.Cli.Tests/Packaging/TemporaryNuGetConfigTests.cs | Verifies explicit globalPackagesFolder override propagation and ignore-when-disabled behavior. |
| tests/Aspire.Cli.Tests/Commands/NewCommandTests.cs | Removes hard-coded SDK version expectation; aligns with VersionHelper.GetDefaultSdkVersion(). |
| tests/Aspire.Cli.Tests/Commands/NewCommandChannelResolutionTests.cs | Expands coverage for identity-channel resolution and explicit prerelease/stable channel pinning behavior. |
| tests/Aspire.Cli.Tests/Commands/CacheCommandTests.cs | Adds tests ensuring cache clear removes staging .nugetpackages subdirectories and handles missing roots. |
| src/Aspire.Cli/Utils/VersionHelper.cs | Broadens “exact current CLI match” eligibility to cases where a channel is selected. |
| src/Aspire.Cli/Utils/CliPathHelper.cs | Introduces staging cache root path helper and feed-url hash key computation. |
| src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs | Writes a persistent absolute globalPackagesFolder for staging/source-override temp configs; selects primary feed URL for keying. |
| src/Aspire.Cli/Packaging/TemporaryNuGetConfig.cs | Adds optional globalPackagesFolderValue passthrough to config generation. |
| src/Aspire.Cli/Packaging/PackageChannel.cs | Preserves current CLI/SDK version in prerelease channel searches to support pinning behavior. |
| src/Aspire.Cli/Packaging/NuGetConfigMerger.cs | Adds default .nugetpackages constant and allows callers to supply an explicit globalPackagesFolder value. |
| src/Aspire.Cli/Commands/NewCommand.cs | Adds TryGetCurrentCliTemplateVersionPackage to synthesize/preserve CLI-matching template version for prerelease explicit channels. |
| src/Aspire.Cli/Commands/CacheCommand.cs | Extends cache clear to wipe staging NuGet cache root under ASPIRE_HOME. |
Copilot's findings
- Files reviewed: 15/15 changed files
- Comments generated: 3
Comment on lines
+43
to
+48
| // Wipe the staging NuGet package cache too. Producers (PrebuiltAppHostServer's | ||
| // temporary nuget.config for the staging channel) deposit SHA-keyed package | ||
| // caches under <ASPIRE_HOME>/.nugetpackages/<sha>; clearing them lets users | ||
| // recover wedged staging restores without filesystem surgery. We hand the parent | ||
| // directory to ClearDirectoryContents so each SHA subdirectory is wiped while | ||
| // the parent itself stays in place for the next staging restore. |
Comment on lines
+91
to
+94
| // Pins that `aspire cache clear` wipes the SHA-keyed staging NuGet package caches under | ||
| // <ASPIRE_HOME>/.nugetpackages — produced by PrebuiltAppHostServer's temporary | ||
| // nuget.config for the staging channel. Without this, a wedged staging restore can only | ||
| // be recovered by manual filesystem surgery. |
Comment on lines
+114
to
+116
| // SHA-keyed subdirectories should be gone; the parent stays so the next staging restore | ||
| // can populate a fresh cache without recreating the .nugetpackages root. | ||
| Assert.False(Directory.Exists(Path.Combine(stagingCacheRoot.FullName, "deadbeef"))); |
…rce mapping (#17528) * Bake AspireCliChannel=staging for release-branch builds even when stabilizing The channel-compute step in build_sign_native.yml was checking $versionKind -eq 'release' BEFORE the release-branch regex check. A 13.4 staging build runs from a release/* branch with StabilizePackageVersion=true, which sets DotNetFinalVersionKind=release, so the wrong arm fired and baked AspireCliChannel=stable into the binary. Downstream, aspire init reads CliExecutionContext.IdentityChannel to pick the channel mappings it writes into the workspace nuget.config. With identity=stable there's no Aspire.* → staging-feed mapping, so aspire add tries to resolve packages from nuget.org and either gets 13.3.5 or fails outright (the apphost.cs template pins #:sdk Aspire.AppHost.Sdk@13.4.0+<sha>, which isn't on nuget.org). Swap the conditions so the release-branch check runs first. Release- branch builds are always staging artifacts; only release-shaped non-release-branch builds (effectively none in practice) get stable. Fixes #17527 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add aspireCliChannelOverride pipeline parameter for GA ship builds With release-branch builds now defaulting to 'staging' (so stabilizing dogfood builds aren't mis-baked as 'stable'), there was no remaining path to produce a real 'stable' GA ship binary — the same release/* branch that produces the staging dogfood drops also produces the final ship build, and the pipeline has no other signal to tell them apart. Add a runtime pipeline parameter 'aspireCliChannelOverride' (auto | stable | staging | daily, default auto) to azure-pipelines.yml and thread it through every build_sign_native.yml invocation. When the release manager kicks off the official GA ship build, they set this to 'stable' so the distributed binary bakes AspireCliChannel=stable and aspire init writes the nuget.org-only nuget.config that matches the promoted package set. Routine stabilizing builds leave it on 'auto' and continue to bake 'staging'. The override is validated against the same accepted-channel set that IdentityChannelReader.IsValidChannel enforces at CLI startup so a typo fails the pipeline step rather than producing a binary that refuses to boot. pr-<N> is intentionally excluded from the override set since PR builds always come from the PullRequest reason arm. The unofficial pipeline doesn't get the parameter — its test builds should always derive the channel from branch+reason, and the template's default 'auto' achieves that. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Route stabilizing staging CLI to SHA-derived darc feed The staging-channel synthesis in PackagingService defaulted to PackageChannelQuality.Both for staging-identity CLIs. With Both, useSharedFeed=true and Aspire.* gets routed to the shared dnceng/dotnet9 daily feed -- which only contains prerelease-tagged 13.4.0-preview.* packages, not the stable-shaped 13.4.0 packages produced during release stabilization (StabilizePackageVersion=true). Net effect on the just-shipped staging build of 13.4: `aspire init` drops a NuGet.config pointing Aspire.* at dotnet9, then `aspire add yarp` fails to resolve Aspire.Hosting.Yarp 13.4.0 because dotnet9 doesn't carry it. The packages actually live in the SHA-derived darc-pub-microsoft-aspire-<hash> feed. Fix: when the CLI's identity is staging, derive the synthesized channel's default quality from the CLI build's version shape: - Stable-shaped (no semver prerelease tag) -> Stable, which makes useSharedFeed=false and routes Aspire.* to the SHA-derived darc feed where stabilizing packages actually live. - Prerelease-shaped -> Both (the historical default), since SHA- specific darc feeds are only created for stable release-branch builds and prerelease staging CLIs must use the shared feed. The identity-staging branch runs before the requested/configured branches in the if/else because `init` (and many other commands) calls GetChannelsAsync(requestedChannelName: "staging") when the running CLI's identity is staging -- short-circuiting on the requested branch would re-introduce the bug. The version-shape predicate is injected via constructor so unit tests can deterministically exercise both paths regardless of the test-host assembly's baked InformationalVersion. Validated end-to-end with a locally-built NAOT `aspire` binary (`/p:AspireCliChannel=staging`) + overrideStagingFeed pointing at darc-pub-microsoft-aspire-0f514452: `aspire init` -> `aspire add yarp` now resolves Aspire.Hosting.Yarp 13.4.0 instead of falling back to 13.3.5. Refs #17527 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pin PackagingService stable-shape predicate to false in CliTestHelper The stabilization-check CI job builds the test host with StabilizePackageVersion=true, which bakes a stable-shaped (no '-') informational version into Aspire.Cli. PackagingService's new identity-staging branch then defaults quality to Stable and requires a SHA suffix in the assembly's InformationalVersion to compute the darc-pub feed URL. Stabilized test-host assemblies don't carry a +sha suffix, so CreateStagingChannel returned null and the UpdateCommand_WhenStagingIdentityRegistersChannel_UsesStagingForUnpinnedProject test fell back to the default channel instead of staging. Default CliTestHelper.PackagingServiceFactory to inject isStableShapedCliVersion: () => false so command-level tests get deterministic prerelease-shaped behavior (quality=Both → shared dotnet9 feed) regardless of how the test host was built. Tests that specifically exercise the stable-shape branch (in PackagingServiceTests) construct PackagingService directly and already pass an explicit predicate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update eng/pipelines/templates/build_sign_native.yml --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
* Prefer current CLI template version Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Pin aspire new template version to CLI Ensure CLI-runtime templates selected from explicit package channels use the current bundled CLI/SDK version instead of floating to newer channel packages that can mismatch the bundled AppHost server. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Folder path (#17573) * Stabilize PrebuiltAppHostServer staging globalPackagesFolder path PackagingService creates the staging channel with ConfigureGlobalPackagesFolder=true so each darc/override feed restore lands in an isolated cache (two staging builds of the same release branch ship as 13.4.0 but from different feeds, and NuGet keys by (id,version) only). PrebuiltAppHostServer was wiring that flag into a TemporaryNuGetConfig whose default globalPackagesFolder value is the relative '.nugetpackages' -- NuGet resolved it under the temp config's own directory, BundleNuGetService baked those temp paths into integration-package-probe-manifest.json, and TemporaryNuGetConfig.Dispose then recursively deleted the cache out from under the manifest. On macOS osx-arm64 polyglot staging builds this surfaced as a hang during DI / assembly loading in aspire-managed. Preserve the per-feed cache isolation behavior and anchor the override at a stable absolute path instead: <ASPIRE_HOME>/.nugetpackages/<first-8-of-CLI-commit-sha> Keying by the truncated commit hash matches the existing darc-pub-microsoft-aspire-<hash> feed URL convention in PackagingService.GetStagingFeedUrl, so the cache key and feed key stay aligned at 8 hex chars. 8 chars is short enough to avoid Windows MAX_PATH blow-ups on deep integration cache trees while keeping SHA collisions negligible. The cache lives under ASPIRE_HOME (not the per-AppHost working directory) so multiple AppHosts on the same machine running against the same staging build can share a single restore -- the unit of isolation here is the staging build, not the individual restore command. Mechanics: - TemporaryNuGetConfig.CreateAsync now accepts an optional globalPackagesFolderValue and propagates it through AddGlobalPackagesFolderToConfigAsync into the merger. - NuGetConfigMerger.AddGlobalPackagesFolderConfiguration takes the optional override and falls back to the workspace-relative default ('.nugetpackages') for the non-temp workspace-merge path. - PrebuiltAppHostServer.ResolveStableGlobalPackagesFolder routes both temp config branches (channel and package-source-override) through the new helper. - VersionHelper.TryGetCurrentCommitHashShort surfaces the truncated SHA from the running CLI's AssemblyInformationalVersion (returns null on clean release builds with no '+sha' suffix; callers fall back to 'default'). - CliPathHelper centralizes the '<ASPIRE_HOME>/.nugetpackages' path so the producer (PrebuiltAppHostServer) and consumer (CacheCommand) can't drift. - CacheCommand.ClearCommand now wipes <ASPIRE_HOME>/.nugetpackages so a wedged staging restore is recoverable through the same UX as every other CLI cache. Tests: - Updated existing PrebuiltAppHostServerTests staging cases to assert the globalPackagesFolder value is absolute and lives outside the temp config directory. - New PrebuiltAppHostServerTests case wires a real PackagingService with overrideStagingFeed on a stable-shaped CLI and pins the same invariant end-to-end. - New TemporaryNuGetConfigTests for the override propagation and for the no-override-when-disabled invariant. - New CacheCommandTests covering the staging cache wipe and the missing-cache no-op path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Key staging globalPackagesFolder cache by feed URL hash The staging globalPackagesFolder fix in the previous commit keyed the \`.nugetpackages\` subdirectory off the CLI's own commit SHA (first 8 hex chars of \`AssemblyInformationalVersionAttribute\`). That handles two darc-shipped staging builds of the same release branch, but it breaks the local-dev case where one CLI is repeatedly retargeted at different \`overrideStagingFeed\` values: the URL changes but the SHA doesn't, so every override silently shares one cache bucket and the second restore reuses the first feed's now-stale \`13.4.0\` assemblies. Switch the cache key to the first 8 hex chars of \`XxHash3\` over the trimmed, lower-cased resolved feed URL: - Override branch passes the explicit \`--source\` URL. - Channel branch passes the channel's \`Aspire*\` mapping source (or the first mapping for forward compatibility). Trim + lower-case before hashing so a stray whitespace from a config file or a hostname-case change doesn't fragment the cache. Non-cryptographic hashing is fine here — the key is a directory name, not a security boundary — and 8 hex chars keep deep integration cache paths well under Windows MAX_PATH while giving ~4 billion buckets, so collisions are negligible across the handful of staging feeds any user ever sees. Removes the now-unused \`VersionHelper.TryGetCurrentCommitHashShort\` helper (added in the previous commit, no remaining callers). Tests: - New \`CliPathHelperTests.ComputeStagingFeedCacheKey_*\` cover determinism, normalization, length defaults, and null/empty input. - Existing \`PrebuiltAppHostServerTests\` strengthened to assert the emitted globalPackagesFolder equals \`<aspireHome>/.nugetpackages/<hash(feed)>\`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Narrow #17564 CLI-pin fallback to prerelease channels PR #17564 introduced a new fallback in TryGetCurrentCliTemplateVersionPackage that pins the template package to the running CLI's SDK version whenever the selected channel is explicit and not local-build. That preserved the intended fix for prerelease channels (daily, staging) where the channel feed filters the running CLI's package out of search results even though the feed can still restore it. But it also fired for the stable channel, where the filter does not apply. For a non-stable-shape CLI (PR build like '13.4.0-pr.X.gY' or a daily-shape preview) invoked with '--channel stable', it forced an unpublishable version into the generated apphost.cs, breaking the SmokeTests LatestCliCanStartStableChannelAppHost and LatestCliCanStartStableChannelTypeScriptAppHost. Exclude the stable channel from the new fallback so that case falls through to the OrderByDescending picker and the user gets the highest shipped stable package they explicitly asked for. The original prerelease-channel motivation ('Aspire.TypeSystem version mismatch when 13.4 CLI floats templates to a 13.5 daily preview') is preserved unchanged. Test updates: - NewCommandChannelResolutionTests.NewCommand_NoChannelArg_ResolvesTemplateFromIdentityChannel gets an expectedVersion theory parameter so the daily case still pins to the CLI version while the stable case asserts the highest shipped stable. - NewCommandChannelResolutionTests.NewCommand_ExplicitChannelArg_OverridesIdentityChannel asserts highest shipped stable (matching the SmokeTest contract). - New NewCommand_ExplicitStableChannel_NonStableCliVersion_FallsBackToHighestShippedStable regression test covers both daily-shape and PR-shape CLI identities. - NewCommandTests.NewCommandWithTypeScriptEmptyTemplatePassesResolvedVersionAndChannelToScaffolding reverts to its pre-#17564 assertion of '9.2.0' from the stable feed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
force-pushed
the
mitchdenny/forward-port-template-fixes-to-main
branch
from
31b1660 to
58b3c85
Compare
mitchdenny
changed the title
Member
Author
|
Pushed update: prepended cherry-pick of #17528 ("Fix 13.4 staging CLI dropping nuget.config without Aspire package source mapping", @radical). #17573's tests reference the |
The cherry-picks of #17564 and #17573 alone do not address the `aspire new` + C# Blazor scenario from #17596 because that path runs through `TemplateNuGetConfigService.ResolveTemplatePackageAsync` (DotNet templates), not `NewCommand.ResolveCliTemplateVersionAsync` (CLI-runtime templates) where #17564's `TryGetCurrentCliTemplateVersionPackage` synthesis lives. Teach `ResolveTemplatePackageAsync` to mirror the identity-channel preference that `ResolveCliTemplateVersionAsync` (NewCommand.cs:376-389) already does for CLI templates: when no `--channel` is supplied and no PR hives exist, prefer the channel whose name matches the running CLI's `CliExecutionContext.IdentityChannel` before falling back to Implicit. Without this, a daily 13.5 CLI on a clean `~/.aspire` silently restricts the template search to nuget.org and returns the shipped stable `Aspire.ProjectTemplates 13.3.5` instead of the matching `13.5.0-preview.1.*` daily prerelease from the `dotnet9` feed. The Implicit fallback preserves prior behavior when the identity channel isn't a registered channel (e.g. a "local" CLI without a corresponding local-build hive). See #17596 for full repro. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mitchdenny
changed the title
Member
Author
|
Added new commit Catching this during review: PR #17564 only modifies In The new commit teaches Tests: existing |
Contributor
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26576856038 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forward-ports two CLI fixes from
release/13.4tomain.Fixes #17596.
What's cherry-picked
3f0998aaspire new5d0da8bPrebuiltAppHostServerstagingglobalPackagesFolderpathBoth PRs are already merged on
release/13.4and need to land onmainso daily builds get the same fixes.Why this matters (#17564)
See #17596 for the full investigation. Short version:
On a clean
~/.aspire,aspire newwith a daily-build CLI frommainand a DotNet template (C# Blazor /aspire-starter) was resolvingAspire.ProjectTemplates 13.3.5from nuget.org instead of the expected13.5.0-preview.1.*from thedotnet9daily feed. Root cause:TemplateNuGetConfigService.ResolveTemplatePackageAsyncrestricts the channel search to the Implicit (nuget.org) channel when neither--channelis supplied nor PR hives exist on disk. Folks with stale~/.aspire/hives/*from past PR installs never saw this — clearing~/.aspireexposes it.PR #17564 adds
NewCommand.TryGetCurrentCliTemplateVersionPackage, which synthesizes aAspire.ProjectTemplatespackage pinned to the CLI's own SDK version for non-stable Explicit channels (daily/staging). A daily 13.5 CLI now lock-steps to 13.5 daily templates regardless of what nuget.org ships.Why this matters (#17573)
PackagingServicecreates the staging channel withConfigureGlobalPackagesFolder=trueso each darc/override feed restore lands in an isolated cache (two staging builds of the same release branch ship as13.4.0but from different feeds, and NuGet keys by(id,version)only).PrebuiltAppHostServerwas wiring that flag into aTemporaryNuGetConfigwhose relative.nugetpackagesdefault resolved under the temp config's own directory;BundleNuGetServicebaked those temp paths intointegration-package-probe-manifest.json, andTemporaryNuGetConfig.Disposethen recursively deleted the cache out from under the manifest. On macOS osx-arm64 polyglot staging builds this surfaced as a hang during DI / assembly loading inaspire-managed.The fix anchors the override at a stable absolute path:
<ASPIRE_HOME>/.nugetpackages/<first-8-of-CLI-commit-sha>, matching the existingdarc-pub-microsoft-aspire-<hash>feed URL convention inPackagingService.GetStagingFeedUrl.Validation
Cherry-picks applied cleanly (no conflicts). Both PRs already passed CI on
release/13.4. Will rely on this PR's CI run to confirm the forward-port compiles and tests pass on top of currentmain.cc @radical @davidfowl @joperezr @sebastienros @danegsta