Skip to content
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.

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

Improve DOTNET_HOST_PATH environment variable handling #7438

Closed
olivier-spinelli opened this issue Oct 25, 2018 · 12 comments · Fixed by NuGet/NuGet.Client#3335
Closed

Improve DOTNET_HOST_PATH environment variable handling #7438

olivier-spinelli opened this issue Oct 25, 2018 · 12 comments · Fixed by NuGet/NuGet.Client#3335
Assignees
Labels
Area:Plugin V2 plugin w/ cross platform support help wanted Considered good issues for community contributions. Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Product:dotnet.exe Type:Bug
Milestone

Comments

@olivier-spinelli
Copy link

Details about Problem

While using NuGet.Core v.4.8.0.6, with the Microsoft/artifacts-credprovider (no client, direct code from a console app), in netcoreapp2.1, runned in AppVeyor. Got:

NuGet: Using C:\Users\appveyor.nuget\plugins\netcore\CredentialProvider.Microsoft\CredentialProvider.Microsoft.dll as a credential provider plugin.
NuGet: Cannot start process because a file name has not been provided.

This is fixed with this environment variable (and the explanation is in the comment):

environment:
  # Workaround for dev/NuGet.Client\src\NuGet.Core\NuGet.Protocol\Plugins\PluginFactory.cs line 161:
  #   FileName = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH"),
  # This line should be:
  #   FileName = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ?? "dotnet",

HTH

@nkolev92 nkolev92 added Product:dotnet.exe Area:Plugin V2 plugin w/ cross platform support WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Oct 30, 2018
@nkolev92
Copy link
Member

I don't think the work around is a good idea.
The plugins have been to be executed by the exact same version of dotnet as the host process.

I'm curious why DOTNET_HOST_PATH is not set however.
Can you provide a repro? I did a quick test myself and I didn't see any problems.

@olivier-spinelli
Copy link
Author

olivier-spinelli commented Oct 30, 2018 via email

@nkolev92
Copy link
Member

I get that the workaround might have been used in other place, but for this scenario, it's super important that the CLI is exactly the same.

This variable should normally always be set.

/cc @nguerrera @livarcocc

Is there a CLI version in which the DOTNET_HOST_PATH is not set?
What was the last version that did not set it?

@nguerrera
Copy link

Any .NET Core SDK 2.0.* or lower would not set it. We started setting it in 2.1.*.

That doesn't appear to be the issue here, because the issues says netcoreapp2.1 and no SDK that would support that would not set it.

Rather, I think it is this: "(no client, direct code from a console app),"

DOTNET_HOST_PATH is currently set by the CLI when shelling out to msbuild, it is not set for all child processes of the dotnet.exe host. Furthermore, even if the dotnet.exe host did that, there is no guarantee that code running on .NET Core is running in a dotnet.exe host. It could be an apphost or some custom host.

@olivier-spinelli
Copy link
Author

I can confirm. By using the NuGet.Protocol packages directly from a console app runned by a "dotnet run" in NetCoreApp2.1, the "safety belt" is kind of required...

@olivier-spinelli
Copy link
Author

olivier-spinelli commented Nov 2, 2018

OK... But... can anyone explain to me how NuGet.Protocol can be used from a .Net console App?
Since I have no control on the launch of the Credential Provider, I can't configure the child process environment variables (startInfo.EnvironmentVariables["DOTNET_HOST_PATH"] = "...";).

It seems to me that there is a critical architectural issue here.... or am I missing something?
(Other than asking the console app users to add "DOTNET_HOST_PATH" = "dotnet" in their own environment variables of course...)

[Edit]
Using Environment.SetEnvironmentVariable( "DOTNET_HOST_PATH", "dotnet" ); makes the environment variable inherited.
Even if I can work with that, I'm wondering if I'm not closing this issue. Feel free to close it if all this must be "by design" (but I doubt it).

@heng-liu heng-liu added Type:Bug and removed WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Jan 25, 2019
@heng-liu heng-liu added this to the Backlog milestone Jan 25, 2019
@ransagy
Copy link

ransagy commented Aug 22, 2019

Sorry for reviving this a bit; But i came across the exact same issue. I'm trying to use NuGet.Protocol (or rather, Using the github.com/NuKeeperDotNet/NuKeeper library) directly with an Azure DevOps feed (and the Azure Artifacts Credential Provider) and getting the exact same issue. I have dotnet SDKs installed ranging from 2.1 to latest 3.0 preview - Yet i do NOT have DOTNET_HOST_PATH set at all.

Not sure where to take this forward from here.

@tillig
Copy link

tillig commented Jan 28, 2020

.NET Core SDK 3.1.100 on MacOS also doesn't set DOTNET_HOST_PATH so credential providers fail mysteriously.

At the very least, a check in that block of code so it's not the process throwing an InvalidOperationException would be nice.

var hostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH");
if (String.IsNullOrEmpty(hostPath))
{
  throw new InvalidOperationException("Missing DOTNET_HOST_PATH environment variable. Unable to start plugin.");
}

var startInfo = new ProcessStartInfo
{
  FileName = hostPath,
  Arguments = $"\"{filePath}\" " + args,
  ...
};

Would folks be open to a PR for that?

@nkolev92
Copy link
Member

I think based on @nguerrera's response

DOTNET_HOST_PATH is currently set by the CLI when shelling out to msbuild, it is not set for all child processes of the dotnet.exe host. Furthermore, even if the dotnet.exe host did that, there is no guarantee that code running on .NET Core is running in a dotnet.exe host. It could be an apphost or some custom host.

we should consider just calling dotnet or dotnet.exe if the env var is not set. That would enable the scenarios for 3rd party apps using NuGet's libraries as @olivier-spinelli suggested.

If we want to be smarter we can expose a component that allows the caller to control exactly what's being used, but that might be overkill.

@dtivel thoughts?

@nkolev92 nkolev92 added help wanted Considered good issues for community contributions. Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. labels Apr 8, 2020
@nkolev92
Copy link
Member

nkolev92 commented Apr 8, 2020

Hey folks,

I think it's ok to fall back to dotnet or dotnet.exe based on the environment we're running in case DOTNET_HOST_PATH is not set.

At this point I'm not sure when we'd be able to get to this, but we'd be happy to take a PR.

@tillig
Copy link

tillig commented Apr 8, 2020

@olivier-spinelli
Copy link
Author

Should I sign a CLA or something?

@aortiz-msft aortiz-msft changed the title Faulty DOTNET_HOST_PATH environment variable handling. Improve DOTNET_HOST_PATH environment variable handling Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Plugin V2 plugin w/ cross platform support help wanted Considered good issues for community contributions. Priority:3 Issues under consideration. With enough upvotes, will be reconsidered to be added to the backlog. Product:dotnet.exe Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants