Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OPTIMIZATION] Use %LocalAppData% instead of hardcoded paths #5298

Open
o3wiz opened this issue Mar 13, 2025 · 7 comments
Open

[OPTIMIZATION] Use %LocalAppData% instead of hardcoded paths #5298

o3wiz opened this issue Mar 13, 2025 · 7 comments
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.

Comments

@o3wiz
Copy link

o3wiz commented Mar 13, 2025

Description of the new feature / enhancement

Winget currently installs packages to the absolute path:

C:\Users\<user_name>\AppData\Local\Microsoft\WinGet\Packages

Instead, using %LocalAppData% would improve flexibility and compatibility:

%LocalAppData%\Microsoft\WinGet\Packages

Proposed technical implementation details

No response

@o3wiz o3wiz added the Issue-Feature This is a feature request for the Windows Package Manager client. label Mar 13, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Triage Issue need to be triaged label Mar 13, 2025
@Trenly
Copy link
Contributor

Trenly commented Mar 13, 2025

It already does use the indirect path to LOCALAPPDATA through the KNOWNFOLDERID Shell API, and does not build an absolute path

case PathName::PortablePackageUserRoot:
result.Path = Settings::User().Get<Setting::PortablePackageUserRoot>();
if (result.Path.empty())
{
result.Path = GetKnownFolderPath(FOLDERID_LocalAppData);
result.Path /= s_PortablePackageUserRoot_Base;
result.Path /= s_PortablePackageRoot;
result.Path /= s_PortablePackagesDirectory;
}
mayBeInProfilePath = true;

std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id)
{
wil::unique_cotaskmem_string knownFolder = nullptr;
THROW_IF_FAILED(SHGetKnownFolderPath(id, KF_FLAG_NO_ALIAS | KF_FLAG_DONT_VERIFY | KF_FLAG_NO_PACKAGE_REDIRECTION, NULL, &knownFolder));
return knownFolder.get();
}

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Author-Feedback Issue needs attention from issue or PR author and removed Needs-Triage Issue need to be triaged labels Mar 13, 2025
@o3wiz
Copy link
Author

o3wiz commented Mar 14, 2025

@Trenly
It looks like the implementation should behave as you've described, but in practice, the absolute path still ends up being written directly into the PATH environment variable.

Image

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Mar 14, 2025
@Trenly
Copy link
Contributor

Trenly commented Mar 14, 2025

@o3wiz - But isn't that the way it should be? Imagine the case where a package is installed with WinGet, then a user changes where %LOCALAPPDATA% points to without moving the already installed files. In that scenario, the installation would become broken and the package would be unusable until either the environment variable is reverted back, or the files are moved

@o3wiz
Copy link
Author

o3wiz commented Mar 14, 2025

@o3wiz - But isn't that the way it should be? Imagine the case where a package is installed with WinGet, then a user changes where %LOCALAPPDATA% points to without moving the already installed files. In that scenario, the installation would become broken and the package would be unusable until either the environment variable is reverted back, or the files are moved

I'm not sure that's a realistic scenario; after all, changing %LocalAppData% manually seems unusual. A more common case might be if the user changes their username, causing hardcoded paths to break.

@Trenly
Copy link
Contributor

Trenly commented Mar 14, 2025

I see. . .

@denelon denelon removed the Needs-Attention Issue needs attention from Microsoft label Mar 16, 2025
@ArnieGA
Copy link

ArnieGA commented Mar 25, 2025

@Trenly
Here's another use case example: usernames that contain a space. It affects other applications that leverage Winget, like UnigetUI, where it causes a warning about winget having to be fixed (reset). In this case in particular, an exception is always thrown in Powershell related to the user's directory in Windows.

For example:

  • Username: "Firstname Lastname"
  • %LocalAppData%: "C:\Windows\Firstname Lastname\AppData\Local"
  • Error thrown when resetting Winget:
'C:\Users\Firstname' is not recognized as an internal or external command,
operable program or batch file.
ERROR: The process "winget.exe" not found.

It seems as if having a space in the username was not considered, or purposely left out.

@whindsaks
Copy link

PathUnExpandEnvStrings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Projects
None yet
Development

No branches or pull requests

5 participants