Skip to content

Do not verify existence of LocalApplicationData directory in dotnetup#54516

Merged
lbussell merged 1 commit into
dotnet:release/dnupfrom
lbussell:lbussell/dnup-issue-54509
May 30, 2026
Merged

Do not verify existence of LocalApplicationData directory in dotnetup#54516
lbussell merged 1 commit into
dotnet:release/dnupfrom
lbussell:lbussell/dnup-issue-54509

Conversation

@baronfel baronfel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Style nit (for dotnetup, not your change specifically): I don't like how default-path computation is so spread out - feels like we had to whack-a-mole a bit to find all the places that could hit this issue. Could/should these be centralized/co-located?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes dotnetup crashing on minimal Linux environments where ~/.local/share doesn't exist by using Environment.SpecialFolderOption.DoNotVerify when resolving LocalApplicationData, so the path is computed rather than returned as empty. Directory creation is already deferred to write-time helpers.

Changes:

  • DotnetupPaths.GetBaseDirectory() now passes DoNotVerify.
  • DotnetupTelemetry.s_defaultStorageDirectory now passes DoNotVerify.
  • DownloadCache default cache path now passes DoNotVerify, with a comment explaining why it can't reuse DotnetupPaths.

Reviewed changes

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

File Description
src/Installer/dotnetup.Library/DotnetupPaths.cs Primary fix: resolve LocalApplicationData without verifying existence.
src/Installer/dotnetup.Library/Telemetry/DotnetupTelemetry.cs Same fix applied to the telemetry default storage directory.
src/Installer/Microsoft.Dotnet.Installation/Internal/DownloadCache.cs Same fix applied to default download cache path; added comment explaining layering.

@lbussell

Copy link
Copy Markdown
Member Author

LGTM!

Style nit (for dotnetup, not your change specifically): I don't like how default-path computation is so spread out - feels like we had to whack-a-mole a bit to find all the places that could hit this issue. Could/should these be centralized/co-located?

I think you could easily have everything centralized if you:

  • Move DotnetupTelemetry's default directory into DotnetupPaths.
  • Remove the default cache directory from Microsoft.DotNet.Installation. Having it default to a dotnetup directory seems kind of odd, if it's intended to be general-use (I'm not sure what the intention is).

@lbussell lbussell enabled auto-merge (squash) May 29, 2026 21:24
@lbussell lbussell merged commit 8dc13b1 into dotnet:release/dnup May 30, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants