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.

Sign up for GitHub

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

feat: stabilize lockfile v4 #12852

Merged
merged 2 commits into from
Jan 30, 2024
Merged

feat: stabilize lockfile v4 #12852

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6af53da
feat(lockfile): stabilize v4 and prepare for v5
weihanglo Oct 19, 2023
f8a47fa
refactor: make clear SourceId serialization will change in v4
weihanglo Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(lockfile): stabilize v4 and prepare for v5
  • Loading branch information
@weihanglo
weihanglo committed Jan 3, 2024
commit 6af53da5ec3865524e168f5f3513a479652a256f
18 changes: 10 additions & 8 deletions src/cargo/core/resolver/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,18 +154,18 @@ impl EncodableResolve {
/// primary uses is to be used with `resolve_with_previous` to guide the
/// resolver to create a complete Resolve.
pub fn into_resolve(self, original: &str, ws: &Workspace<'_>) -> CargoResult<Resolve> {
let unstable_lockfile_version_allowed = ws.config().cli_unstable().next_lockfile_bump;
let path_deps = build_path_deps(ws)?;
let mut checksums = HashMap::new();

let mut version = match self.version {
Some(4) if ws.config().nightly_features_allowed => {
if unstable_lockfile_version_allowed {
Comment on lines -157 to -163
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create V5 at the same time?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice suggestion. Done.

ResolveVersion::V4
Some(n @ 5) if ws.config().nightly_features_allowed => {
if ws.config().cli_unstable().next_lockfile_bump {
ResolveVersion::V5
} else {
anyhow::bail!("lock file version 4 requires `-Znext-lockfile-bump`");
anyhow::bail!("lock file version `{n}` requires `-Znext-lockfile-bump`");
}
}
Some(4) => ResolveVersion::V4,
Some(3) => ResolveVersion::V3,
Some(n) => bail!(
"lock file version `{}` was found, but this version of Cargo \
Expand Down Expand Up @@ -694,6 +694,7 @@ impl ser::Serialize for Resolve {
metadata,
patch,
version: match self.version() {
ResolveVersion::V5 => Some(5),
ResolveVersion::V4 => Some(4),
ResolveVersion::V3 => Some(3),
ResolveVersion::V2 | ResolveVersion::V1 => None,
Expand Down Expand Up @@ -797,9 +798,10 @@ fn encodable_source_id(id: SourceId, version: ResolveVersion) -> Option<Encodabl
if id.is_path() {
None
} else {
Some(match version {
ResolveVersion::V4 => EncodableSourceId::new(id),
_ => EncodableSourceId::without_url_encoded(id),
Some(if version >= ResolveVersion::V4 {
EncodableSourceId::new(id)
} else {
EncodableSourceId::without_url_encoded(id)
})
}
}
10 changes: 6 additions & 4 deletions src/cargo/core/resolver/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,14 @@ pub enum ResolveVersion {
/// V3 by default staring in 1.53.
#[default]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we plan to do to track updating the default?

btw now that we have MSRV piped into the resolve logic (#12560), should we make the default based on MSRV with a fallback if no MSRV is set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #12120 (comment)

I'll post another PR for doc update.

For MSRV-aware lockfile generating, see #12861.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the latest, we might want to default to the new format for 2024-edition crates, since those crates won't build with old cargo anyway so there won't be any "flapping" between formats for different Cargo versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further more, it does sound like we might want to check if edition and rust-version are in sync and don't conflict with each other.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have checks that the rust-version isn't older than the edition allows.

Falling back to an edition check if the rust-version isn't set would be useful. The problem is we don't have a workspace edition field. We could do something like rust-version and check the oldest edition present.

V3,
/// SourceId URL serialization is aware of URL encoding. For example,
/// `?branch=foo bar` is now encoded as `?branch=foo+bar` and can be decoded
/// back and forth correctly. Introduced in 2024 in version 1.77.
V4,
/// Unstable. Will collect a certain amount of changes and then go.
///
/// Changes made:
///
/// * SourceId URL serialization is aware of URL encoding.
V4,
V5,
}

impl ResolveVersion {
Expand All @@ -95,7 +97,7 @@ impl ResolveVersion {
///
/// Update this when you're going to stabilize a new lockfile format.
pub fn max_stable() -> ResolveVersion {
ResolveVersion::V3
ResolveVersion::V4
}
}

Expand Down
20 changes: 7 additions & 13 deletions tests/testsuite/lockfile_compat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ perhaps a crate was updated and forgotten to be re-vendored?
}

#[cargo_test]
fn v4_is_unstable() {
fn next_version_is_always_unstable() {
let p = project()
.file(
"Cargo.toml",
Expand All @@ -903,7 +903,7 @@ fn v4_is_unstable() {
),
)
.file("src/lib.rs", "")
.file("Cargo.lock", "version = 4")
.file("Cargo.lock", "version = 5")
.build();

p.cargo("fetch")
Expand All @@ -913,7 +913,7 @@ fn v4_is_unstable() {
error: failed to parse lock file at: [CWD]/Cargo.lock

Caused by:
lock file version `4` was found, but this version of Cargo does not \
lock file version `5` was found, but this version of Cargo does not \
understand this lock file, perhaps Cargo needs to be updated?
",
)
Expand All @@ -928,7 +928,7 @@ Caused by:
error: failed to parse lock file at: [CWD]/Cargo.lock

Caused by:
lock file version 4 requires `-Znext-lockfile-bump`
lock file version `5` requires `-Znext-lockfile-bump`
",
)
.run();
Expand All @@ -950,9 +950,7 @@ fn v4_cannot_be_created_from_scratch() {
.file("src/lib.rs", "")
.build();

p.cargo("fetch -Znext-lockfile-bump")
.masquerade_as_nightly_cargo(&["-Znext-lockfile-bump"])
.run();
p.cargo("fetch").run();

let lockfile = r#"# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
Expand Down Expand Up @@ -1124,8 +1122,7 @@ dependencies = [
.file("Cargo.lock", "version = 4")
.build();

p.cargo("check -Znext-lockfile-bump")
.masquerade_as_nightly_cargo(&["-Znext-lockfile-bump"])
p.cargo("check")
.with_stderr(format!(
"\
[UPDATING] git repository `{url}`
Expand All @@ -1141,10 +1138,7 @@ dependencies = [

// Unlike v3_and_git_url_encoded, v4 encodes URL parameters so no git
// repository re-clone happen.
p.cargo("check -Znext-lockfile-bump")
.masquerade_as_nightly_cargo(&["-Znext-lockfile-bump"])
.with_stderr("[FINISHED] dev [..]")
.run();
p.cargo("check").with_stderr("[FINISHED] dev [..]").run();
}

#[cargo_test]
Expand Down