Skip to content

Commit c863660

Browse files
committed
Auto merge of #12399 - epage:normalize, r=weihanglo
fix(package): Recognize and normalize `cargo.toml` ### What does this PR try to resolve? This solution is a blend of conservative and easy - Normalizes `cargo.toml` to `Cargo.toml` on publish - Ensuring we publish the `prepare_for_publish` version and include `Cargo.toml.orig` - Avoids dealing with trying to push existing users to `Cargo.toml` - All other cases of `Cargo.toml` are warnings - We could either normalize or turn this into an error in the future - When helping users with case previously, we've only handle the `cargo.toml` case - We already should have a fallback in case a manifest isn't detected - I didn't want to put in too much effort to make the code more complicated to handle this As a side effect, if a Linux user has `cargo.toml` and `Cargo.toml`, we'll only put one of them in the `.crate` file. We can extend this out to also include a warning for portability for case insensitive filesystems but I left that for after #12235. ### How should we test and review this PR? A PR at a time will show how the behavior changed as the source was edited This does add a direct dependency on `unicase` to help keep case-insensitive comparisons easy / clear and to avoid riskier areas for bugs like writing an appropriate `Hash` implementation. `unicase` is an existing transitive dependency of cargo. ### Additional information Fixes #12384 [Discussion on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.60cargo.2Etoml.60.20on.20case.20insensitive.20filesystems)
2 parents e999957 + cc6b6c9 commit c863660

File tree

4 files changed

+180
-38
lines changed

4 files changed

+180
-38
lines changed

Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ thiserror = "1.0.40"
9494
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
9595
toml = "0.7.0"
9696
toml_edit = "0.19.0"
97+
unicase = "2.6.0"
9798
unicode-width = "0.1.5"
9899
unicode-xid = "0.2.0"
99100
url = "2.2.2"
@@ -177,6 +178,7 @@ termcolor.workspace = true
177178
time.workspace = true
178179
toml.workspace = true
179180
toml_edit.workspace = true
181+
unicase.workspace = true
180182
unicode-width.workspace = true
181183
unicode-xid.workspace = true
182184
url.workspace = true

src/cargo/ops/cargo_package.rs

+65-38
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use flate2::{Compression, GzBuilder};
2525
use log::debug;
2626
use serde::Serialize;
2727
use tar::{Archive, Builder, EntryType, Header, HeaderMode};
28+
use unicase::Ascii as UncasedAscii;
2829

2930
pub struct PackageOpts<'cfg> {
3031
pub config: &'cfg Config,
@@ -227,58 +228,84 @@ fn build_ar_list(
227228
src_files: Vec<PathBuf>,
228229
vcs_info: Option<VcsInfo>,
229230
) -> CargoResult<Vec<ArchiveFile>> {
230-
let mut result = Vec::new();
231+
let mut result = HashMap::new();
231232
let root = pkg.root();
232-
for src_file in src_files {
233-
let rel_path = src_file.strip_prefix(&root)?.to_path_buf();
234-
check_filename(&rel_path, &mut ws.config().shell())?;
235-
let rel_str = rel_path
236-
.to_str()
237-
.ok_or_else(|| {
238-
anyhow::format_err!("non-utf8 path in source directory: {}", rel_path.display())
239-
})?
240-
.to_string();
233+
234+
for src_file in &src_files {
235+
let rel_path = src_file.strip_prefix(&root)?;
236+
check_filename(rel_path, &mut ws.config().shell())?;
237+
let rel_str = rel_path.to_str().ok_or_else(|| {
238+
anyhow::format_err!("non-utf8 path in source directory: {}", rel_path.display())
239+
})?;
241240
match rel_str.as_ref() {
242-
"Cargo.toml" => {
243-
result.push(ArchiveFile {
244-
rel_path: PathBuf::from(ORIGINAL_MANIFEST_FILE),
245-
rel_str: ORIGINAL_MANIFEST_FILE.to_string(),
246-
contents: FileContents::OnDisk(src_file),
247-
});
248-
result.push(ArchiveFile {
249-
rel_path,
250-
rel_str,
251-
contents: FileContents::Generated(GeneratedFile::Manifest),
252-
});
253-
}
254241
"Cargo.lock" => continue,
255242
VCS_INFO_FILE | ORIGINAL_MANIFEST_FILE => anyhow::bail!(
256243
"invalid inclusion of reserved file name {} in package source",
257244
rel_str
258245
),
259246
_ => {
260-
result.push(ArchiveFile {
261-
rel_path,
262-
rel_str,
263-
contents: FileContents::OnDisk(src_file),
264-
});
247+
result
248+
.entry(UncasedAscii::new(rel_str))
249+
.or_insert_with(Vec::new)
250+
.push(ArchiveFile {
251+
rel_path: rel_path.to_owned(),
252+
rel_str: rel_str.to_owned(),
253+
contents: FileContents::OnDisk(src_file.clone()),
254+
});
265255
}
266256
}
267257
}
258+
259+
// Ensure we normalize for case insensitive filesystems (like on Windows) by removing the
260+
// existing entry, regardless of case, and adding in with the correct case
261+
if result.remove(&UncasedAscii::new("Cargo.toml")).is_some() {
262+
result
263+
.entry(UncasedAscii::new(ORIGINAL_MANIFEST_FILE))
264+
.or_insert_with(Vec::new)
265+
.push(ArchiveFile {
266+
rel_path: PathBuf::from(ORIGINAL_MANIFEST_FILE),
267+
rel_str: ORIGINAL_MANIFEST_FILE.to_string(),
268+
contents: FileContents::OnDisk(pkg.manifest_path().to_owned()),
269+
});
270+
result
271+
.entry(UncasedAscii::new("Cargo.toml"))
272+
.or_insert_with(Vec::new)
273+
.push(ArchiveFile {
274+
rel_path: PathBuf::from("Cargo.toml"),
275+
rel_str: "Cargo.toml".to_string(),
276+
contents: FileContents::Generated(GeneratedFile::Manifest),
277+
});
278+
} else {
279+
ws.config().shell().warn(&format!(
280+
"no `Cargo.toml` file found when packaging `{}` (note the case of the file name).",
281+
pkg.name()
282+
))?;
283+
}
284+
268285
if pkg.include_lockfile() {
269-
result.push(ArchiveFile {
270-
rel_path: PathBuf::from("Cargo.lock"),
271-
rel_str: "Cargo.lock".to_string(),
272-
contents: FileContents::Generated(GeneratedFile::Lockfile),
273-
});
286+
let rel_str = "Cargo.lock";
287+
result
288+
.entry(UncasedAscii::new(rel_str))
289+
.or_insert_with(Vec::new)
290+
.push(ArchiveFile {
291+
rel_path: PathBuf::from(rel_str),
292+
rel_str: rel_str.to_string(),
293+
contents: FileContents::Generated(GeneratedFile::Lockfile),
294+
});
274295
}
275296
if let Some(vcs_info) = vcs_info {
276-
result.push(ArchiveFile {
277-
rel_path: PathBuf::from(VCS_INFO_FILE),
278-
rel_str: VCS_INFO_FILE.to_string(),
279-
contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
280-
});
281-
}
297+
let rel_str = VCS_INFO_FILE;
298+
result
299+
.entry(UncasedAscii::new(rel_str))
300+
.or_insert_with(Vec::new)
301+
.push(ArchiveFile {
302+
rel_path: PathBuf::from(rel_str),
303+
rel_str: rel_str.to_string(),
304+
contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
305+
});
306+
}
307+
308+
let mut result = result.into_values().flatten().collect();
282309
if let Some(license_file) = &pkg.manifest().metadata().license_file {
283310
let license_path = Path::new(license_file);
284311
let abs_file_path = paths::normalize_path(&pkg.root().join(license_path));

tests/testsuite/package.rs

+112
Original file line numberDiff line numberDiff line change
@@ -2983,3 +2983,115 @@ src/main.rs.bak
29832983
],
29842984
);
29852985
}
2986+
2987+
#[cargo_test]
2988+
#[cfg(windows)] // windows is the platform that is most consistently configured for case insensitive filesystems
2989+
fn normalize_case() {
2990+
let p = project()
2991+
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
2992+
.file("src/bar.txt", "") // should be ignored when packaging
2993+
.build();
2994+
// Workaround `project()` making a `Cargo.toml` on our behalf
2995+
std::fs::remove_file(p.root().join("Cargo.toml")).unwrap();
2996+
std::fs::write(
2997+
p.root().join("cargo.toml"),
2998+
r#"
2999+
[package]
3000+
name = "foo"
3001+
version = "0.0.1"
3002+
authors = []
3003+
exclude = ["*.txt"]
3004+
license = "MIT"
3005+
description = "foo"
3006+
"#,
3007+
)
3008+
.unwrap();
3009+
3010+
p.cargo("package")
3011+
.with_stderr(
3012+
"\
3013+
[WARNING] manifest has no documentation[..]
3014+
See [..]
3015+
[PACKAGING] foo v0.0.1 ([CWD])
3016+
[VERIFYING] foo v0.0.1 ([CWD])
3017+
[COMPILING] foo v0.0.1 ([CWD][..])
3018+
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
3019+
[PACKAGED] 4 files, [..] ([..] compressed)
3020+
",
3021+
)
3022+
.run();
3023+
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
3024+
p.cargo("package -l")
3025+
.with_stdout(
3026+
"\
3027+
Cargo.lock
3028+
Cargo.toml
3029+
Cargo.toml.orig
3030+
src/main.rs
3031+
",
3032+
)
3033+
.run();
3034+
p.cargo("package").with_stdout("").run();
3035+
3036+
let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
3037+
validate_crate_contents(
3038+
f,
3039+
"foo-0.0.1.crate",
3040+
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
3041+
&[],
3042+
);
3043+
}
3044+
3045+
#[cargo_test]
3046+
#[cfg(target_os = "linux")] // linux is generally configured to be case sensitive
3047+
fn mixed_case() {
3048+
let manifest = r#"
3049+
[package]
3050+
name = "foo"
3051+
version = "0.0.1"
3052+
authors = []
3053+
exclude = ["*.txt"]
3054+
license = "MIT"
3055+
description = "foo"
3056+
"#;
3057+
let p = project()
3058+
.file("Cargo.toml", manifest)
3059+
.file("cargo.toml", manifest)
3060+
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
3061+
.file("src/bar.txt", "") // should be ignored when packaging
3062+
.build();
3063+
3064+
p.cargo("package")
3065+
.with_stderr(
3066+
"\
3067+
[WARNING] manifest has no documentation[..]
3068+
See [..]
3069+
[PACKAGING] foo v0.0.1 ([CWD])
3070+
[VERIFYING] foo v0.0.1 ([CWD])
3071+
[COMPILING] foo v0.0.1 ([CWD][..])
3072+
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
3073+
[PACKAGED] 4 files, [..] ([..] compressed)
3074+
",
3075+
)
3076+
.run();
3077+
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
3078+
p.cargo("package -l")
3079+
.with_stdout(
3080+
"\
3081+
Cargo.lock
3082+
Cargo.toml
3083+
Cargo.toml.orig
3084+
src/main.rs
3085+
",
3086+
)
3087+
.run();
3088+
p.cargo("package").with_stdout("").run();
3089+
3090+
let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
3091+
validate_crate_contents(
3092+
f,
3093+
"foo-0.0.1.crate",
3094+
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
3095+
&[],
3096+
);
3097+
}

0 commit comments

Comments
 (0)