Cargo.lock trailing newline removal in 1.38.0

[est31]

Consider this:

cargo +stable init --bin test_thing
cargo +stable check # to set Cargo.lock
git add . && git commit -m "Initial"
cargo +beta check
git diff

It will show you a difference! One line has been removed at the end of the file. Edit: Note that this only affects simple examples without dependencies. If there is a dependency, the newline behaviour is the same.

As time of writing, Rust beta is 1.38 and Rust stable is 1.37.

This bug may seem insignificant but it's the same "class" of change that #6180 was. In any environment where people use multiple versions of rustc to commit to the same codebase with a checked in Cargo.lock, you might run into situations where unrelated commits change Cargo.lock back and forth because the people just did cargo add. Environment with different versions of rustc can just mean that they haven't ran rustup in a while. #6180 at least had a phase in period.

I think the most likely culprit for this regression is #7070. I see some newline related changes there. cc @alexcrichton

I think that the old behaviour should be retained or at least the files with the trailing newline be kept so that there isn't jitter.

[est31]

This bug wouldn't be so bad if a simple cargo check wouldn't trigger it. IMO cargo check, cargo build etc. should change Cargo.lock only if there is a good reason to, like when Cargo.toml added a dependency or when explicitly asked (I'd see cargo update as such). The code currently serializes Cargo.lock and compares it to the stored Cargo.lock. It would be much better if the code would compare the parsed toml Value's instead, then stuff like comments or newlines wouldn't matter. And even better if the code would compare higher level concepts like resolution graphs so that maybe even the upcoming Cargo.lock format wouldn't be introduced by an unrelated command like cargo build.

[est31]

cc @pietroalbini from release team as this is a beta regression.

[pietroalbini]

Not sure if I would consider this a beta regression at the same level as the other ones: an old project still works, there is just a bit of churn in the lockfile. However, your suggestion of touching the lockfile only when needed is a good one, and could make this issue less of a problem.

Bringing in the loop the whole release team anyway, just to be sure. cc @rust-lang/release

[alexcrichton]

This is a regression, not everyone can fix everything in 19 hours since this was reported. All Cargo contributors are very busy, we'll make sure this is fixed before the next release.

[est31]

This is a regression, not everyone can fix everything in 19 hours since this was reported. All Cargo contributors are very busy, we'll make sure this is fixed before the next release.

@alexcrichton I don't think that any of my statements can be interpreted as expectation of such a quick fix. Despite that, for the case you got such an impression, I'm deeply sorry for causing such an impression. Of course I don't expect that! I just wasn't sure whom to ping as you seemed busy (you still seem to be) and I tend to forget about bugs like this one myself so it's always best if it's in other people's trackers. Great respect for your work including that PR.

Thanks for your statement that the bug will be fixed until the next release.