Use Environment.SystemDirectory from GetFolderPath(System) on Windows

[stephentoub]

[Benchmark]
public string SystemDir() => Environment.GetFolderPath(Environment.SpecialFolder.System);

Method	Toolchain	Mean	Error	StdDev	Ratio	Code Size	Allocated	Alloc Ratio
SystemDir	\main\corerun.exe	1,469.60 ns	9.071 ns	7.082 ns	1.00	1,986 B	64 B	1.00
SystemDir	\pr\corerun.exe	53.36 ns	0.618 ns	0.516 ns	0.04	2,152 B	64 B	1.00

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[eerhardt]

I thought @jkotas had reasons why we shouldn't do this, but I could be wrong. IIRC, it was differences in behavior between

https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getsystemdirectoryw
and
https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetknownfolderpath

[stephentoub]

If there's a meaningful functional difference, we obviously should close this. We should also be very hesitant to add an analyzer (#83591).

[stephentoub]

[stephentoub]

I see Jan's comment at dotnet/aspnetcore#47269 (comment). I guess we're not sure if System is one of the ones affected, so better safe than sorry...

That said, I think this would primarily be about corner-case compat. As a user reading our docs and trying to understand whether to use Environment.SystemDirectory or Environment.GetFolderPath(Environment.SpecialFolders.System), I wouldn't expect there to be any difference.

[jkotas]

I think this would primarily be about corner-case compat.

Yes, it is about compat for non-mainstream Windows configurations. I have stepped through the Windows implementation of ShGetFolderPath and it has many special cased situations (registry overrides, appcontrainers, ...). It is hard to tell whether any of those special cases matter for system directory.

runtime/src/libraries/System.Runtime.Extensions/tests/System/EnvironmentTests.cs

Lines 384 to 393 in c8f9f29

	if (PlatformDetection.IsWindowsNanoServer)
	{
	// https://github.com/dotnet/runtime/issues/21430
	// On Windows Nano, ShGetKnownFolderPath currently doesn't give
	// the correct result for SystemDirectory.
	// Assert that it's wrong, so that if it's fixed, we don't forget to
	// enable this test for Nano.
	Assert.NotEqual(Environment.GetFolderPath(Environment.SpecialFolder.System), Environment.SystemDirectory);
	return;
	}

is going to fail. If we want to take this change, this test will need to be fixed.

[stephentoub]

If we want to take this change, this test will need to be fixed.

Interestingly, this test also assumes that the two variants are the same in all other cases.

[jkotas]

Thanks

[MichalStrehovsky]

Any reason not to also do it for .Windows with GetWindowsDirectoryW?

[jkotas]

Any reason not to also do it for .Windows with GetWindowsDirectoryW?

There is no distinction between .Win32.cs and .Windows.cs anymore. It is left-over from the UWP days. *.Win32.cs files can be renamed or merged into .Windows.cs files, nobody bothered to do it just yet.

[MichalStrehovsky]

Oh I meant SpecialFolder.Windows.