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

Fix build metadata handling in SemVer versions #6451

Closed
sdroege opened this issue May 8, 2023 · 7 comments
Closed

Fix build metadata handling in SemVer versions #6451

sdroege opened this issue May 8, 2023 · 7 comments
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior

Comments

@sdroege
Copy link

sdroege commented May 8, 2023

Current Behavior

Currently there's apparently not checking. Yesterday https://crates.io/crates/const_fn_assert was updated with a new version, and apparently cargo does not like the checksum of that crate and reports

error: failed to sync
Caused by:
  failed to download packages
Caused by:
  failed to verify the checksum of `const_fn_assert v0.1.2+deprecated`

Expected Behavior

This ideally should've been caught before accepting the new version. cargo itself checks this before uploading AFAIU, so I kind of assume that this was done intentionally and manually to "deprecate" this crate a bit harder.

Steps To Reproduce

  1. cargo init
  2. cargo add const_fn_assert
  3. cargo check
  Downloaded const_fn_assert v0.1.2+deprecated
error: failed to verify the checksum of `const_fn_assert v0.1.2+deprecated`

Environment

No response

Anything else?

No response

@Turbo87
Copy link
Member

Turbo87 commented May 8, 2023

let hex_cksum: String = Sha256::digest(&tarball_bytes).encode_hex();
suggests that we calculate the checksum ourself, so I'm quite curious now what went wrong with that publish 🤔

@Turbo87 Turbo87 added C-bug 🐞 Category: unintended, undesired behavior A-backend ⚙️ labels May 8, 2023
@Turbo87
Copy link
Member

Turbo87 commented May 8, 2023

I think the problem here is how "build metadata" in the version field is handled. You can see at https://crates.io/crates/const_fn_assert/versions that crates.io now has 0.1.2 and 0.1.2+deprecated. crates.io currently treats them as different versions (for no good reason IMHO), and I assume cargo ignores the build metadata and downloads a different crate file that does not correspond to the build metadata.

/cc @rust-lang/cargo

@Turbo87 Turbo87 changed the title Check checksum of published crates before accepting Fix build metadata handling in SemVer versions May 8, 2023
@sdroege
Copy link
Author

sdroege commented May 8, 2023

Ah that would make sense and would at least be an honest mistake. Thanks for checking this so fast :)

@epage
Copy link

epage commented May 8, 2023

rust-lang/cargo#11412 for where the cargo team has discussed this

@epage
Copy link

epage commented May 8, 2023

Ah, that pointed me to this being a dupe of #1059

@Turbo87
Copy link
Member

Turbo87 commented May 8, 2023

for cross-referencing: we discussed the situation on Zulip (see https://rust-lang.zulipchat.com/#narrow/stream/318791-t-crates-io/topic/const_fn_assert/near/356688583)

we decided to remove const_fn_assert#0.1.2+deprecated from crates.io since our backend shouldn't have accepted the publish in the first place. I've also sent an email to the crate owner explaining the situation.

since we already have #1059 for tracking the problem I'll go ahead and close this issue here since the const_fn_assert situation itself has been resolved AFAICT.

@Turbo87 Turbo87 closed this as completed May 8, 2023
@Turbo87
Copy link
Member

Turbo87 commented May 30, 2023

#6518 has now been deployed. in other words: crates.io no longer allows "publishes with versions only differing in metadata"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

No branches or pull requests

3 participants