Skip to content

Remove redundant Path.GetDirectoryName call and rename _appPath/_AppPath to _appDirectoryPath/AppDirectoryPath in PrebuiltAppHostServer#15332

Merged
joperezr merged 3 commits intorelease/13.2from
copilot/remove-path-getdirectoryname-call
Mar 18, 2026
Merged

Remove redundant Path.GetDirectoryName call and rename _appPath/_AppPath to _appDirectoryPath/AppDirectoryPath in PrebuiltAppHostServer#15332
joperezr merged 3 commits intorelease/13.2from
copilot/remove-path-getdirectoryname-call

Conversation

Copy link
Contributor

Copilot AI commented Mar 17, 2026

Customer Impact

With aspire polyglot, NuGet.config files in the same directory as the apphost aren't respected.

Description

_appPath in PrebuiltAppHostServer already holds a directory path, making the Path.GetDirectoryName calls redundant. This cleans up two such callsites, renames the private field to _appDirectoryPath, and renames the public property AppPath to AppDirectoryPath to accurately reflect their semantics.

Changes in PrebuiltAppHostServer.cs:

  • Renamed _appPath_appDirectoryPath throughout
  • Renamed AppPath property → AppDirectoryPath
  • RestoreNuGetPackagesAsync: removed intermediate appHostDirectory local and pass _appDirectoryPath directly as workingDirectory
  • ResolveChannelNameAsync: removed Path.GetDirectoryName(_appPath)! and pass _appDirectoryPath directly to AspireJsonConfiguration.Load

Changes in IAppHostServerProject.cs:

  • Renamed AppPath property → AppDirectoryPath

Changes in DotNetBasedAppHostServerProject.cs:

  • Renamed AppPath property → AppDirectoryPath

Before:

var appHostDirectory = Path.GetDirectoryName(_appPath);
return await _nugetService.RestorePackagesAsync(..., workingDirectory: appHostDirectory, ...);

After:

return await _nugetService.RestorePackagesAsync(..., workingDirectory: _appDirectoryPath, ...);

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
    • No

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI and others added 2 commits March 17, 2026 17:26
…yPath in PrebuiltAppHostServer

Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
…and implementations

Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 17, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • centralus-2.in.applicationinsights.azure.com
    • Triggering command: /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests --internal-msbuild-node /home/REDACTED/.local/share/c3763b7b477943e6978acca6c0d47ec9/.p --filter-not-trait category=failing --filter-not-trait quarantined=true --filter-not-trait outerloop=true (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=null Aspire.Cli.Tests.CliSmokeTests&#43;&lt;&gt;c &lt;LocaleOverrideReturnsExitCode&gt;b__4_0 /tmp/bespesno.343 invalid-locale False ASPIRE_LOCALE_OVERRIDE (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=null Aspire.Cli.Tests.CliSmokeTests&#43;&lt;&gt;c &lt;LocaleOverrideReturnsExitCode&gt;b__4_0 /tmp/bespesno.343 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Remove redundant Path.GetDirectoryName call and rename _appPath to _appDirectoryPath in PrebuiltAppHostServer Remove redundant Path.GetDirectoryName call and rename _appPath/_AppPath to _appDirectoryPath/AppDirectoryPath in PrebuiltAppHostServer Mar 17, 2026
Copilot AI requested a review from eerhardt March 17, 2026 20:16
@eerhardt eerhardt marked this pull request as ready for review March 17, 2026 22:07
@eerhardt eerhardt requested review from Copilot and mitchdenny March 17, 2026 22:07
@github-actions
Copy link
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15332

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15332"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR clarifies and standardizes “app path” semantics for CLI AppHost server projects by treating the value as a directory path and removing redundant/incorrect Path.GetDirectoryName usage.

Changes:

  • Renamed AppPathAppDirectoryPath on IAppHostServerProject and updated implementations.
  • Renamed PrebuiltAppHostServer’s backing field to _appDirectoryPath and removed redundant Path.GetDirectoryName call sites.
  • Simplified NuGet restore/config loading to operate directly on the app host directory path.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Aspire.Cli/Projects/PrebuiltAppHostServer.cs Renames internal storage to _appDirectoryPath and uses it directly for NuGet restore working directory and local .aspire/settings.json loading.
src/Aspire.Cli/Projects/IAppHostServerProject.cs Renames the interface property to AppDirectoryPath to reflect directory semantics.
src/Aspire.Cli/Projects/DotNetBasedAppHostServerProject.cs Updates the implementation to expose AppDirectoryPath (mapped to existing normalized _appPath).

@joperezr joperezr added the Servicing-approved Approved for servicing release label Mar 18, 2026
@joperezr joperezr merged commit 777972e into release/13.2 Mar 18, 2026
508 of 511 checks passed
@joperezr joperezr deleted the copilot/remove-path-getdirectoryname-call branch March 18, 2026 17:53
@dotnet-policy-service dotnet-policy-service bot added this to the 13.2 milestone Mar 18, 2026
Copilot AI added a commit that referenced this pull request Mar 18, 2026
…ath to _appDirectoryPath/AppDirectoryPath in PrebuiltAppHostServer (#15332)

* Initial plan

* Remove Path.GetDirectoryName call and rename _appPath to _appDirectoryPath in PrebuiltAppHostServer

Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>

* Rename AppPath property to AppDirectoryPath in IAppHostServerProject and implementations

Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants