dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

Environment.ExpandEnvironmentVariables on Linux has Windows behavior #25792

Open FeodorFitsner opened 6 years ago

FeodorFitsner commented 6 years ago

Environment.ExpandEnvironmentVariables() on Linux expands variables in %VAR_NAME% format rather than $VAR_NAME.

SDK version: 2.1.4 OS: Ubuntu 16.04

A simple test:

Environment.ExpandEnvironmentVariables("$HOME/test");

Expected value: /home/{user}/test Actual value: $HOME/test

However running:

Environment.ExpandEnvironmentVariables("%HOME%/test");

gives expanded result /home/{user}/test.

danmoseley commented 6 years ago

My guess is this was done intentionally -- so existing code passing %VAR_NAME% "just works" when used on Unix. For code written for Unix, it is certainly unexpected. The API could also respect $ - if it is worth diverging further and becoming potentially confusing. Another option is an API

public string Environment.ExpandEnvironmentVariables(string name, EnvironmentVariablesExpandFlavor flavor = EnvironmentVariablesExpandFlavor.PlatformDefault);

[Flags]
public enum EnvironmentVariablesExpandFlavor
{
           PlatformDefault = 0x1,
           PercentSigns = 0x2,
           DollarSigns = 0x4
}

with better names...

Maybe @stephentoub recalls context.

stephentoub commented 6 years ago

Maybe @stephentoub recalls context.

Mainly because that's how it's documented: https://docs.microsoft.com/en-us/dotnet/api/system.environment.expandenvironmentvariables

A string containing the names of zero or more environment variables. Each environment variable is quoted with the percent sign character (%).

I could be convinced that you'd never do that on unix and this is really just surfacing a Windows-ism and so we should use $ on unix instead of %. But we'd want to understand how badly breaking that would be. I'd be worried that code has already done string.Replace('$','%') to make it work the desired way on unix, and then we'd be breaking those folks, for example.

joshfree commented 6 years ago

I think we should add a new overload that supports this.

I'd be worried that code has already done string.Replace('$','%') to make it work the desired way on unix,

ps - string.Replace('$', '%') would add $ suffixes to your expanded env vars 😉

gfoidl commented 6 years ago

For cross-platform development both variants should be allowed, similar to the path-separator (\ vs. /).

Batch allows the $ as valid variable name, but then it would have to be quoted by %, so %$%. This case can be handled. In Bash the % isn't allowd in variable names, so this case can be handled too.

Further there wouldn't be any breaking changes, since %var% and $var would work.

stephentoub commented 6 years ago

Further there wouldn't be any breaking changes, since %var% and $var would work.

Except $var on Windows may start to be reinterpreted, changing output in some cases.

aaronfranke commented 6 years ago

It may also be worth considering expanding ~. I know that it is used by some programs in file names, so I'd suggest only expanding it if it's the first character in the given path string.

RickStrahl commented 3 years ago

Maybe a whole new API should be used for this because ExpandEnvironmentVariables doesn't sound like it's necessarily path specific.

Personally I would prefer something like FixupOsPath(path) or ExpandOsPath() that fixes up any path that could be used by the operating system directly. This would include the appropriate platform specific environment vars, as well as root folder expansion (ie. ~) and anything else that might be available for a specific Os target.

LethiferousMoose commented 2 years ago

Maybe I am misunderstanding the complexity of Environment.ExpandEnvironmentVariables(), but couldn't the code just do an OS check (Windows, Mac, Unix, etc.) and look for the proper symbols per OS?

tanith87 commented 10 months ago

Maybe a whole new API should be used for this because ExpandEnvironmentVariables doesn't sound like it's necessarily path specific.

Because it is not path specific. Environment variables can be anything. Not only paths.

Maybe I am misunderstanding the complexity of Environment.ExpandEnvironmentVariables(), but couldn't the code just do an OS check (Windows, Mac, Unix, etc.) and look for the proper symbols per OS?

This would certainly be a breaking change for any code that already relies on the %VAR% pattern.

moh-hassan commented 2 weeks ago

Is there any progress for this feature. .NET 9 is about to release in Nov 2024.

CyrusNajmabadi commented 2 weeks ago

@moh-hassan no proposal has been made for this.