dotnet / install-scripts

MIT License
145 stars 76 forks source link

dotnet-install.sh uses the wrong name when dot-sourced #520

Closed JayBazuzi closed 1 month ago

JayBazuzi commented 4 months ago

Make a script my-script.sh:

curl -sSL https://dot.net/v1/dotnet-install.sh -O
. ./dotnet-install.sh --help

When run it prints out:

Usage:
       # Install a .NET SDK of a given Quality from a given Channel
       my-script.sh [-c|--channel <CHANNEL>] [-q|--quality <QUALITY>]
       # Install a .NET SDK of a specific public version
       my-script.sh [-v|--version <VERSION>]
       my-script.sh -h|-?|--help

my-script.sh is a simple command line interface for obtaining dotnet cli.
...

Note that the help says my-script.sh instead of dotnet-install.sh.

missymessa commented 2 months ago

@mairaw do you know who owns this now?

mairaw commented 2 months ago

Adding @MichalPavlik and @YuliiaKovalova

MichalPavlik commented 2 months ago

@baronfel, I think the problem is caused by this line: https://github.com/dotnet/install-scripts/blob/c4501ac2f3714e080e425ce499a0f077ab15a0b0/src/dotnet-install.sh#L1738

Is this behavior expected?

baronfel commented 2 months ago

It's not bad in the sense that it's not harming anything, but generally you wouldn't dot source this script. Is not that kind of script. We can change this to hard-code the name but it's kind of a mixed bag, because then you're incorrect if the users just rename the script.

JayBazuzi commented 2 months ago

generally you wouldn't dot source this script

Right here it says it is intended to be dot-sourced: https://github.com/dotnet/install-scripts/blob/c4501ac2f3714e080e425ce499a0f077ab15a0b0/src/dotnet-install.sh#L1860

JayBazuzi commented 2 months ago

One option is to stop supporting dot-sourcing all together.

An approach I have used elsewhere is to add an option to suppress all other stdout and only print the path. Then the caller can do:

export PATH="$( ./dotnet-install.sh --print-path-only )":"$PATH"
baronfel commented 2 months ago

There is no portable way to get the name of a script that is sourced - you have to do bash- or zsh- or other-shell-specific code to get that. I'm not convinced that this is a high priority to be 100% correct in all scenarios. We should either ignore this side effect or hard-code the dotnet-install.sh name.

YuliiaKovalova commented 2 months ago

There is no portable way to get the name of a script that is sourced - you have to do bash- or zsh- or other-shell-specific code to get that. I'm not convinced that this is a high priority to be 100% correct in all scenarios. We should either ignore this side effect or hard-code the dotnet-install.sh name.

I would vote for hardcoding the name. I don't envision a change of it .

MichalPavlik commented 2 months ago

Hardcoding was my first thought. I don't think we will change the name, and I don't see a reason why would customers change the script name...

JayBazuzi commented 2 months ago

I don't see a reason why would customers change the script name

Because the instructions are basically "download a file and run it", they might download it to any name of their choosing. If they download in a browser, the browser will add a (1) or similar if there's a conflict. If they copy it in Explorer, it will get Copy of.

That said, I think hard-coding the name and being wrong in these cases is reasonable.

MichalPavlik commented 2 months ago

I didn't think too much about browsers and Explorer as this script is designed to be used in CI. Anyway, we will hardcode the name :)