Drop nuget.config alongside single-file apphost.cs in aspire init - #16636
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16636Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16636" |
6efb099 to
2a6cdc0
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2a6cdc0 to
0d7107f
Compare
There was a problem hiding this comment.
Pull request overview
This PR creates a whitespace-only change intended to open a PR and trigger deployment E2E test runs (e.g., /deployment-test) to reproduce consistent failures on main.
Changes:
- Adds a trailing newline at the end of
eng/Versions.props(no functional change)
| <SystemTextJsonLTSVersion>8.0.6</SystemTextJsonLTSVersion> | ||
| </PropertyGroup> | ||
| </Project> | ||
|
|
There was a problem hiding this comment.
PR description says the whitespace-only change is in README.md, but the actual change is a trailing newline in eng/Versions.props. Please update the PR description (or adjust the changed file) so reviewers/test triage can accurately understand what was modified to trigger the deployment E2E runs.
|
/deployment-test |
|
🚀 Deployment tests starting on PR #16636... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
| // directive — neither the implicit restore done by `aspire add` (`dotnet package | ||
| // add --file apphost.cs`) nor `dotnet run --file apphost.cs` will succeed against | ||
| // a CLI built from a local hive. | ||
| DropNuGetConfigForLocalHives(workingDirectory); |
There was a problem hiding this comment.
What happens when more hives get added? Is that handled elsewhere?
There was a problem hiding this comment.
Refactored in b30f94a — �spire init now calls the shared TemplateNuGetConfigService (the same path aspire new uses for template output). Under the hood that delegates to NuGetConfigMerger.CreateOrUpdateAsync, which merges missing sources into an existing nuget.config rather than overwriting it. So when more hives get added (e.g. a subsequent aspire add/aspire update/aspire new against a different channel writes to the workspace, or the user reconfigures their channel and re-runs an aspire command), the merger picks up the new sources from the resolved channel and adds them. Same handling as for templates today.
| return Task.FromResult(configResult); | ||
| } | ||
|
|
||
| private void DropNuGetConfigForLocalHives(DirectoryInfo workingDirectory) |
There was a problem hiding this comment.
Seems too special. Dont we have shared logic for nuget.config creation?
There was a problem hiding this comment.
Done — see b30f94a. InitCommand now injects TemplateNuGetConfigService and calls PromptToCreateOrUpdateNuGetConfigAsync(channelName: null, ...), which is exactly the path aspire new uses. Channel resolution comes from aspire config get channel so it works for any non-stable channel (local-*, dev-*, pr-*, run-*, staging) — not just channels literally named local — and the underlying NuGetConfigMerger handles create-or-merge of the workspace nuget.config consistently with templates.
Address PR review feedback (davidfowl, radical) by replacing the inline ad-hoc nuget.config creation in InitCommand with a call to the shared TemplateNuGetConfigService — the same path used by 'aspire new' for template output. This delegates to NuGetConfigMerger, which: * Resolves the user's configured channel (from 'aspire config get channel'), so it works for any non-stable hive — local-*, dev-*, pr-*, run-*, staging — not just channels named 'local'. * Creates a new nuget.config or merges missing sources into an existing one, so additional hives added later (e.g. via subsequent aspire commands) are handled the same way they are for templates. * No-ops for stable/implicit channels, matching prior behavior of doing nothing for the default user case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous refactor used PromptToCreateOrUpdateNuGetConfigAsync, which in the in-place case (workingDir == outputPath, the case for aspire init) prompts the user with 'Create NuGet.config for selected channels?'. The existing E2E and unit tests for aspire init don't expect any prompt, so they hung waiting for the next expected text. Add a new public method CreateOrUpdateNuGetConfigWithoutPromptAsync that always uses the silent merge path, and switch InitCommand to call it. This keeps the same shared NuGetConfigMerger create-or-merge semantics without changing the interactive UX of aspire init. Also register TemplateNuGetConfigService in the test DI container so InitCommand can resolve it from CliTestHelper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/deployment-test |
|
🚀 Deployment tests starting on PR #16636... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
The AspireInitAsync E2E test helper detects the NuGet.config Y/n prompt
by looking for the literal substring 'NuGet.config' on screen. The shared
NuGetConfigPrompter (used in the silent path) emits a confirmation message
that also contains 'NuGet.config', causing the helper to false-match the
message as a prompt and get out of sync with subsequent prompts (most
notably the agent-init Y/n).
Fix: in TemplateNuGetConfigService.CreateOrUpdateNuGetConfigWithoutPromptAsync,
bypass NuGetConfigPrompter and call NuGetConfigMerger directly so no
built-in confirmation message is emitted. Return a bool indicating whether
a config was written, and let InitCommand emit a neutral message
('Created package sources file') that does not contain the trigger
substring.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 2 jobs were identified as retry-safe transient failures in the CI run attempt.
Matched test failure patterns (4 tests)
|
|
/deployment-test |
|
🚀 Deployment tests starting on PR #16636... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
|
✅ Deployment E2E Tests passed — 33 passed, 0 failed, 0 cancelled View test results and recordings
|
|
⏳ Started quarantine operation for |
|
Mitch Denny (@mitchdenny) ❌ Quarantine failed. See the workflow run for full details. |
Three consecutive PR CI reruns failed with different transient timeouts in: - StopNonInteractiveMultipleAppHostsShowsError (3m timeout on 'AppHost started successfully') - StopAllAppHostsFromUnrelatedDirectory (8m20s timeout on success prompt) Both go through 'aspire new' + 'aspire start' (unrelated to the init/nuget.config changes in this PR). Tracking issue: #16643. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 76 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #25213730076 |
|
No documentation PR is required for this change. This is a bug fix that restores correct behavior for users with local hive configurations — Automated check by the docs-check workflow.
|
|
/backport to release/13.3 |
|
Started backporting to |
|
Mitch Denny (@mitchdenny) backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: chore: whitespace change to trigger deployment test repro
Applying: Drop nuget.config alongside single-file apphost.cs in `aspire init`
Applying: Use shared TemplateNuGetConfigService for init nuget.config
Using index info to reconstruct a base tree...
M src/Aspire.Cli/Commands/InitCommand.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Aspire.Cli/Commands/InitCommand.cs
Applying: Add silent CreateOrUpdateNuGetConfigWithoutPromptAsync overload
Using index info to reconstruct a base tree...
M src/Aspire.Cli/Commands/InitCommand.cs
M src/Aspire.Cli/Templating/TemplateNuGetConfigService.cs
M tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Aspire.Cli/Commands/InitCommand.cs
Auto-merging src/Aspire.Cli/Templating/TemplateNuGetConfigService.cs
CONFLICT (content): Merge conflict in src/Aspire.Cli/Templating/TemplateNuGetConfigService.cs
Auto-merging tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0004 Add silent CreateOrUpdateNuGetConfigWithoutPromptAsync overload
Error: The process '/usr/bin/git' failed with exit code 128 |
Problem
After #15918 redesigned
aspire initto drop a single-fileapphost.cs(with a#:sdk Aspire.AppHost.Sdk@<version>directive) and anaspire.config.jsoninstead of a full.csprojproject, the generated workspace contains nonuget.config.When a CLI built from a local hive (e.g.
~/.aspire/hives/<name>/packages) is used, the local hive is invisible to MSBuild — so the SDK directive cannot be resolved and any operation that triggers MSBuild restore fails. The deployment E2E tests run against exactly this configuration and were failing 12 of 13 with:The same failure mode also breaks real users with a local hive who run any of:
aspire add <pkg>(which runsdotnet package add --file apphost.cs— the path the deployment E2E tests hit)dotnet run --file apphost.csaspire startRoot cause
InitCommand.DropCSharpSingleFileSkeletonAsyncwritesapphost.csandaspire.config.jsonbut nothing that would let NuGet/MSBuild discover the local hive. Previously, the full-project template path inherited package source resolution from the dev environment; the single-file path has no such inheritance and needs an explicitnuget.confignext toapphost.cs.Fix
When
aspire initdrops the single-file C# skeleton, also drop a workspace-localnuget.configlisting every local hive under~/.aspire/hives/<name>/packagesas a NuGet package source.nuget.configis written.nuget.configin the workspace is preserved (no clobbering).AddCommandPR-hive logic.Validation
Aspire.Clilocally — clean build./deployment-teston this PR. Previous run (without the fix) had 13 failures / 24 passes; new run with the fix has 37 passes, 0 failures, 1 skipped — all 12aspire add ... → SDK 'Aspire.AppHost.Sdk/...' could not be foundfailures are gone.WaitUntiltimeout — Pattern B — being tracked separately).Note on this PR
This branch was originally created purely to reproduce the deployment-test failures with a whitespace commit; the whitespace commit on
eng/Versions.propsis preserved here for traceability of the repro. Reviewers can squash on merge.