Skip to content

Do not consider DOTNET_ROOT when finding dotnet#69010

Merged
jaredpar merged 1 commit intodotnet:mainfrom
jaredpar:fix-launch
Jul 17, 2023
Merged

Do not consider DOTNET_ROOT when finding dotnet#69010
jaredpar merged 1 commit intodotnet:mainfrom
jaredpar:fix-launch

Conversation

@jaredpar
Copy link
Member

This changes our dotnet launch code to not consider $DOTNET_ROOT as that is a apphost specific variable.

After discussion with the runtime team determined that only $PATH will be considered for finding dotnet.

dotnet/runtime#88754

This changes our `dotnet` launch code to not consider `$DOTNET_ROOT` as
that is a apphost specific variable.

After discussion with the runtime team determined that only `$PATH` will
be considered for finding `dotnet`.

dotnet/runtime#88754
@jaredpar jaredpar requested a review from a team as a code owner July 12, 2023 21:20
@ghost ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label Jul 12, 2023
@jaredpar jaredpar added this to the 17.8 milestone Jul 12, 2023
@333fred
Copy link
Member

333fred commented Jul 13, 2023

Reading the linked issue, I have no idea why not looking in DOTNET_ROOT is the right thing to do. If anything, I draw the opposite conclusion. Before we proceed with this, I think we need the guidance @richlander mentioned clearly documented.

@jaredpar
Copy link
Member Author

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables#dotnet_root-dotnet_rootx86

You can see there that DOTNET_ROOT is meant only for apphost scenarios. Our use of it in my previous change ended up breaking scenarios where it was being set in infra for apphost only but we were using it for normal launch. That discussion is what spawned that linked issue.

/// </summary>
/// <remarks>
/// See the following issue for rationale why only %PATH% is considered
/// https://github.com/dotnet/runtime/issues/88754
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd honestly prefer the docs link here, that was far clearer than this thread 🙂

@jaredpar
Copy link
Member Author

@dotnet/roslyn-compiler can i get one more review here, small change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants