Update dependency versions in the various `Cargo.toml`s to the version that we actually use

[oli-obk]

See #57435 (comment) for related comments and a PR that just updates Cargo.lock without any Cargo.toml reflecting the dependency version bump.

Bonus points for finding a way to do this in an automated manner.

[agnxy]

Hi @oli-obk, I'd like to work on this issue.
IIUC this is to check the version of each dependency in Cargo.lock and update the version in the corresponding Cargo.toml.

[mati865]

@agnxy keep in mind there is single Cargo.lock and multiple Cargo.toml in this repository.

[polybuildr]

Hello! I thought this would be an interesting problem to try to solve in an automated manner and gave it a shot.

First off, I know I did not ask on this issue before working on this, so I completely understand if @agnxy wants to do this instead. :) Would still like to know if I got the approach right, @oli-obk.

This was more involved than I had initially expected and I made a lot of guesses along the way, so please let me know if I've done something completely wrong (or maybe entirely misunderstood the issue in the first place).

First, I parsed the root Cargo.lock using the toml crate. I looked inside the package array and used the semver crate to parse all the available versions and put them into a HashMap<String, Vec<semver::Version>> where the String is for the package name. I then sorted the Vec so that the versions get ordered largest (latest) to smaller (oldest) and stored the map to use later.

I then used the glob crate to find all the Cargo.toml files in the repo. I then parsed each of those files but this time using the toml_edit crate (a format preserving TOML parser that cargo-edit uses).

For each dependency in the [dependencies] section, I looked for either a string value (dep = "0.1.0") or an "inline table" (dep = { path = "../something", version = "0.1.0" }) and read the current version used. I then parsed the current version as a semver requirement (semver::VersionReq) which should be treating it as a caret semver requirement and then looked in the available versions in the earlier prepared package map to find the latest compatible version. If one was found, I updated the version in the Cargo.toml file.

As far as I can tell (without much extensive testing), this seems to work. Hopefully, I've understood the issue correctly. Here's some sample output:

[2019-03-26T23:17:52Z TRACE sync_cargo_lock] Package build_helper, no version
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated cmake: '0.1.23' → '0.1.33' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated filetime: '0.2' → '0.2.4' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated num_cpus: '1.0' → '1.8.0' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated getopts: '0.2' → '0.2.17' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated cc: '1.0.1' → '1.0.28' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated libc: '0.2' → '0.2.50' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated serde: '1.0.8' → '1.0.82' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated serde_derive: '1.0.8' → '1.0.81' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated serde_json: '1.0.2' → '1.0.33' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated toml: '0.4' → '0.4.10' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated lazy_static: '0.2' → '0.2.11' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated time: '0.1' → '0.1.40' in "src/bootstrap/Cargo.toml"
[2019-03-26T23:17:52Z INFO  sync_cargo_lock] updated petgraph: '0.4.13' → '0.4.13' in "src/bootstrap/Cargo.toml"

[oli-obk]

Heh, sweet setup! Would you be ok with moving this tool into the rust-lang org? If so, @rust-lang/compiler we could run this tool unconditionally in tidy.

updated petgraph: '0.4.13' → '0.4.13'

What happened here?

looked in the available versions in the earlier prepared package map to find the latest compatible version.

Can you also add a mode where we just attempt to move all dependencies to the highest version number irrelevant of breaking changes? It's undesirable to have e.g. lazy_static 0.1 and lazy_static 0.2 within rustc, even if that works.

[polybuildr]

Would you be ok with moving this tool into the rust-lang org?

Sure! Sounds good to me. Would that be in a new repository or somewhere inside this repo? I saw a lot of scripts/tools in this repo too, which is why I'm curious.

What happened here?

Ah, yes. Probably an edge case where the version in Cargo.toml already matches the highest compatible version. I didn't check to see if it's the same version before doing a "replace".

Can you also add a mode...

Yeah, that seems simple enough. I'm guessing using that flag would need some additional checks since incompatible changes could actually be breaking?

[oli-obk]

I'm guessing using that flag would need some additional checks since incompatible changes could actually be breaking?

Yea, but if we fix it once, any future changes will already have been caused by the user, so it should be fine.

I saw a lot of scripts/tools in this repo too, which is why I'm curious.

I think this tool is useful for all workspace repositories, and I don't think we have anything in-tree that we are publishing via crates.io.

[polybuildr]

I see, so you were planning for this to be published and distributed via crates.io. This makes things a teensy bit complicated - my employer's rules allow for submitting patches to public repositories using an approved open source license quite easily. Creating a new repository (aka "releasing" a new project) is a bit more complicated: first deploy it internally, get approvals, then release to the outside world and even after that - need a CLA for other contributors. sigh

I'll reach out to them and see if something can be done to make the process simpler in this case. :)

[oli-obk]

Ah, well... What if we create a repo with the appropriate license and you contribute your code? Basically ownership would lie with the rust-lang org.

I mean, originally I thought we'd put it into the rust-lang/rust repo, too. But we are currently moving a lot of things out into separate repos. So that might have ended up happening to your code, too, even if it started out in the rust-lang/rust repo

[polybuildr]

Aha, I have good news! I spoke to the relevant team at work and they confirmed that your approach seems fine to them: create a new repository with the appropriate license and then I'll contribute to it. They said I'm still contributing to the overall Rust project, it doesn't matter what the actual organisation of repositories you use.

So that's great. I'll spend some time cleaning up the code I wrote. It's probably going to need multiple changes to be more idiomatic Rust anyway, but I might as well do clean-ups that I think make sense. :)

Let me know when the relevant repository gets created and I'll send a PR. In local code, I've called it cargo-lock-sync for lack of a better name, but I'd love a better name.

[polybuildr]

Oops, I'm using "sync-cargo-lock" but that is just as bad as the previous order. :P

[agnxy]

Awesome! Thanks @polybuildr . Sorry I'm busy these days and don't have a chance to work on this task.

[polybuildr]

Thanks for confirming, @agnxy! :)

[polybuildr]

Sorry for the delay here. I've been refactoring and cleaning up the code and I'm closer to being able to send it for review.

However, I realised my initial logic felt a bit off and I'll need a bit of help, @oli-obk, to figure out what the right approach is. I realised I was mixing semver::Version and semver::VersionReqs in sightly odd ways, considering that versions in Cargo.toml dependencies are caret version requirements by default.

If a package my-package has its version requirement specified as 1.2 and in the Cargo.lock, I find 2.1.3, 1.5.2, 1.2.4 and 0.8.2, what should Cargo.toml be updated to use?

If my understanding is correct, ^1.2 means any 1.x version, so both 1.2.4 and 1.5.2 are compatible with it, while 2.1.3 and 0.8.2 are not. Should I be updating to the latest compatible release, in this case "1.5.2"? Should that include the patch version or should it say "1.5"?

If 1.5.2 was not an available version in Cargo.lock, then should I change "1.2" to "1.2.4"?

I remember you asked for a flag that force updated to the latest available version, so in that case it should update to "2.1.3"?

And also, for my understanding, would you be running the tool with the forced update flag by default or would that be a one-off manual update?