Skip to content

Commit 50d52eb

Browse files
committed
Auto merge of #10551 - jeremyBanks:Cargo.toml.orig, r=weihanglo
Reserve filename `Cargo.toml.orig` in `cargo package` ### What does this PR try to resolve? _([previously discussed on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Reserving.20.2FCargo.2Etoml.2Eorig))_ Currently, `cargo package` will detect if there is an existing file named `.cargo_vcs_info.json` in the project folder and raise an error. This is to avoid collisions with the file it generates of the same name. However, there is no such logic for the other file it generates, `Cargo.toml.orig`. If such a file exists in the project folder, instead of an error, or one file taking precedence, cargo will generate a tarball containing two files with the same name. For example, from a package I uploaded as a test: ```sh curl https://crates.io/api/v1/crates/a-/0.0.0--a-/download -L | gunzip | tar -tv ``` ```text -rw-r--r-- 0 0 0 8 29 Nov 1973 a--0.0.0--a-/.gitignore -rw-r--r-- 0 0 0 150 31 Dec 1969 a--0.0.0--a-/Cargo.lock -rw-r--r-- 0 0 0 641 31 Dec 1969 a--0.0.0--a-/Cargo.toml -rw-r--r-- 0 0 0 160 29 Nov 1973 a--0.0.0--a-/Cargo.toml.orig <-- generated -rw-r--r-- 0 0 0 14 29 Nov 1973 a--0.0.0--a-/Cargo.toml.orig <-- existing -rw-r--r-- 0 0 0 45 29 Nov 1973 a--0.0.0--a-/src/main.rs ``` This PR is a two-line change to add this filename to the existing check for `.cargo_vcs_info.json`. ### How should we test and review this PR? I have added a unit test to verify that the expected error is produced, copying the existing unit test for `.cargo_vcs_info.json`. I have manually tested the change locally and confirmed that it raises an error if the file exists, and has no effect if it does not. Given the small size of this change, I think this is sufficient, but please let me know if anything else is expected. ### Additional information This raises a separate question of whether Cargo or crates.io should prohibit tarballs with duplicate filenames. It seems like most (all?) `tar` implementations will give precedence to the last file (which will be the existing copy here, not the generated one), but I don't believe this is specified behaviour, and it's possible that scripts scanning through tarballs directly (without first extracting to the filesystem) may not handle this case correctly. For example, I was working on a rough script to compare packaged libraries to their corresponding Git commits where possible, and this wasn't a case I had anticipated. In any case, it seems like preventing such tarballs from being created by accident (this PR) is a good place to start.
2 parents 1073915 + e71fb81 commit 50d52eb

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

src/cargo/ops/cargo_package.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub struct PackageOpts<'cfg> {
3636
pub cli_features: CliFeatures,
3737
}
3838

39+
const ORIGINAL_MANIFEST_FILE: &str = "Cargo.toml.orig";
3940
const VCS_INFO_FILE: &str = ".cargo_vcs_info.json";
4041

4142
struct ArchiveFile {
@@ -219,8 +220,8 @@ fn build_ar_list(
219220
match rel_str.as_ref() {
220221
"Cargo.toml" => {
221222
result.push(ArchiveFile {
222-
rel_path: PathBuf::from("Cargo.toml.orig"),
223-
rel_str: "Cargo.toml.orig".to_string(),
223+
rel_path: PathBuf::from(ORIGINAL_MANIFEST_FILE),
224+
rel_str: ORIGINAL_MANIFEST_FILE.to_string(),
224225
contents: FileContents::OnDisk(src_file),
225226
});
226227
result.push(ArchiveFile {
@@ -230,10 +231,9 @@ fn build_ar_list(
230231
});
231232
}
232233
"Cargo.lock" => continue,
233-
VCS_INFO_FILE => anyhow::bail!(
234-
"invalid inclusion of reserved file name \
235-
{} in package source",
236-
VCS_INFO_FILE
234+
VCS_INFO_FILE | ORIGINAL_MANIFEST_FILE => anyhow::bail!(
235+
"invalid inclusion of reserved file name {} in package source",
236+
rel_str
237237
),
238238
_ => {
239239
result.push(ArchiveFile {

tests/testsuite/package.rs

+39
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,45 @@ in package source
283283
.run();
284284
}
285285

286+
#[cargo_test]
287+
fn orig_file_collision() {
288+
let p = project().build();
289+
let _ = git::repo(&paths::root().join("foo"))
290+
.file(
291+
"Cargo.toml",
292+
r#"
293+
[project]
294+
name = "foo"
295+
description = "foo"
296+
version = "0.0.1"
297+
authors = []
298+
license = "MIT"
299+
documentation = "foo"
300+
homepage = "foo"
301+
repository = "foo"
302+
exclude = ["*.no-existe"]
303+
"#,
304+
)
305+
.file(
306+
"src/main.rs",
307+
r#"
308+
fn main() {}
309+
"#,
310+
)
311+
.file("Cargo.toml.orig", "oops")
312+
.build();
313+
p.cargo("package")
314+
.arg("--no-verify")
315+
.with_status(101)
316+
.with_stderr(
317+
"\
318+
[ERROR] invalid inclusion of reserved file name Cargo.toml.orig \
319+
in package source
320+
",
321+
)
322+
.run();
323+
}
324+
286325
#[cargo_test]
287326
fn path_dependency_no_version() {
288327
let p = project()

0 commit comments

Comments
 (0)