Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

dotnet tool update does not throw but install when it is not installed #10205

Merged
merged 3 commits into from
Oct 22, 2018

Conversation

wli3
Copy link

@wli3 wli3 commented Oct 21, 2018

No description provided.

@livarcocc livarcocc added this to the 3.0.1xx milestone Oct 21, 2018
Copy link

@livarcocc livarcocc left a comment

Choose a reason for hiding this comment

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

I remember we have a bug asking for this. Could you find it and link it here?

@wli3
Copy link
Author

wli3 commented Oct 21, 2018

https://github.com/dotnet/cli/issues/9850
https://github.com/dotnet/cli/issues/9482
https://github.com/dotnet/cli/issues/9064

They are related issue. I talked to Kathleen, we hope this change + local tools could be enough to give scripting with tools a smooth experience. This is considered a breaking change(throw to no throw), and we hope to do minimal.

@wli3
Copy link
Author

wli3 commented Oct 21, 2018

@dotnet-bot test Linux x64 Release Build

@wli3 wli3 merged commit ce73d49 into dotnet:master Oct 22, 2018
@wli3 wli3 deleted the update-no-error-on-non-exists branch October 22, 2018 20:31
@AnthonySteele
Copy link

AnthonySteele commented Apr 16, 2019

Did this ship? I'm not seeing this behaviour.

I think that #9850 is correct. For scripting purposes, it is ideal to have a one-liner that can try to install a tool and

  • if the tool needs installing, install it.
  • If tool needs updating, update it.
  • If the tool is present and up to date, do nothing.

All success cases with a zero exit code.

  • Fail only if it cannot be installed or updated when necessary.

The idea that dotnet tool install -g sometool is required the first time that a script is run, and is an error the second time around makes scripting unnecessarily hard.

TL;DR: idempotence is good here.

@wli3
Copy link
Author

wli3 commented Apr 17, 2019

@AnthonySteele this will be in 3.0.1xx, still in preview.

I tired to be "idempotence" in the beginning, and also hope it can solve the concurrency and roll back issue(in the end we still use traditional three-phase commit). But it is odd in this situation. By idempotence, if I run the same install command twice, it should give me the same result. However, what if there is 1 year in between and the package is updated, should it install the updated version? Since the feed keeps changing, the command is not the only input.

So I chose "clear action". If it is an install command, there must be an install to happen. I do agree it made the scripting hard. This PR should achieve the one-liner to install a tool requirement by dotnet tool update.

@AnthonySteele
Copy link

AnthonySteele commented Apr 17, 2019

My first thought was that this would require a separate command for the new behaviour, e.g. dotnet tool upsert, dotnet tools checkinstall, or dotnet tool reachdesiredstate. But I am happy to use dotnet tool update for this scenario.

@KathleenDollard
Copy link

KathleenDollard commented Jun 28, 2019

To summarize this change:

dotnet tool update <PACKAGE_ID> [--version <VERSION>]
  • Download the NuGet package
  • If the tool exists, check the newly downloaded one is same or higher version
  • Install

The effect will be:

  • If the tool is not installed, install it.
  • If the tool is installed but a lower version, update it (uninstall/install).
  • If the tool exists and is the same version, repair it (uninstall/install). (Note 1)

The operation will return zero in all of these cases.

The operation will fail and return 1 if:

  • The newly downloaded package is a lower version than what is installed
  • Install fails.

Note 1: We made this choice because a) the download is generally the slow part and must be done for us to correctly check the version and b) there are several scenarios where this will be helpful (install issues) and c) we think tools should be designed so repeated installs are not problematic.

@burtonrodman
Copy link

sorry to post on an old thread, but why couldn't this be "idempotent when a version is specified" (either on the command line or in a manifest)?

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

Successfully merging this pull request may close these issues.

5 participants