Update dependency versions in the various Cargo.tomls 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?

[oli-obk]

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"?

yes

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?

I think we'd start with "off" and then move to "forced" once we have been able to address all the changes.

[polybuildr]

Thanks for the clarification! Could you please help with this part too?

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"?

[oli-obk]

Let's start without patch versions. So just 1.5 (except if the Cargo.toml already says 1.5.x, then leave it be)

[polybuildr]

Here's a sample update performed by the tool: polybuildr#2

What do you think?

There is still at least one bug (one of the sections getting reordered for no reason) but I could fix that by hand for now. Unsure why it's happening, could be something in toml_edit. Speaking of, I was having some trouble with toml_edit's public API, so I added in one function -- unsure if they will accept it upstream.

[mati865]

Nice! Is the code available somewhere?

[polybuildr]

Unfortunately, due to employer restrictions (more at #57443 (comment)), I'd be more comfortable sending a pull request to the skeleton repository once it's created. In any case, the code will need many rounds of review before it's ready.

[oli-obk]

Nominating for discussion at the team meeting. Let's create a repo for this. It's a general purpose tool that we want to use in rustbuild. As per this year's strategy of moving things out of tree, I want to start this out of tree. I can take care of everything once we have a repo where I have merge or r+ powers.

[eddyb]

Could this be something we can add to cargo-edit and maybe even make cargo-edit official?

[nikomatsakis]

Discussed in the triage meeting. I pointed @oli-obk at the procedure for creating new crates

[torhovland]

Seems to me this ended up in killercup/cargo-edit#297. Shouldn't this issue be closed?