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

Should snap installs automatically create symlinks? #13

Open
nagilson opened this issue May 7, 2024 · 16 comments
Open

Should snap installs automatically create symlinks? #13

nagilson opened this issue May 7, 2024 · 16 comments

Comments

@nagilson
Copy link

nagilson commented May 7, 2024

Based on the history of dotnet/sdk#10403.

When dotnet is installed via snap, the dotnet on PATH resolves to a realpath of /usr/bin/snap and not to a dotnet executable next to the sdk folder.

This breaks an assumption of the resolver that we can use realpath on dotnet on PATH to find the SDK. Thus, for a snap install to execute successfully, there are cases where manual intervention is required from the user to create a symbolic link to the actual .NET Path on their system, e.g.

ln -s /snap/dotnet-sdk/current/dotnet /usr/local/bin/dotnet.

Should we consider doing this automatically for snap installs?

@mateusrodrigues
Copy link
Member

Hi @nagilson! Thank you for bringing this to our attention.

Currently, there are no plans to create the /usr/local/bin/dotnet -> /snap/dotnet-sdk/current/dotnet symlink automatically, especially because we'll be relying even more on the snap's own entry point when we release vNext of the .NET snap.

The new snap will include an installer tool that lets users pick and choose the runtimes and SDKs they want installed on their systems, effectively allowing them to have more than one major version installed at once (which is currently not possible with the existing dotnet-sdk snap), and that feature relies heavily on the snap's ability to have a custom entry-point.

Are there any alternatives that we can discuss, such as creating the /etc/dotnet/install_location{_x64} files that point to the real path of the dotnet executable? This is a classic snap, so doing that should not be a problem.

@nagilson
Copy link
Author

Thanks for following up with my issue! :)

I think the alternative solution you propose is a better idea. Really, the symlink is more of a patch than solving the underlying issue, though both work well, I understand why you don't have plans to make that symlink automatically now. I'm not sure on the benefit of /etc/ -- in this case is it just a convention or are there aspects of this folder that make it for example get added to the path by default by snap, etc?

I don't have a ton of knowledge/authority in this space, but @richlander does and I'm curious to hear his take.
cc @baronfel as well

@mateusrodrigues
Copy link
Member

Well, if I understand correctly, the /etc/dotnet/install_location{_x64} files are already an established .NET convention on Linux/macOS (see here) and our .deb packages currently implement it.

Since that's a well-known location for getting the absolute path of the dotnet executable, the snaps can also follow the same convention.

For reference, I currently have the .NET .debs installed on my machine, this is what I see on both of these files:

$ cat /etc/dotnet/install_location
/usr/lib/dotnet
$ cat /etc/dotnet/install_location_x64 
/usr/lib/dotnet

Curious to hear the others takes as well 😃

@leecow
Copy link

leecow commented Jun 19, 2024

Since that's a well-known location for getting the absolute path of the dotnet executable, the snaps can also follow the same convention.

@mateusrodrigues - are you suggesting that snaps also utilize /etc/dotnet/install_location to mark an installed snap's entrypoint?

@mateusrodrigues
Copy link
Member

@leecow not the snap's entrypoint, but the actual location of the .NET installation. It would contain something like /var/snap/dotnet/common/dotnet, which is where we'll be placing the .NET files in the new snap.

Since the issue was raised because the snap's own executable on PATH does not link to the dotnet executable directly (see snippet below), /etc/dotnet/install_location would effectively be a place where you could find the path to the actual .NET location.

$ ls -l $(which dotnet)
lrwxrwxrwx 1 root root 13 Jun 14 09:46 /snap/bin/dotnet -> /usr/bin/snap

@leecow
Copy link

leecow commented Jun 20, 2024

Seems like a good solution to me but would want @richlander, @baronfel or @marcpopMSFT to weigh in.

@baronfel
Copy link

baronfel commented Jun 20, 2024

Yes, that should be an acceptable solution. It would conflict with a .NET installation from a deb pkg but I don't think that's necessarily a bad thing - we generally don't recommend that users mix installation methods anyway.

@richlander
Copy link

There are two reasons why that file could be used:

  • Discover path to the dotnet launcher
  • Discover root of the dotnet directory hive

I don't know much about snaps, but I'm wondering if they would break the second use case. It isn't clear to me that it would be a deal killer, but good to understand what the behavior will be.

@mateusrodrigues
Copy link
Member

I don't know much about snaps, but I'm wondering if they would break the second use case.

@richlander IIUC the "dotnet directory hive" is the .NET root folder that contains the dotnet executable + the host, shared, sdk, and so on directories, correct?

If so, the fact that we're shipping with snaps shouldn't matter, because the behavior of /etc/dotnet/install_location is essentially the same as the deb pkg one. So, one can assume that:

  • The dotnet launcher lives in $(cat /etc/dotnet/install_location)/dotnet
  • The hive lives in cat /etc/dotnet/install_location

Being more specific on the .NET snap case, /etc/dotnet/install_location would contain /var/snap/dotnet/common/dotnet, where:

  • /var/snap/dotnet/common... is a root-writable directory that belongs to the dotnet snap.
  • .../dotnet is a directory that contains the .NET directory hive.

Hope this makes sense :)

@mateusrodrigues
Copy link
Member

Also, if we all agree with this approach, I can work on the PR for this change 😃

@mateusrodrigues
Copy link
Member

So I have found the culprit in the GetDotnetExeDirectory function:
https://github.com/dotnet/sdk/blob/a2de2d815a5fd9d51a56d686959589660f73b507/src/Resolvers/Microsoft.DotNet.NativeWrapper/EnvironmentProvider.cs#L76C17-L81C18

@baronfel @nagilson Ideally, I'd like to trigger this code path, so that I'm able to attach a debugger and work further on this function. I tried doing that through numerous dotnet CLI invocations, but none seem to go through GetDotnetExeDirectory, they always exit without ever hitting the break-point I set right at the beginning of it.

I'm a bit unfamiliar with the Resolver itself, so I would appreciate any pointers on how I could achieve that, or if there's a better way even :)

@nagilson
Copy link
Author

nagilson commented Jul 18, 2024

@mateusrodrigues Thank you so much for your willingness to help investigate the issue. 😄 I apologize for our long delay to get back to you as we have been focused on a lot of other things atm. Great eye on spotting the GetDotnetExeDirectory function!

I was able to attach a debugger to the GetDotnetExeDirectory function. Here is one way to do it.
Run dotnet --debug build --arch x86. (Other architecture values are OK too.)
This will trigger a breakpoint in the architecture resolution logic, which then triggers RID resolution logic, which triggers GetDotnetExeDirectory. There are other ways to trigger it too, this is just a simpler one :)

I hope that answers your question? Btw, you need to also run the commands shown above after build and attach to the process ID as in my screenshot to get it to work. I show it in VS and using the windows script, but we have a shell script equivalent for linux. Hopefully this is helpful.
image

@mateusrodrigues
Copy link
Member

Hey @nagilson! Thank you so much, that was very helpful!

I noticed that the realpath logic sits inside the if statement on line 70 that is getting evaluated to false with the invocation dotnet --debug build --arch x86:

...
            if (string.IsNullOrEmpty(dotnetExe) || !Path.GetFileNameWithoutExtension(dotnetExe)
                    .Equals(Constants.DotNet, StringComparison.InvariantCultureIgnoreCase))
...

Going a little bit on a tangent here, but when is Path.GetFileNameWithoutExtension(dotnetExe) not going to be dotnet? Is this in the case when an application bundles the runtime and the invoked binary is not dotnet? If this is the (only) point of the if statement, is this scenario debuggable?

I was able to go into the if statement by inverting the logic temporarily anyway, so just asking out of curiosity 😄

@nagilson
Copy link
Author

That's a good question, honestly I can't think of any scenario where that would be the case. That might just be a safe-guard. @JanKrivanek are you aware of a case where Path.GetFileNameWithoutExtension(dotnetExe) is not dotnet?

@JanKrivanek
Copy link

IIRC it was for selfcontained executables and sdk tests (code is hosted in runner).

@nagilson
Copy link
Author

nagilson commented Jul 22, 2024

Thanks @JanKrivanek! That makes sense.

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

No branches or pull requests

6 participants