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

Stabilize install-upgrade. #7560

Open
wants to merge 1 commit into
base: master
from

Conversation

@ehuss
Copy link
Contributor

ehuss commented Nov 4, 2019

Tracking issue: #6797

This stabilizes the install-upgrade feature, which causes cargo install to reinstall a package if it appears to be out of date, or exit cleanly if it is up-to-date.

There are no changes from -Zinstall-upgrade. See the old unstable docs for a refresher on the details of what it does.

This also stabilizes the following changes:

  • --version no longer allows an implicit version requirement like 1.2. It must be either contain all 3 components (like 1.2.3) or use a requirement operator (like ^1.2). This has been a warning for a very long time, and is now changed to a hard error.
  • Added --no-track to disable install tracking.

Motivation

I just personally prefer this behavior, and it has been requested a few times in the past. I've been using it locally, and haven't run into any issues. If this goes into 1.41, then it will release on Jan 30, about 10 months since it was added in #6798.

Concerns

Regarding some of the concerns I had:

  • Is it tracking the correct set of information?

    I'm satisfied with the current set. It also tracks, but does not use, the version of rustc and the version specified in the --version flag, in case we ever want to use that in the future. It is also designed to be backwards and forwards compatible, so more information can be added in the future. I think the current set strikes a good balance of tracking the really important things, without causing unnecessary rebuilds.

  • Method to upgrade all outdated packages?

    This can always be added as a new flag or command in the future, and shouldn't block stabilization.

  • Should --no-track be kept? Does it work correctly?

    I kept it. It's not too hard to support, and nobody said anything (other than maybe using a less confusing name).

  • Should this be the default? Should there be a way to use the old behavior?

    I like it as the default, and don't see a real need for the old behavior. I think we could always bring back the old behavior with a flag in the future, but I would like to avoid it for simplicity. There is also the workaround of which foo || cargo install foo.

Closes #6797.
Closes #6727.
Closes #6485.
Closes #2082.

@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Nov 4, 2019

r? @Eh2406

(rust_highfive has picked a reviewer for you, use r? to override)

@ehuss

This comment has been minimized.

Copy link
Contributor Author

ehuss commented Nov 4, 2019

@rfcbot fcp merge

@RalfJung

This comment has been minimized.

Copy link
Member

RalfJung commented Nov 4, 2019

I assume with -f it still always reinstalls no matter what?

@ehuss

This comment has been minimized.

Copy link
Contributor Author

ehuss commented Nov 4, 2019

I assume with -f it still always reinstalls no matter what?

Correct.

@rfcbot

This comment has been minimized.

Copy link
Collaborator

rfcbot commented Nov 4, 2019

Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@lzutao

This comment has been minimized.

Copy link
Contributor

lzutao commented Nov 11, 2019

@rfcbot

This comment has been minimized.

Copy link
Collaborator

rfcbot commented Nov 11, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.