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

Update lock file encodings on changes #8554

Merged
merged 1 commit into from
Jul 29, 2020

Conversation

alexcrichton
Copy link
Member

This commit alters Cargo's lockfile encoding update strategy from its
previous incarnation. Previously Cargo had two versions, one for new
lock files and one for old lock files. Each of these versions were
different and would affect how Cargo manages lock file updates. The
intention was that we'd roll out defaults to new lock files first and
then later to preexisting lock files. This requires two separate
changes, though, and it's not necessarily clear when to start updating
old lock files. Additionally when old lock files were opted in it would
break builds using --locked if they simply updated Cargo because Cargo
would would want to bring the lock file versions forward.

The purpose of this change is to solve these issues. The new strategy
for updating a lock file's encoding is to simply preserve what's already
existing on the filesystem until we actually decide to write out a new
lock file. When Cargo updates a lock file on-disk then it will, at that
time, update the lock file encoding to whatever the current default is.
This means that there's only one version number to keep track of (the
default for encoding). Cargo will always preserve the preexisting
encoding unless another change is required to the lock file.

This commit alters Cargo's lockfile encoding update strategy from its
previous incarnation. Previously Cargo had two versions, one for new
lock files and one for old lock files. Each of these versions were
different and would affect how Cargo manages lock file updates. The
intention was that we'd roll out defaults to new lock files first and
then later to preexisting lock files. This requires two separate
changes, though, and it's not necessarily clear when to start updating
old lock files. Additionally when old lock files were opted in it would
break builds using `--locked` if they simply updated Cargo because Cargo
would would want to bring the lock file versions forward.

The purpose of this change is to solve these issues. The new strategy
for updating a lock file's encoding is to simply preserve what's already
existing on the filesystem until we actually decide to write out a new
lock file. When Cargo updates a lock file on-disk then it will, at that
time, update the lock file encoding to whatever the current default is.
This means that there's only one version number to keep track of (the
default for encoding). Cargo will always preserve the preexisting
encoding unless another change is required to the lock file.
@rust-highfive
Copy link

r? @Eh2406

(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 Jul 28, 2020
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 28, 2020

I like the idea! The fact that we have not force updated to the current logfile makes me thing that this policy matchs what we were willing to do better than the nominal old policy. Over all +1.

I have made too many mistakes lately to approve on my own. But the code looks good.

I do have one thought, what amount of change triggers the update to the version? cargo update, or adding a dep, or any change, or only when there is not a previous lockfile?

@alexcrichton
Copy link
Member Author

Oh no worries! Let's r? @ehuss here to get a second set of eyes.

The intention of this change is that if Cargo is writing a lock file it always writes it in the maximum of the default encoding version and the version of the previous lock file. Cargo only writes a lock file if it changes (or didn't exist) which means that only when lock files change is the lock file encoding updated. The lock file can change for any of the reasons you mentioned, basically. To try to list them as a bulleted list:

  • Adding a dependency
  • Removing a dependency
  • Changing a version requirement on a dependency
  • Running cargo update
  • Running cargo update -p
  • Deleting the previous lock file (or if there isn't one)
  • Changing names of crates

A bug this is fixing is that if you take an old project (as of this Cargo version) and run cargo build --locked it won't update the lock file even if the default encoding is updated (unlike Cargo as of before this PR), yay!

@rust-highfive rust-highfive assigned ehuss and unassigned Eh2406 Jul 28, 2020
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 28, 2020

@bors r? @ehuss

I do love the code and docs simplicity of the "any change" answer. But I wonder if the users will expect some changes to be "so small" as to not justify being upgraded. So do we want the bug reports or do we want the more complicated code. 🤷

@ehuss
Copy link
Contributor

ehuss commented Jul 29, 2020

Seems reasonable to me. v2 was introduced in 1.38 (09-26-2019), made the default in 1.41 (01-30-2020) and this would hit stable in 1.47 (10-08-2020), giving over a year span of stable releases. If anyone wants to support older versions, they can stay on v1, and do updates with any cargo older than 1.47.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2020

📌 Commit fdbe9f8 has been approved by ehuss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 29, 2020
@bors
Copy link
Contributor

bors commented Jul 29, 2020

⌛ Testing commit fdbe9f8 with merge b1d4a2e...

@bors
Copy link
Contributor

bors commented Jul 29, 2020

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing b1d4a2e to master...

@bors bors merged commit b1d4a2e into rust-lang:master Jul 29, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 30, 2020
Update cargo

14 commits in aa6872140ab0fa10f641ab0b981d5330d419e927..974eb438da8ced6e3becda2bbf63d9b643eacdeb
2020-07-23 13:46:27 +0000 to 2020-07-29 16:15:05 +0000
- Fix O0 build scripts by default without `[profile.release]` (rust-lang/cargo#8560)
- Emphasize git dependency version locking behavior. (rust-lang/cargo#8561)
- Update lock file encodings on changes (rust-lang/cargo#8554)
- Fix sporadic lto test failures. (rust-lang/cargo#8559)
- build-std: Fix libraries paths following upstream (rust-lang/cargo#8558)
- Flag git http errors as maybe spurious (rust-lang/cargo#8553)
- Display builtin aliases with `cargo --list` (rust-lang/cargo#8542)
- Check manifest for requiring nonexistent features (rust-lang/cargo#7950)
- Clarify test name filter usage (rust-lang/cargo#8552)
- Revert Cargo Book changes for default edition (rust-lang/cargo#8551)
- Prepare for not defaulting to master branch for git deps (rust-lang/cargo#8522)
- Include `+` for crates.io feature requirements in the Cargo Book section on features (rust-lang/cargo#8547)
- Update termcolor and fwdansi versions (rust-lang/cargo#8540)
- Cargo book nitpick in Manifest section (rust-lang/cargo#8543)
@mathstuf
Copy link
Contributor

If anyone wants to support older versions, they can stay on v1, and do updates with any cargo older than 1.47.

Unfortunately, some useful nightly flags still exist (-Z minimal-versions) which this has now broken.

@ehuss ehuss added this to the 1.47.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants