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

fix(upgrade): Implement compatible/incompatible/pinned behavior #804

Merged
merged 18 commits into from
Sep 14, 2022

Conversation

epage
Copy link
Collaborator

@epage epage commented Sep 14, 2022

This is part of an experiment on the path to upstreaming into cargo
that asks "what if we started from scratch, what
would cargo update look like?". Besides getting us to think outside
the box, I hope that we can deprecate cargo update and replace it with
cargo upgrade. To build up a case to justify the ecosystem churn, we
need to get run time with the expected behavior to see how it works.

The new behavior is

  • Only upgrade compatible dependencies by default
  • Users may pass in --incompatible or --pinned to upgrade others to incompatible versions
  • Users may pass in --compatible false to exclusive upgrade incompatible versions when used with another flag

This is disruptive to existing cargo-edit users though. So why
move away from the traditional behavior?

Reason 1: The new model for cargo upgrade is "what if cargo update updated the
manifest to match the lock file". cargo update only deals with
compatible upgrades and it would be disruptive to that common workflow
to deal with incompatible upgrades (ie exacerbate ecosystem churn)

Reason 2: There are several workflows we are trying to cover that
involve needing to upgrade only a subset of dependencies. Before, we
had been working under the assumption of incompatible-by-default which
required ways to opt-out which have been a bit awkward.
Compatible-by-default provides a simple way of supporting most documented
workflows

  • Application authors who want the latest features and bug fixes can get
    it with a simple cargo upgrade -i
  • Users who want to defer breaking changes can do so by leaving off
    --incompatible or having it but using it with --exclude
  • For maintainers who want isolated PRs, they can either run cargo upgrade and then cargo upgrade --incompatible or, if they don't want data races, can do cargo upgrade and then cargo upgrade --incompatible --compatible false
  • Library authors who want to keep their compatible versions low will run cargo upgrade --incompatible --compatible false
  • Checking for any upgrade is cargo upgrade --locked --incompatible and checking for only incompatible upgrades is cargo upgrade --locked --incompatible --compatible false

In getting here, we considered several other alternatives, including

  • Compatible by default without an opt-out. This was what this PR initially did until I wrote p the use cases and realized there was a gaping hole. The new behavior has the appearance of this old behavior but with an opt-out
  • Provide --no-compatible which feels out of place and awkward to tack
    on
  • Make --compatible the default if no flags are specified but
    otherwise make the flags additive (cargo upgrade would upgade
    compatible but cargo upgrade --incompatible wouldn't and would
    require cargo upgrade --incompatible --compatible). This feels like
    it'd be too confusing
  • Make --compatible the default but make all flags mutually exclusive.
    This makes the specifying-a-specific case short but requires running
    2-3 times to cover multiple cases. Road humps in the users workflow
    like this are frustrating.

Other reasons I went with the current approach

  • While it adds some awkward ceremony to some cases, it works well for composing in scripts or with aliases
  • Its consistent with --recursive
  • Cargo already uses optional values else where (cargo check --features)
    • Its safest when there are no positional arguments which cargo usually avoids (cargo add being an exception)

Other behavior change

  • Pinned dependencies will now get compatible version upgrades
  • The dependency table go some clean up
  • The --help output got some clean up
  • Maintaining the version req syntax now has higher precedence than compatible upgrades (since we don't support upgrading all version req syntaxes)

This is mostly a cleanup to make it easier to detect when we have a new
incompatible but it did have a slight behavior change.
On the path to upstreaming into cargo, this is part of an experiment
that asks "what if we started from scratch, what
would `cargo update` look like?".  Besides getting us to think outside
the box, I hope that we can deprecate `cargo update` and replace it with
`cargo upgrade`.  To build up a case to justify the ecosystem churn, we
need to get run time with the expected behavior to see how it works.

This is disruptive to existing cargo-edit users though.  So why
move away from the traditional behavior?

Reason 1: The new model for `cargo upgrade` is "what if `cargo update` updated the
manifest to match the lock file".  `cargo update` only deals with
compatible upgrades and it would be disruptive to that common workflow
to deal with incompatible upgrades.

Reason 2: There are several workflows we are trying to cover that
involve needing to upgrade only a subset of dependencies.  Before, we
had been working under the assumption of incompatible-by-default which
required ways to opt-out which have been a bit awkward.
Compatible-by-default provides a simple way of supporting most documented
workflows
- Application authors who want the latest features and bug fixes can get
  it with a simple `cargo upgrade -i`
- Users who want to defer breaking changes can do so by leaving off
  `--incompatible` or having it but using it with `--exclude`
- For maintainers who want isolated PRs, so long as someone has done
  `cargo upgrade`, merged it, and then done `cargo upgrade
  --incompatible`, they'll get that.

The workflows we are underserving still are:
- Library authors who want to keep their compatible versions low will
  need to go through awkwards steps of `cargo upgrade --dry-run` and
  then re-running with `-p` for each incompatible dependency
- Checking for upgrades is easy but checking for only incomptible
  upgrades will be a pain

Our alteratives to these haven't been great though
- Provide `--no-compatible` which feels out of place and awkward to tack
  on
- Make the flags `--compatible=<true|false>`, etc which would be
  cumbersome to specify.  We could make them `true` if no value is
  specified.  This makes things feel more consistent but its still
  cumbersome to have incompatible-only upgrades (`--compatible=false`
  instead of `--no-compatible`).  This would fit in with
  `--recursive=<true|false>`.
- Make `--compatible` the default if no flags are specified but
  otherwise make the flags additive (`cargo upgrade` would upgade
  compatible but `cargo upgrade --incompatible` wouldn't and would
  require `cargo upgrade --incompatible --compatible`).  This feels like
  it'd be too confusing
- Make `--compatible` the default but make all flags mutually exclusive.
  This makes the specifying-a-specific case short but requires running
  2-3 times to cover multiple cases.  Road humps in the users workflow
  like this are frustrating.
The advantage of this route
- All use cases are covered
- No surprises
- Users can define aliases to get the behavior they want
This reduces noise in the note column and has the unchanged summary
better reflect what would be changed with flags.
With all of the recent changes, we've not re-evaluated what rows are
shown or what colors are used for the columns.
@epage epage merged commit 68a4e3c into killercup:master Sep 14, 2022
@epage epage deleted the exp branch September 14, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant