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

cargo update prints Adding + Removing lines when multiple versions of the same crate are updated #9408

Open
Nemo157 opened this issue Apr 24, 2021 · 4 comments
Labels
C-bug Category: bug Command-update S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@Nemo157
Copy link
Member

Nemo157 commented Apr 24, 2021

Problem
The more concise Updating lines are not printed if multiple versions of the same crate are updated in one command

Setup

> cargo new foo && cd foo
> echo 'bs58_02 = { package = "bs58", version = "0.2.0" }' >> Cargo.toml
> echo 'bs58_03 = { package = "bs58", version = "0.3.0" }' >> Cargo.toml
> cargo generate-lockfile
Updating crates.io index

> cargo update -p bs58:0.2.5 --precise 0.2.0
Updating crates.io index
Updating bs58 v0.2.5 -> v0.2.0

> cargo update -p bs58:0.3.1 --precise 0.3.0
Updating crates.io index
Updating bs58 v0.3.1 -> v0.3.0

Current

> cargo update
Updating crates.io index
Removing bs58 v0.2.0
Removing bs58 v0.3.0
Adding bs58 v0.2.5
Adding bs58 v0.3.1

Expectation

> cargo update
Updating crates.io index
Updating bs58 v0.2.0 -> v0.2.5
Updating bs58 v0.3.0 -> v0.3.1

Notes

Output of cargo version: cargo 1.53.0-nightly (65d57e6f3 2021-04-04)

@Nemo157 Nemo157 added the C-bug Category: bug label Apr 24, 2021
@heisen-li
Copy link
Contributor

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 17, 2023
Github action to periodically `cargo update` to keep dependencies current

Opens a PR periodically with the results of `cargo update`. If an unmerged PR for the branch `cargo_update` already exists, it will edit then reopen it if necessary.

~~This also uses [`cargo-upgrades`](https://gitlab.com/kornelski/cargo-upgrades) to provide a list of available major upgrades in the PR body.~~

It includes the list of changes output by `cargo update` in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up.

Example PR: pitaj#2
Example action run: https://github.com/pitaj/rust/actions/runs/5035731903
Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F

Up for discussion:
- What period do we want? Currently weekly
- What user should it use? Currently "Github Actions"
- Do we need the extra security of provided by executing `cargo update` and `cargo-upgrades` in a separate job?
  If not I can simplify it to not need artifacts.
- PR message wording
- PR should probably always be `rollup=always`?
- What branch should it use?
- What should it do if no updates are available? Currently fails the job on empty commit
- Should the yml file live in `src/ci` instead of directly under workflows?
- ~~Is using the latest nightly toolchain enough to ensure compatibility with `Cargo.lock` and `Cargo.toml`s in master?~~
  Now pulls the bootstrap version from stage0.json

r? infra
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 17, 2023
Github action to periodically `cargo update` to keep dependencies current

Opens a PR periodically with the results of `cargo update`. If an unmerged PR for the branch `cargo_update` already exists, it will edit then reopen it if necessary.

~~This also uses [`cargo-upgrades`](https://gitlab.com/kornelski/cargo-upgrades) to provide a list of available major upgrades in the PR body.~~

It includes the list of changes output by `cargo update` in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up.

Example PR: pitaj#2
Example action run: https://github.com/pitaj/rust/actions/runs/5035731903
Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F

Up for discussion:
- What period do we want? Currently weekly
- What user should it use? Currently "Github Actions"
- Do we need the extra security of provided by executing `cargo update` and `cargo-upgrades` in a separate job?
  If not I can simplify it to not need artifacts.
- PR message wording
- PR should probably always be `rollup=always`?
- What branch should it use?
- What should it do if no updates are available? Currently fails the job on empty commit
- Should the yml file live in `src/ci` instead of directly under workflows?
- ~~Is using the latest nightly toolchain enough to ensure compatibility with `Cargo.lock` and `Cargo.toml`s in master?~~
  Now pulls the bootstrap version from stage0.json

r? infra
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 17, 2023
Github action to periodically `cargo update` to keep dependencies current

Opens a PR periodically with the results of `cargo update`. If an unmerged PR for the branch `cargo_update` already exists, it will edit then reopen it if necessary.

~~This also uses [`cargo-upgrades`](https://gitlab.com/kornelski/cargo-upgrades) to provide a list of available major upgrades in the PR body.~~

It includes the list of changes output by `cargo update` in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up.

Example PR: pitaj#2
Example action run: https://github.com/pitaj/rust/actions/runs/5035731903
Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F

Up for discussion:
- What period do we want? Currently weekly
- What user should it use? Currently "Github Actions"
- Do we need the extra security of provided by executing `cargo update` and `cargo-upgrades` in a separate job?
  If not I can simplify it to not need artifacts.
- PR message wording
- PR should probably always be `rollup=always`?
- What branch should it use?
- What should it do if no updates are available? Currently fails the job on empty commit
- Should the yml file live in `src/ci` instead of directly under workflows?
- ~~Is using the latest nightly toolchain enough to ensure compatibility with `Cargo.lock` and `Cargo.toml`s in master?~~
  Now pulls the bootstrap version from stage0.json

r? infra
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jun 18, 2023
Github action to periodically `cargo update` to keep dependencies current

Opens a PR periodically with the results of `cargo update`. If an unmerged PR for the branch `cargo_update` already exists, it will edit then reopen it if necessary.

~~This also uses [`cargo-upgrades`](https://gitlab.com/kornelski/cargo-upgrades) to provide a list of available major upgrades in the PR body.~~

It includes the list of changes output by `cargo update` in the commit message and PR body. Note that this output is currently sub-optimal due to rust-lang/cargo#9408, but if updates are made more regularly that is less likely to show up.

Example PR: pitaj/rust#2
Example action run: https://github.com/pitaj/rust/actions/runs/5035731903
Prior discussion: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/dependabot.20updates.3F

Up for discussion:
- What period do we want? Currently weekly
- What user should it use? Currently "Github Actions"
- Do we need the extra security of provided by executing `cargo update` and `cargo-upgrades` in a separate job?
  If not I can simplify it to not need artifacts.
- PR message wording
- PR should probably always be `rollup=always`?
- What branch should it use?
- What should it do if no updates are available? Currently fails the job on empty commit
- Should the yml file live in `src/ci` instead of directly under workflows?
- ~~Is using the latest nightly toolchain enough to ensure compatibility with `Cargo.lock` and `Cargo.toml`s in master?~~
  Now pulls the bootstrap version from stage0.json

r? infra
@epage epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Feb 2, 2024
@epage
Copy link
Contributor

epage commented Feb 2, 2024

These messages are outputted by diffing the resolved dependency tree before and after.

We make the assumption that we can report Updated and Downgrading messages when there is just one added and one removed version of a package in the tree.

The challenge is knowing how to line up adds and removes when there are multiple versions of a package in the tree. Maybe we can make the assumption that, so long as they are semver compatible, that its an upgrade? I worry there are corner cases with that where version unification doesn't happen, like two git sources with different hashes, and would want further input from someone like @Eh2406 to know if there are a set of restrictions where this could be done and if there are any future plans that could further complicate it (like would we ever extend build-std's independent resolve graphs for other big projects like gitoxide or bevy and would that cause problems?).

@Eh2406
Copy link
Contributor

Eh2406 commented Feb 7, 2024

This is a two-way dore. If this is not compatible with some other change we want to make we can back this out. The heuristic seems reasonable to me. (I am not remembering how GIT is handled in enough detail to know what exceptions need to be made there, but we can add a "and all versions are from a registry source" if needed.) It is also similar to Alex's suggestions in #9884

A bigger question is what happens to the message when cargo update is extended to semver breaking updates. And that is an area where you have more context then I.

@epage
Copy link
Contributor

epage commented Feb 7, 2024

Agreed its a two way door. More so my concern is we are duplicating knowledge and either the hassle for tracking it or how acceptable we find "bugs" in this as we don't take this into account.

As for the breaking update, that would just not be subject to this (goes back to Adding and Removing) and we'd just tweak the algorithm more to cover more cases, if desired enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-update S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants