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

Setting DOTNET_HOST_PATH is not a good move :) #123

Closed
dsyme opened this issue Nov 11, 2021 · 6 comments · Fixed by #126
Closed

Setting DOTNET_HOST_PATH is not a good move :) #123

dsyme opened this issue Nov 11, 2021 · 6 comments · Fixed by #126

Comments

@dsyme
Copy link
Contributor

dsyme commented Nov 11, 2021

ProjInfo sets DOTNET_HOST_PATH to an invalid value (the name of the currently executing program) when being run inside a .NET tool (e.g. fsdocs.exe rather than dotnet.exe) here

match System.Environment.GetEnvironmentVariable "DOTNET_HOST_PATH" with

This is because the default logic for dotnetPath assumes the current process is dotnet.exe but for a tool host like fsdocs.exe it isn't run by dotnet.exe, e.g.

In any case, the setting of DOTNET_HOST_PATH should be strongly protected by a try/finally? If used at all?

(Setting DOTNET_HOST_PATH to an invalid value confuses the heck out of FSharp.Compiler.Service.dll )

@baronfel
Copy link
Collaborator

I think DOTNET_HOST_PATH was required for some msbuild targets/props/shenanigans, so we should require it in some way, but I agree with you that the last-chance fallback of the app name isn't suitable for cases where the caller is in an AppHost and not the dotnet host. I think we can remove that last-chance and raise an exception in the init function if that's the case. That will require callers to set at least one of the valid detection variables, though, which ideally would be represented as an input to the function somehow...

@baronfel
Copy link
Collaborator

One hurdle here is that the .NET SDK doesn't forward any of the environment variables we look for on to local or global tools, like it does for other commands. It might be good to advocate for that feature, because as it stands, tools have no consistent way to detect the dotnet installation that's running them.

@baronfel
Copy link
Collaborator

Talking with some of the SDK team, the DOTNET_ROOT variable is only set if the install location isn't one of the well-known ones. So we should do probing if there's no DOTNET_ROOT.

@baronfel
Copy link
Collaborator

The final algorithm should look something like:

  • DOTNET_HOST_PATH
  • DOTNET_ROOT/DOTNET_ROOT(x86)
  • probe PATH
  • probe default install locations per-OS
  • fail hard if none found

@baronfel
Copy link
Collaborator

@dsyme this was released in 0.55.1

@baronfel
Copy link
Collaborator

@dsyme ok, 0.55.2 has a more sane strategy that doesn't touch DOTNET_HOST_PATH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants