Skip to content

Add missing truncate when writing .crate files#16688

Merged
weihanglo merged 4 commits intorust-lang:masterfrom
Embers-of-the-Fire:fix-16683
Mar 6, 2026
Merged

Add missing truncate when writing .crate files#16688
weihanglo merged 4 commits intorust-lang:masterfrom
Embers-of-the-Fire:fix-16683

Conversation

@Embers-of-the-Fire
Copy link
Contributor

What does this PR try to resolve?

Fixes #16683 by adding manuall truncate.

How to test and review this PR?

See the original issue for mCVE.

@rustbot rustbot added Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2026

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ehuss, @epage, @weihanglo
  • @ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
  • Random selection from ehuss, epage, weihanglo

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. Would you mind adding a test for it?

For bugfixes, we usually follow a variant of atomic commit pattern:

  1. Commit a test that asserts the current buggy behavior (passes).
  2. In the next commit, fix the bug and update the test/snapshot.

Every commit passes, and the test/snapshot diff shows the behavior change.

View changes since this review

@ehuss
Copy link
Contributor

ehuss commented Mar 1, 2026

Should we consider backporting this to beta (now 1.95)? Looks like this was introduced in 1.93.

@weihanglo
Copy link
Member

Should we consider backporting this to beta (now 1.95)? Looks like this was introduced in 1.93.

Agreed. Thanks for pointing this out.

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated to backport to the beta branch. label Mar 1, 2026
0xPoe

This comment was marked as outdated.

0xPoe
0xPoe previously requested changes Mar 4, 2026
Copy link
Member

@0xPoe 0xPoe left a comment

Choose a reason for hiding this comment

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

Thanks!

I believe the temp registry has the same issue. Could you please fix it as well?

This case cloud help us catch the bug:

#[cargo_test]
fn repackage_smaller_local_dep_tmp_registry_checksum_match() {
    let reg = registry::init();
    let big_file_contents = "x".repeat(100_000);
    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [workspace]
                members = ["foo", "bar"]
                resolver = "2"
            "#,
        )
        .file(
            "foo/Cargo.toml",
            r#"
                [package]
                name = "foo"
                version = "0.0.1"
                edition = "2021"

                [dependencies]
                bar = { path = "../bar", version = "0.0.1" }
            "#,
        )
        .file("foo/src/lib.rs", "pub fn foo() { bar::bar(); }")
        .file(
            "bar/Cargo.toml",
            r#"
                [package]
                name = "bar"
                version = "0.0.1"
                edition = "2021"
                include = ["src/**", "Cargo.toml", "big.txt"]
            "#,
        )
        .file("bar/src/lib.rs", "pub fn bar() {}")
        .file("bar/big.txt", &big_file_contents)
        .build();

    p.cargo("package --workspace --no-verify")
        .replace_crates_io(reg.index_url())
        .run();

    fs::remove_file(p.root().join("bar/big.txt")).unwrap();
    p.change_file(
        "bar/Cargo.toml",
        r#"
            [package]
            name = "bar"
            version = "0.0.1"
            edition = "2021"
            include = ["src/**", "Cargo.toml"]
        "#,
    );
    p.cargo("package --workspace --no-verify")
        .replace_crates_io(reg.index_url())
        .run();

    let index_line = read_to_string(p.root().join("target/package/tmp-registry/index/3/b/bar"))
        .unwrap()
        .lines()
        .last()
        .unwrap()
        .to_owned();
    let expected_cksum = serde_json::from_str::<serde_json::Value>(&index_line)
        .unwrap()
        .get("cksum")
        .and_then(|value| value.as_str())
        .unwrap()
        .to_owned();

    let crate_contents = fs::read(p.root().join("target/package/tmp-registry/bar-0.0.1.crate"))
        .unwrap();
    let actual_cksum = registry::cksum(&crate_contents);

    assert_eq!(
        expected_cksum, actual_cksum,
        "tmp-registry crate checksum should match index entry"
    );

    p.cargo("package --workspace")
        .replace_crates_io(reg.index_url())
        .run();
}

I would suggest that when adding the test, the Conventional Commit Message should start with test: instead of add: .

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 4, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@Embers-of-the-Fire
Copy link
Contributor Author

Well, seems irrelavent with my code. @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Mar 5, 2026
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

@weihanglo weihanglo enabled auto-merge March 6, 2026 01:11
@weihanglo weihanglo dismissed 0xPoe’s stale review March 6, 2026 01:11

PR author has made the test and change.

@weihanglo weihanglo added this pull request to the merge queue Mar 6, 2026
Merged via the queue into rust-lang:master with commit 0ada615 Mar 6, 2026
45 of 58 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2026
weihanglo added a commit to weihanglo/cargo that referenced this pull request Mar 6, 2026
<!--
_Thanks for the pull request 🎉!_
_Please read the contribution guide: <https://doc.crates.io/contrib/>._
-->

### What does this PR try to resolve?

Fixes rust-lang#16683 by adding manuall truncate.
<!--
_Explain the motivation behind this change._
_A clear overview along with an in-depth explanation are helpful._
-->

### How to test and review this PR?

See the original issue for mCVE.
<!--
_Demonstrate how you test this change and guide reviewers through your
PR._
_With a smooth review process, a pull request usually gets reviewed
quicker._
-->
weihanglo added a commit to weihanglo/cargo that referenced this pull request Mar 6, 2026
<!--
_Thanks for the pull request 🎉!_
_Please read the contribution guide: <https://doc.crates.io/contrib/>._
-->

### What does this PR try to resolve?

Fixes rust-lang#16683 by adding manuall truncate.
<!--
_Explain the motivation behind this change._
_A clear overview along with an in-depth explanation are helpful._
-->

### How to test and review this PR?

See the original issue for mCVE.
<!--
_Demonstrate how you test this change and guide reviewers through your
PR._
_With a smooth review process, a pull request usually gets reviewed
quicker._
-->
epage added a commit that referenced this pull request Mar 6, 2026
Beta backports

- #16688

In order to make CI pass, the following PRs are also cherry-picked:

-
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-nominated Nominated to backport to the beta branch. Command-package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cargo package does not truncate old file content

6 participants