Skip to content

win32: add fallback to environment vars for system folder #109673

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jan 28, 2025
Merged

win32: add fallback to environment vars for system folder #109673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a5ba6e
win32: add fallback to environment vars for system folder
kasperk81 Nov 9, 2024
3d347fe
feedback
kasperk81 Nov 16, 2024
8c2909e
Update src/libraries/System.Private.CoreLib/src/System/Environment.Wi…
kasperk81 Nov 16, 2024
0c735d6
Merge branch 'main' into patch-3
kasperk81 Nov 17, 2024
2cb73ea
Merge branch 'main' into patch-3
kasperk81 Dec 14, 2024
03f1500
Merge branch 'main' into patch-3
tannergooding Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/libraries/Microsoft.Extensions.DependencyModel/tests/PackageResolverTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.IO;
using FluentAssertions;
using Microsoft.DotNet.XUnitExtensions;
using Microsoft.Extensions.DependencyModel.Resolution;
using Xunit;
using F = Microsoft.Extensions.DependencyModel.Tests.TestLibraryFactory;
Expand All @@ -28,7 +27,7 @@ public void ShouldUseEnvironmentVariableToGetDefaultLocation()
result.Should().Contain(PackagesPath);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] // https://github.com/dotnet/runtime/issues/21430
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ViktorHofer @akoeplinger I removed this condition, and it's now passing where it was failing before: link to comment. I didn’t update other environment tests since they compare values against win32 SHGetFolderPathW, which returns a general error (-2147467259) on nano server

[Fact]
public void ShouldUseNugetUnderUserProfile()
{
var environment = EnvironmentMockBuilder.Create()
Expand Down
21 changes: 14 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
// The only SpecialFolderOption defines we have are equivalent to KnownFolderFlags.

string folderGuid;

string? fallbackEnv = null;
switch (folder)
{
// Special-cased values to not use SHGetFolderPath when we have a more direct option available.
Expand All @@ -230,12 +230,15 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
// Map the SpecialFolder to the appropriate Guid
case SpecialFolder.ApplicationData:
folderGuid = Interop.Shell32.KnownFolders.RoamingAppData;
fallbackEnv = "APPDATA";
break;
case SpecialFolder.CommonApplicationData:
folderGuid = Interop.Shell32.KnownFolders.ProgramData;
fallbackEnv = "ProgramData";
break;
case SpecialFolder.LocalApplicationData:
folderGuid = Interop.Shell32.KnownFolders.LocalAppData;
fallbackEnv = "LOCALAPPDATA";
break;
case SpecialFolder.Cookies:
folderGuid = Interop.Shell32.KnownFolders.Cookies;
Expand Down Expand Up @@ -292,9 +295,11 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
break;
case SpecialFolder.ProgramFiles:
folderGuid = Interop.Shell32.KnownFolders.ProgramFiles;
fallbackEnv = "ProgramFiles";
break;
case SpecialFolder.CommonProgramFiles:
folderGuid = Interop.Shell32.KnownFolders.ProgramFilesCommon;
fallbackEnv = "CommonProgramFiles";
break;
case SpecialFolder.AdminTools:
folderGuid = Interop.Shell32.KnownFolders.AdminTools;
Expand Down Expand Up @@ -346,12 +351,15 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
break;
case SpecialFolder.UserProfile:
folderGuid = Interop.Shell32.KnownFolders.Profile;
fallbackEnv = "USERPROFILE";
break;
case SpecialFolder.CommonProgramFilesX86:
folderGuid = Interop.Shell32.KnownFolders.ProgramFilesCommonX86;
fallbackEnv = "CommonProgramFiles(x86)";
break;
case SpecialFolder.ProgramFilesX86:
folderGuid = Interop.Shell32.KnownFolders.ProgramFilesX86;
fallbackEnv = "ProgramFiles(x86)";
break;
case SpecialFolder.Resources:
folderGuid = Interop.Shell32.KnownFolders.ResourceDir;
Expand All @@ -364,18 +372,17 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
break;
case SpecialFolder.Windows:
folderGuid = Interop.Shell32.KnownFolders.Windows;
fallbackEnv = "windir";
break;
}

Guid folderId = new Guid(folderGuid);

int hr = Interop.Shell32.SHGetKnownFolderPath(folderId, (uint)option, IntPtr.Zero, out string path);
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is part of the shell experience, so desktop only. So I'm not surprised that it fails on systems without shell. This will probably also fix usages in system services, etc. And probably everything else where no user registry hives are loaded.

if (hr != 0) // Not S_OK
{
return string.Empty;
}
if (hr == 0)
return path;

return path;
// Fallback logic if SHGetKnownFolderPath failed (nanoserver)
return fallbackEnv != null ? Environment.GetEnvironmentVariable(fallbackEnv) ?? string.Empty : string.Empty;
}

// Separate type so a .cctor is not created for Environment which then would be triggered during startup
Expand Down
Loading