Eval env variables in config values

[TravisTheTechie]

Applies to NuGet/Home#1852

Takes environment variable references in config values, and evaluates those references to what's in the environment. Uses Environment.ExpandEnvironmentVariables, so it should be cross platform already.

Execution

• Settings.GetValue()
• Settings.GetSettingValues()
• Documentation Issue 1852 eval env variables in config values NuGetDocs#431

Tested

• Windows
• Mac OS X
• Linux
• Unit tests
• Functional tests

Considerations

• Environment.ExpandEnvironmentVariables looks for %ENV_NAME% and *nix-based systems use the format $ENV_NAME. Both could be supported, but looking at a couple other projects, it seems the Windows-style naming is consistent on .NET-based cross platform projects.
• Are there other channels this misses? ConfigurationDefaults, for example, uses both GetValue and GetSettingValues. I didn't obviously see another location that taps into this different.

[dnfclas]

Hi @TravisTheTechie, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

[dnfclas]

@TravisTheTechie, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[yishaigalatzer]

nit: use the string.IsNullOrEmpty casing

[TravisTheTechie]

I was matching the casing existing in the file, e.g. https://github.com/TravisTheTechie/NuGet.Client/blob/issue-1852-eval-env-variables-in-config-values/src/NuGet.Core/NuGet.Configuration/Settings/Settings.cs#L59, should I go ahead an update the other references to match or just leave it with my changes?

[yishaigalatzer]

@TravisTheTechie looks pretty neat! I'm excited about this fix.

I would add a functional test for this (add a test that uses the nuget command line to read a source location) and properly prints it out (or restores from it). You can use a local folder that you put in a config file and expand it.

It will also be nice to create a PR in the docs repo (CC @csharpfritz) showing an example for this cool feature

[yishaigalatzer]

@emgarten @zhili1208 @deepakaravindr can you guys take a look?

[deepakaravindr]

nit: spelling error

[TravisTheTechie]

Applied feedback, minus functional test. I'll poke around and see if I can figure that out. I see my changes working locally, but everyone shouldn't have to reproduce my setup:

Or on my Mac.

$ cat nuget.config 
<configuration>
    <config>
        <add key="repositoryPath" value="%HOME%\NuGetPackages-with-env" />
    </config>
</configuration>

$ mono NuGet.exe config repositoryPath
/Users/tsmith\NuGetPackages-with-env

Is there a way I can build just NuGet.exe using build.ps1? It takes me on the order of 30 minutes to fully build everything and it would be awesome to have a quicker turn around on that.

[TravisTheTechie]

And I can make a PR for docs once this is complete.

[yishaigalatzer]

Building fast is build.ps1 -SkipTests would get you 4-5 minute build

[TravisTheTechie]

So I added a PowerShell script which should test this functionality. Though I can't figure out how to execute it correctly. Am I going down the right track for functional tests?

[yishaigalatzer]

This should be a simple unit test spinning nuget.exe look at the tests in the nuget.commandline.test

[TravisTheTechie]

Added functional test.

Tested on Ubuntu.

I believe this covers everyone's feedback. Anything additional?

[yishaigalatzer]

Docs pr

[TravisTheTechie]

NuGet/NuGetDocs#431 covers the docs. Unless someone has other feedback for me, I think this is done.

It doesn't look like you do it normally, but if you want I can squash my commits for cleaner history.

[yishaigalatzer]

We actually do squash it normally (some people missed it in the last few weeks). So yes Please!

[yishaigalatzer]

@deepakaravindr please have a final review and merge (just the code, we will merge the doc PR when we ship). @TravisTheTechie it is better that you squash so we don't accidentally remove you as the author

[TravisTheTechie]

Squashed. Will squash docs as well.

[yishaigalatzer]

@deepakaravindr ping

[deepakaravindr]

Merged with 603907b