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

Jump to bottom

feat(cargo-lock): support lockfile v4 #1206

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat(cargo-lock): support lockfile v4 #1206

wants to merge 2 commits into from

Conversation

weihanglo
Copy link
Contributor

In 1.78.0 Cargo introduce v4 lockfile:
rust-lang/cargo#12852

The v3-to-v4 change is minimal: encode URL params with URL encoding.
However, roundtrip test failed because v4 we can only have one
serialization implementation at a time.

I don't know how to proceed. Maybe we could

  • Make v4 serialization the default when it becomes the default in Cargo
  • Provide API for people to generate different versions of lockfiles.
  • Work with upstream Cargo with an in-tree pacakge for lock serialization. (This is on me?)

Opened this PR for discussion.

@weihanglo weihanglo changed the title test(cargo-lock): show we cannot to parse v4 feat(cargo-lock): show we cannot to parse v4 Jun 24, 2024
@weihanglo weihanglo changed the title feat(cargo-lock): show we cannot to parse v4 feat(cargo-lock): support lockfile v4 Jun 24, 2024
@weihanglo weihanglo marked this pull request as draft June 24, 2024 18:17
weihanglo
weihanglo commented Jun 24, 2024
View reviewed changes
cargo-lock/tests/examples/Cargo.lock.v4
[[package]]
name = "toml"
version = "0.8.14"
source = "git https://github.com/weihanglo/toml.git?branch=a-_+#$)z#9e406273177740fa85b86b78e6d5105e932edef0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generated from https://github.com/weihanglo/toml/tree/a-_+#$)z.

@weihanglo
Copy link
Contributor Author

Hmm… missing some commits. Wait a sec.

weihanglo
weihanglo commented Jun 24, 2024
View reviewed changes
cargo-lock/tests/examples/Cargo.lock.v4
[[package]]
name = "url"
version = "2.5.2"
source = "git https://github.com/weihanglo/rust-url.git?tag=a-_+#$)z#54346fa288e16b25b71c45149d7067c752b450e0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generated from https://github.com/weihanglo/rust-url/releases/tag/a-_+#$)z.

@weihanglo
test(cargo-lock): show we cannot to parse v4
cdba7c1
@weihanglo
feat(cargo-lock): support lockfile v4
367cb98 
In 1.78.0 Cargo introduce v4 lockfile:

rust-lang/cargo#12852

The v3-to-v4 change is minimal: encode URL params with URL encoding.
However, roundtrip test failed because v4 we can only have one
serialization implementation at a time.

I don't know how to proceed. Maybe we could

* Make v4 serialization the default when it becomes the default in Cargo
* Provide API for people to generate different versions of lockfiles.
* Work with upstream Cargo with an in-tree pacakge for lock serialization.
  (This is on me?)

Opened this PR for discussion.
@tarcieri
Copy link
Member

Provide API for people to generate different versions of lockfiles.

@weihanglo it already supports this. Lockfile::version is consulted at serialization time and used to generate different versions of lockfiles.

@weihanglo
Copy link
Contributor Author

weihanglo commented Jun 24, 2024 •
edited
Loading

Provide API for people to generate different versions of lockfiles.

@weihanglo it already supports this. Lockfile::version is consulted at serialization time and used to generate different versions of lockfiles.

Yeah but not for SourceId. We need to make SourceId aware of resolve version. On the Cargo side there is an EncodableSourceId doing that job. I don't think this cargo-lock crate is particularly interested in copying over the wrapper from there.

@tarcieri
Copy link
Member

A lot of the code has been copied from there. It would be good to follow the same approach they're using.

@weihanglo
Copy link
Contributor Author

Work with upstream Cargo with an in-tree pacakge for lock serialization. (This is on me?)

I am mostly thinking through this. The Cargo team has published a new crate https://crates.io/crates/cargo-util-schemas providing basic serialization schemas for common types. I wonder if we could get there so cargo-lock can be a thin tool built upon crates from Cargo.

@tarcieri
Copy link
Member

Sure, that looks potentially useful

@tarcieri
Copy link
Member

tarcieri commented Aug 1, 2024

FWIW I took a look at cargo-util-schemas. It sure would be nice to use several types defined there, like PackageIdSpec, SourceKind and GitReference, however for those in particular they'd be nice to have as part of the public API, and unfortunately the latter two are enums so they can't really be wrapped in newtypes.

Unfortunately, it doesn't seem ready for that as they just released semver breaking changes a week ago.

In the meantime it doesn't otherwise seem to have types relating specifically to Cargo.lock, only Cargo.toml.

@weihanglo
Copy link
Contributor Author

In the meantime it doesn't otherwise seem to have types relating specifically to Cargo.lock, only Cargo.toml.

That's true.

I think maybe we should just copy over from there one more time 😞.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@weihanglo @tarcieri