-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Comments
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. |
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. |
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. |
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.
From my experience with these, running |
This matches my expectations as a user. |
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. |
I agree! |
Please continue comment this issues if you think dotnet/cli#10205 cannot solve the problem of your scenario and I will reopen it |
It's so frustrating not to get the common convention of a 0 exit when the tool installs or is already present. |
@cottsak Is there a reason using |
@KathleenDollard It's not a "bad" choice. But IMO, it's less than ideal. When scripting something it would be great to simply use |
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. 😂 |
Replied here https://github.com/dotnet/cli/issues/11259#issuecomment-487447234 (back to folding clothes) |
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 Please just add the flag and be done with it so we can all continue with our work. |
@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? |
I created https://github.com/dotnet/cli/issues/11494 to discuss this specific option |
My use case (and I'm sure for many others) is for CI scripts. I should be able to put 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 ( |
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 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. |
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:
|
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 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? |
Is there any chance this is going to be re-visited? It is a little ridiculous that |
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 |
Posting here because I agree with above comments should at least have a flag to return 0 if installed |
Makes it a pain in ansible |
@gravufo The |
Any movement on this? |
Chiming in here, |
A typical Microsoft way of doing things. |
This is unintuitive |
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 install -g <tool> | echo "already installed" |
@isidore solution works as a charm. Thanks. |
That's brilliant. Except I changed the echo output to "dotnet install is dumb" |
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 We would likely add options to allow the old behavior, for example something like |
That is good news @dsplaisted ! Expectations of future behavior that I believe most agree on:
|
@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 |
I do not agree with the proposed downgrade behavior. With |
@dsplaisted @riverar Following the principle of deterministic builds, if the Thus, I'd argue that should be the other way around e.g. A edit: Also, the name of the option is |
This is still a needed feature |
This should be released in 8.0.200 next month! |
Closing this as completed by @JL03-Yue, let us know if you have any feedback! 🥳 |
If I understand correctly, and based on what I'm seeing with on 8.0.402, the package is just reinstalled every time, even if the version is the same? It would've been nice to have it be idempotent like the issue author was requesting. Don't suppose this will change things now but current behavior feels wasteful of bandwidth and cpu when all we're doing is checking versions. |
I agree with that. We have to use other workarounds as other parallel running processes which depend on dotnet tools fail when the update reinstalls the tools. |
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 installedActual behavior
Running dotnet tool install twice fails scripts because the second time it runs it will exit code 0
The result of this is that I have write more complicate code like this:
Environment data
dotnet --info
output:The text was updated successfully, but these errors were encountered: