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

dotnet tool install should return exit code 0 if tool is already installed #9500

Closed
natemcmaster opened this issue Jun 13, 2018 · 45 comments
Closed

Comments

@natemcmaster
Copy link
Contributor

I'm trying to write a script that will install global tools if necessary. I would like to just run "dotnet tool install", but this returns exit code 0 if the tool is already present, so I have to first execute dotnet tool list and grep the output.

Expected behavior

Calling dotnet tool install should no-op AND exit code 0 if the tool is already installed

Actual behavior

Running dotnet tool install twice fails scripts because the second time it runs it will exit code 0

PS> & dotnet tool install --tool-path "$(pwd)/.tools" sleet --version 2.3.25 --add-source https://api.nuget.org/v3/index.json
You can invoke the tool using the following command: sleet
Tool 'sleet' (version '2.3.25') was successfully installed.
PS> $lastexitcode
0
PS>  & dotnet tool install --tool-path "$(pwd)/.tools" sleet --version 2.3.25 --add-source https://api.nuget.org/v3/index.json
Tool 'sleet' is already installed.
PS>  $lastexitcode
1

The result of this is that I have write more complicate code like this:

if (& $dotnet tool list --tool-path "$PSScriptRoot/.tools" | Select-String "sleet") {
    Write-Host -f Yellow 'Skipping install of sleet. It''s already installed'
}
else {
    Invoke-Block { & $dotnet tool install --tool-path "$PSScriptRoot/.tools" sleet --version 2.3.25 --add-source https://api.nuget.org/v3/index.json }
}

Environment data

dotnet --info output:

.NET Core SDK (reflecting any global.json):
 Version:   2.1.300
 Commit:    adab45bf0c

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17134
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\dev\aspnet\Universe20\.dotnet\sdk\2.1.300\

Host (useful for support):
  Version: 2.1.0
  Commit:  caa7b7e2ba

.NET Core SDKs installed:
  2.1.300 [C:\dev\aspnet\Universe20\.dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.0 [C:\dev\aspnet\Universe20\.dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.0 [C:\dev\aspnet\Universe20\.dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.0 [C:\dev\aspnet\Universe20\.dotnet\shared\Microsoft.NETCore.App]
@wli3
Copy link

wli3 commented Jun 13, 2018

Another side of the coin will be "I don't know if it will trigger asset download if I run this command". I slightly prefer current approach. But note, both approach will cause another side unhappy.

@natemcmaster
Copy link
Contributor Author

Another side of the coin will be "I don't know if it will trigger asset download if I run this command"

Can you explain more what you mean? I'm not sure I understand the concern. It seems clear to me that when calling "dotnet tool install" I may download some assets.

@wli3
Copy link

wli3 commented Jun 13, 2018

related issue https://github.com/dotnet/cli/issues/9064

If exists return 0 has extra magic. The installation is triggered or not depends on the existing disk state. The current approach i think is more strict which is better for script since it is less likely to swallow error in script.

Also, "dotnet tool install" is not idempotent at all. It will download a different version if there is new version in the feed. In this case, what "dotnet tool" install should do when this happens and what the user will expect? And the expectation will also be different for scripting and user typing. The current strict approach will move this question to the user which I think is better than mismatch expectation.

@natemcmaster
Copy link
Contributor Author

This is overly strict and IMO a bad default. The error code currently issued (exit code 1) is the same error code issued for legitimate errors, so I can't just swallowed the error and move on.

You've brought up a good point though -- what to do about versions. IMO it would be good to look at what other tools do, e.g.

brew install
apt-get install
npm install

From my experience with these, running install (without a version) will return exit code 0 and will not attempt to upgrade if the tool is already installed. Running install with a version will attempt to run and upgrade (or downgrade) to bring the installed tool into alignment with the value of --version

cc @KathleenDollard @richlander

@tmds
Copy link
Member

tmds commented Jun 14, 2018

npm install returns 1 when it can't change to the specified version.
npm install returns 0 when the version is already installed.

npm install without a version specified will update to the latest version when the package is already installed.

This matches my expectations as a user.

@KathleenDollard
Copy link

Will and I talked about this.

We do not currently determine what the version of a tool is. Install fails if it is found, update uninstalls and reinstalls it regardless. And, we're concerned about users being able to differentiate between "we didn't do anything" and "we think you are in a broken state".

All of this relates to a decision we're working on regarding whether we use a manifest for repo tools (expected) and whether we retrofit that approach to global tool or have two different internal approaches. If we change global tools to maintain manifest, it will be easier for users to understand what they have requested, for us to understand the version that is present, etc.

So, this is on hold for a bit.

@ErikSchierboom
Copy link
Contributor

npm install returns 1 when it can't change to the specified version.
npm install returns 0 when the version is already installed.

npm install without a version specified will update to the latest version when the package is already installed.

This matches my expectations as a user.

I agree!

@wli3
Copy link

wli3 commented Oct 23, 2018

Please continue comment this issues if you think dotnet/cli#10205 cannot solve the problem of your scenario and I will reopen it

@wli3 wli3 closed this as completed Oct 23, 2018
@cottsak
Copy link

cottsak commented Dec 17, 2018

It's so frustrating not to get the common convention of a 0 exit when the tool installs or is already present.

@KathleenDollard
Copy link

@cottsak Is there a reason using dotnet update instead of dotnet install is a bad choice when you want to either install or update depending on whether the tool is on the box?

@cottsak
Copy link

cottsak commented Jan 30, 2019

@KathleenDollard It's not a "bad" choice. But IMO, it's less than ideal. When scripting something it would be great to simply use dotnet install as an idempotent command meaning that it will implicitly "install" if not present, or return 0 if it's already installed. It just makes life easier, and seems to be an established convention.

@sandcastle
Copy link

I honestly think if a survey was run (focused on ux), it would clearly show that the current behaviour is not preferred. It's inconsistent with so many other experiences like the ones @natemcmaster mentioned above.

No joy is being sparked with the current behaviour. 😂

@wli3
Copy link

wli3 commented Apr 29, 2019

Replied here https://github.com/dotnet/cli/issues/11259#issuecomment-487447234

(back to folding clothes)

@Shoh
Copy link

Shoh commented Jun 5, 2019

I don't understand why a flag can't be added, like it was suggested in dotnet/cli#11259, and just be done with it? It's not removing or changing any default behavior (so no users will be surprised) and it will satisfy the needs of all those that want the option to not throw an error when installing a tool that has already been installed. Using dotnet update is NOT a proper solution for it. What if I don't want to update a tool every time a new version is released? Because it's usually not a good idea to update right away for most things (for obvious reasons).

Please just add the flag and be done with it so we can all continue with our work.

@wli3
Copy link

wli3 commented Jun 5, 2019

@Shoh i hope to understand your scenario better. You do not want to update to the latest version, but if according to your suggestion, with the new flag, the dotnet install will still install the latest version.

We added "--version" option to dotnet tool update (in 3.0.100 preview). If you want to pin to an older version, will always run "dotnet tool update mytool --version PINNED_VERSION" works?

@wli3
Copy link

wli3 commented Jun 6, 2019

I created https://github.com/dotnet/cli/issues/11494 to discuss this specific option

@Shoh
Copy link

Shoh commented Jun 6, 2019

My use case (and I'm sure for many others) is for CI scripts. I should be able to put dotnet tool install (inserttoolhere) and not worry about it failing on subsequent builds. IMO, it doesn't feel right to use update when you're actually looking to just install a specific version.

Since the current behavior is to fail with exit code 1 when a tool is already installed, IMO, the safest solution is to add a flag to disable the "exit with error if tool is installed" behavior for users that opt to use the flag.

I thought either of the suggestions that were in dotnet/cli#11259 were fine (--slient|-s or --no-errors|-ne) but I'd be okay with something like --skip-installed.

@Shoh
Copy link

Shoh commented Jun 6, 2019

I will say that I agree with the others though, that the current behavior is not normal behavior and "gracefully failing" (i.e. exit with code 0 if already installed) should be the default behavior. If anything, the current behavior should be "opted" in, if a user wants it. i.e. something like dotnet tool install sometool --error-if-installed

I was just providing an option to satisfy the needs of those that want normal behavior without breaking or surprising any users that have gotten used to or have some reason to prefer the current behavior.

I am personally okay with including a flag to get the behavior that I want but I would prefer the current behavior to be "fixed" to something more normal. (If that was an option on the table) Again, ultimately, I'm okay with whatever is decided is best for the majority.

@chrisgilbert
Copy link

This broke our makefile, which tries to install 4 tools so we can use them to do various things - lambda, cake, and unit test coverage for an AWS lambda project we have.

If any one of the tools are installed, it will exit with a failure, and not install the rest (that's the normal behaviour of make).

That's not expected behaviour to me - I went searching for a way around it, and found this issue.

Since there's not a flag to change behaviour, to fix it we would either have to parse the exit text (maybe what I'll choose), or just ignore failures for any reason, and carry on with the rest of the installs (not ideal).

I can't presume exit code 1 is "success" though, because that's what it gives for DNS lookup failure, unable to connect to a nuget server, etc too.

Here's our example Makefile:

install-dependencies:
		dotnet tool install --global Amazon.Lambda.Tools --version 3.2.3
		dotnet tool install --global Cake.Tool --version 0.33.0
		dotnet tool install --global dotnet-reportgenerator-globaltool --version 4.2.1
		dotnet tool install --global altcover.global --version 5.3.675

@dquist
Copy link

dquist commented Sep 28, 2019

Ran into this problem literally the first hour of trying to set up a CI pipeline for my first .net core 3 project. This behavior is so... disheartening and is bad DX.

The added upgrade option is nice I guess, but it still downloads the tool every time making it impossible to utilize pipeline caching.

All I want to do is install the tool if it doesn't exist without having to download any extra binaries. Why does this have to be so complicated?

@msftgits msftgits transferred this issue from dotnet/cli Jan 31, 2020
@msftgits msftgits added this to the Discussion milestone Jan 31, 2020
foxmoat added a commit to foxmoat/mslearn-tailspin-spacegame-web that referenced this issue Feb 11, 2020
@rmsy
Copy link

rmsy commented Jul 10, 2020

Is there any chance this is going to be re-visited? It is a little ridiculous that dotnet tool install returns 1 when the tool already exists. At the very least, would you consider adding a flag as proposed in #10242? This is inconsistent with the behavior of every other package manager out there, that I know of at least.

@gravufo
Copy link

gravufo commented Jul 14, 2020

How many people will have to complain here for Microsoft to stop burying its head in the ground and scream that everything is fine?

Edit: I saw that dotnet tool update was changed to have the kind of behavior we are expecting here. In my opinion, Microsoft still has it completely upside down. How does it make sense for a command called update to install if the tool doesn't exist whereas the install command fails if the tool already is installed??

@kylecooleylba
Copy link

Posting here because I agree with above comments should at least have a flag to return 0 if installed

@zoechi
Copy link

zoechi commented Jul 29, 2020

Makes it a pain in ansible

@rmsy
Copy link

rmsy commented Jul 29, 2020

@gravufo The dotnet tool update command "uninstalls and reinstalls a tool, effectively updating it." That is quite a joke and definitely not what an idempotent install command should do, so even that command can't serve as a proper substitute. The reality is that dotnet tool install should be a graceful no-op if the tool is already installed (as everyone here seems to agree, except for MSFT), and dotnet tool update should only update if there is a newer version determined to be available (but that's a different issue for a different thread).

fsouza added a commit to fsouza/dotfiles that referenced this issue Sep 12, 2021
dotnet is funny. `tool install` fails if the tool is already installed,
and the upstream issue (dotnet/sdk#9500) recommends using `update`
instead of `install`.
@bsolovij-ebsco
Copy link

Any movement on this?

@riverar
Copy link
Contributor

riverar commented Nov 4, 2021

Chiming in here, update does not appear to be a valid workaround for preview versions, which require an explicit version be specified.

@NaZaRKIN123
Copy link

A typical Microsoft way of doing things.

@RDavis3000
Copy link

This is unintuitive
This is also undocumented (https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-tool-install)
This is universally rejected by MS customers in this thread
This is telling you that we don't need separate install/update commands
This is telling you that in CI/CD world we want UPSERT everywhere and parameters for version targeting etc

@cyril265
Copy link

cyril265 commented Sep 12, 2022

How to annoy customers 101

Possible workaround without --update (reinstalling package every time). Remove --toolpath $NUGET_PACKAGES_DIRECTORY if you are installing it in the default directory.

(( $(dotnet tool list --tool-path $NUGET_PACKAGES_DIRECTORY | grep 'dotnet-reportgenerator-globaltool' | wc -l)==0 )) && \
dotnet tool install dotnet-reportgenerator-globaltool --tool-path $NUGET_PACKAGES_DIRECTORY

@isidore
Copy link

isidore commented Jan 6, 2023

dotnet tool install -g <tool> | echo "already installed"

@juescuder
Copy link

@isidore solution works as a charm. Thanks.

@HannahVernon
Copy link

dotnet tool install -g <tool> | echo "already installed"

That's brilliant. Except I changed the echo output to "dotnet install is dumb"

@dsplaisted
Copy link
Member

As an update, we're thinking of changing the install behavior so that it will not fail if the tool is already installed. This will mean that dotnet tool install will pretty much work the same way dotnet tool update already does. Because of that, over time we might deprecate dotnet tool update.

We would likely add options to allow the old behavior, for example something like --fail-if-already-installed and --update-only.

@augustoproiete
Copy link

That is good news @dsplaisted !

Expectations of future behavior that I believe most agree on:

  • dotnet tool install without a --version specified will try to update to the latest version when the tool is already installed, return 0 exit code if latest version already installed or if upgrade is successful, 1 otherwise

  • dotnet tool install with a --version specified will try to upgrade or downgrade to the specified version when the tool is already installed but with a different version than specified, return 0 exit code if specified version already installed or if upgrade or downgrade is successful, 1 otherwise

@dsplaisted
Copy link
Member

@augustoproiete I'm not sure what the behavior should be if the new version is a downgrade. If a higher version is already installed maybe something depends on the higher version and would fail with the downgrade. Maybe we should error on downgrades by default and have an --allow-downgrade option which would allow it to succeed.

@riverar
Copy link
Contributor

riverar commented Mar 29, 2023

I do not agree with the proposed downgrade behavior. With --version present, a downgrade should be considered an error without a --force.

@augustoproiete
Copy link

augustoproiete commented Mar 29, 2023

@dsplaisted @riverar Following the principle of deterministic builds, if the --version is present it means my process expects that particular version every time it runs, consistently.

Thus, I'd argue that should be the other way around e.g. A --do-not-downgrade option, which would prevent a downgrade when a higher version is already installed.

edit: Also, the name of the option is --version... If option were called --minimum-version, then sure... I'd agree a downgrade would have to be --forced

@LevYas
Copy link

LevYas commented Jan 18, 2024

This is still a needed feature

@baronfel
Copy link
Member

baronfel commented Jan 18, 2024

This should be released in 8.0.200 next month!

@nagilson
Copy link
Member

Closing this as completed by @JL03-Yue, let us know if you have any feedback! 🥳

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

No branches or pull requests