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

Simplified Update Multiple Crate Output Formats #9884

Closed
wants to merge 4 commits into from

Conversation

heisen-li
Copy link
Contributor

close #9408

@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2021
@heisen-li heisen-li changed the title Simplified output format Simplified Update Multiple Crate Output Formats Sep 8, 2021
@alexcrichton
Copy link
Member

Thanks for the PR!

I'm not sure if this is the best fix though since this is just sort of blindly assuming that each pariwise mapping of these lists are the crate verisons that follow the updates. For example it could show 1->2.1 and 2.2->3.2 when in fact the update could be 1->3.2 and 2.1->2.2. I don't know of the best way to solve this, though. One possible solution is to bucket everything based on Cargo's idea of semver compatibility, and then for everything in its own bucket we could basically pair them either arbitrarily or in a sorted fashion.

@heisen-li
Copy link
Contributor Author

heisen-li commented Sep 19, 2021

Thanks for the PR!

I'm not sure if this is the best fix though since this is just sort of blindly assuming that each pariwise mapping of these lists are the crate verisons that follow the updates. For example it could show 1->2.1 and 2.2->3.2 when in fact the update could be 1->3.2 and 2.1->2.2. I don't know of the best way to solve this, though. One possible solution is to bucket everything based on Cargo's idea of semver compatibility, and then for everything in its own bucket we could basically pair them either arbitrarily or in a sorted fashion.

I'm very sorry, but the reply was too late for some reasons.

I am thinking whether it is complicated to think about the problem? The current cargo update command seems to only upgrade the current version to the latest version in the current major version, but not the latest version in all versions.

for example:

PS D:\Rust\temp\update_test> cat .\Cargo.toml
[package]
name = "update_test"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
rand_1 = { package = "rand", version = "0.1.1" }
rand_2 = { package = "rand", version = "0.8.3" }

then:

PS D:\Rust\temp\update_test> cargo update -p rand:0.1.4 --precise 0.1.1 > update.txt
    Updating `http://mirrors.tools.huawei.com/rust/crates.io-index/` index
    Removing libc v0.1.12
    Removing log v0.2.5
    Updating rand v0.1.4 -> v0.1.1

PS D:\Rust\temp\update_test> cargo update -p rand:0.8.4 --precise 0.8.3 > update.txt
    Updating `mirror` index
    Updating rand v0.8.4 -> v0.8.3

When we execute update, we will only update to the latest version of the current major version, that is, 0.1.1 to 0.1.4 instead of 0.8.4.
When I execute:

PS D:\Rust\temp\update_test> cargo update > update.txt
    Updating `mirror` index
      Adding libc v0.1.12
      Adding log v0.2.5
    Removing rand v0.1.1
    Removing rand v0.8.3
      Adding rand v0.1.4
      Adding rand v0.8.4

Therefore, after I modified it, it does not seem to affect the final result. The result is what I want.

PS D:\Rust\temp\update_test> cargo update > update.txt
    Updating `http://mirrors.tools.huawei.com/rust/crates.io-index/` index
      Adding libc v0.1.12
      Adding log v0.2.5
    Updating rand v0.1.1 -> v0.1.4
    Updating rand v0.8.3 -> v0.8.4

@alexcrichton
Copy link
Member

Yes I understand the purpose of this change, but I believe my comment still applies where the logic of matching what-updated-to-what is not super principled and I think it could be better.

@bors
Copy link
Contributor

bors commented Sep 27, 2021

☔ The latest upstream changes (presumably #9945) made this pull request unmergeable. Please resolve the merge conflicts.

@heisen-li
Copy link
Contributor Author

Sorry, the reply was late because of a long vacation.

For example it could show 1->2.1 and 2.2->3.2 when in fact the update could be 1->3.2 and 2.1->2.2.

I must be frank, I don't quite understand your explanation. What you mean by this sentence is that if ex_1 is 1.1->3.2 and ex_2 is 2.1->2.2, the result will not be what we want?

I conducted a supplementary test.

PS D:\Rust\temp\test\foo2> cat .\Cargo.toml
 ...
[dependencies]
rand_1 = { package = "rand", version = "0.1.1" }
rand_2 = { package = "rand", version = "0.8.3" }

then, i run cargo generate-lockfile, and cargo update -p rand:0.8.4 --precise 0.8.3cargo update -p rand:0.1.4 --precise 0.1.1

I modify the content that needs to be updated: 0.1.1->^0.5.*.

PS D:\Rust\temp\test\foo2> cat .\Cargo.toml
...
[dependencies]
rand_1 = { package = "rand", version = "^0.5.*" }
rand_2 = { package = "rand", version = "0.8.3" }

The following is the comparison result.
Before modification:

PS D:\Rust\temp\test\foo2> cargo update > update_old.txt
    Updating `mirror` index
      Adding bitflags v1.3.2
      Adding cloudabi v0.0.3
      Adding fuchsia-cprng v0.1.1
    Removing rand v0.1.1                <-------
    Removing rand v0.8.3                <-------
      Adding rand v0.5.6                <-------
      Adding rand v0.8.4                <-------
      Adding rand_core v0.3.1
      Adding rand_core v0.4.2
      Adding winapi v0.3.9
      Adding winapi-i686-pc-windows-gnu v0.4.0
      Adding winapi-x86_64-pc-windows-gnu v0.4.0

After modification:

PS D:\Rust\temp\test\foo2> cargo update > update_new.txt
    Updating `mirror` index
      Adding bitflags v1.3.2
      Adding cloudabi v0.0.3
      Adding fuchsia-cprng v0.1.1
    Updating rand v0.1.1 -> v0.5.6        <-------
    Updating rand v0.8.3 -> v0.8.4        <-------
      Adding rand_core v0.3.1
      Adding rand_core v0.4.2
      Adding winapi v0.3.9
      Adding winapi-i686-pc-windows-gnu v0.4.0
      Adding winapi-x86_64-pc-windows-gnu v0.4.0

Hope my understanding is correct. I also want to hear your opinion.

@alexcrichton
Copy link
Member

Yes to reiterate I understand what this change is doing, and I understand why it's being done. My point is that this is a very simple heuristic being applied that I don't think is solving the underlying issue. I think that a better solution here would be to determine a pairwise "upgrade" for each version of the same crate that changed, and then these pairwise upgrades can be printed as additions if they came from nothing, deletions if they went to nothing, and upgrades if it came from something and went to something.

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2021
@alexcrichton
Copy link
Member

Thanks for the PR, but I'm going to be stepping down from the Cargo team so I'm going to un-assign myself from this. The Cargo team will help review this when they get a chance.

@alexcrichton alexcrichton removed their assignment Mar 8, 2022
@ehuss
Copy link
Contributor

ehuss commented Mar 23, 2022

Closing as this has become a bit stale, and it looks like there are some design considerations here. Unfortunately the Cargo team does not have the capacity to accept changes of this kind at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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