Respect DOTNET_NUGET_CONFIG and GLOBAL_JSON environment variables

ViktorHofer commented 7 months ago

NuGet Product(s) Involved

NuGet.exe, MSBuild.exe, dotnet.exe

The Elevator Pitch

The VMR - Virtual Monolithic Repository builds the .NET SDK from a single repository. Every sub-repository has a NuGet.config and global.json file and these need to be changed during the VMR build. The VMR already sets the RestoreConfigFile property but that isn't respected by the nuget sdk resolver which doesn't have access to msbuild properties. Therefore, we want NuGet client to respect a new NUGET_CONFIG environment variable which would be preferred over the manual path walking when set.

In addition to the NuGet.config file, the VMR also needs to update the global.json file which is read by the nuget sdk resolver. For the same reason, we want NuGet client to respect a new GLOBAL_JSON environment variable which would win and skip the manual path walking when set. I plan to open an issue for the dotnet host as well to respect that new GLOBAL_JSON environment variable.

The code paths in question that would need to be updated are:

The VMR effort would greatly benefit from these additions as we currently hack around that limitation with a custom sdk resolver which has limitations and doesn't work in all scenarios.

We (@mmitche, @ericstj, @jkoritzinsky and I) discussed this with @jeffkl a few weeks ago and he asked us to open an issue to track our ask. If the proposed solution sounds good, I will submit a PR. We would like to consume this new feature as soon as possible as it will greatly improve the overall VMR experience.

Additional Context and Details

More context in https://github.com/dotnet/source-build/issues/3781

jeffkl commented 7 months ago

Respecting GLOBAL_JSON is a no brainer. @nkolev92 what do you think about an environment variable NUGET_CONFIG that simply overrides the path to NuGet.Config, skipping the search for it in parent folders?

richlander commented 7 months ago

We should prepend DOTNET to our ENVs wherever we can. NUGET is sufficient for that, IMO.

We want to:

Avoid collisions with our ecosystems.
Make it easy for others to self-identify that a given ENV is for a given dev plat when someone inspects the result of set or export.
DOTNET ENVs all appear/sort together.

We should also decide if we want DOTNET_SDK_BLAH.

@baronfel

nkolev92 commented 7 months ago

I feel like the concerns of the SDK resolver and NuGet in general are slightly different. The SDK resolver respecting GLOBAL_JSON seems like an easy one I agree, since it doesn't really affect NuGet as a whole.

I've been a fan of having a single way to specify config file, but env vars are not ideal due to lack of transparency. What are the commands that are missing a restore config file? Would having a resolver specific solution work or are there other scenarios to consider?

ViktorHofer commented 7 months ago

The nuget sdk resolver reads the global.json and NuGet.config file and doesn't have access to msbuild properties or CLI flags as it is loaded by the SDK.

What are the commands that are missing a restore config file? Would having a resolver specific solution work or are there other scenarios to consider?

Yes, this only affects the nuget sdk resolver. Other components respect the RestoreConfigFile msbuild property.

jeffkl commented 7 months ago

We'll discuss this in today's triage

jeffkl commented 7 months ago

Two things came from our discussion:

We're fine with respecting GLOBAL_JSON or whatever, just get consensus from the .NET folks on the name (like should it be DOTNET_GLOBAL_JSON or whatever

We'd like the implementation of respecting both GLOBAL_JSON and especially NUGET_CONFIG to log something to indicate that the environment variable is set and its side effect. So at the end of restore, NuGet has a summary:

NuGet Config files used:
  C:\Users\jeffkl\AppData\Roaming\NuGet\NuGet.Config
  C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config

Feeds used:
  https://api.nuget.org/v3/index.json
  C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\

If that summary said something like "Used D:\Something\NuGet.config because it was specified in the NUGET_CONFIG environment variable", that would be great. We're very afraid of adding this magic and it not being obvious to users of the side effect. We've personally experienced cases where an environment variable is set and can't for the life of us figure out why something isn't working. Informing the user at the end of restore will greatly help them from scratching their heads when restore is using an unexpected config.

nkolev92 commented 7 months ago

I think plumbing the fact that an env var is being used in the summary might be more challenging.

Personally, I'm fine if it's in the standard log

richlander commented 7 months ago

FYI ...

rich@mazama:~$ cat global.json 
{
  "sdk": {
    "version": "8.0.100"
  }
}
rich@mazama:~$ export DOTNET_ROOT=/home/rich/dotnet
rich@mazama:~$ dotnet --info
.NET SDK:
 Version:           8.0.102
 Commit:            b7800db369
 Workload version:  8.0.100-manifests.03fa9662

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  23.10
 OS Platform: Linux
 RID:         ubuntu.23.10-x64
 Base Path:   /usr/lib/dotnet/sdk/8.0.102/

.NET workloads installed:
 Workload version: 8.0.100-manifests.03fa9662
There are no installed workloads to display.

Host:
  Version:      8.0.2
  Architecture: x64
  Commit:       1381d5ebd2

.NET SDKs installed:
  8.0.102 [/usr/lib/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.2 [/usr/lib/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.2 [/usr/lib/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/home/rich/dotnet]

global.json file:
  /home/rich/global.json

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

ViktorHofer commented 7 months ago

Ok, NUGET_CONFIG and DOTNET_GLOBAL_JSON it is then. I will explore what needs to change to indicate that these env var overrides are used.

NuGet / Home