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(package): Recognize and normalize cargo.toml #12399

Merged
merged 6 commits into from
Jul 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ thiserror = "1.0.40"
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
toml = "0.7.0"
toml_edit = "0.19.0"
unicase = "2.6.0"
unicode-width = "0.1.5"
unicode-xid = "0.2.0"
url = "2.2.2"
Expand Down Expand Up @@ -177,6 +178,7 @@ termcolor.workspace = true
time.workspace = true
toml.workspace = true
toml_edit.workspace = true
unicase.workspace = true
unicode-width.workspace = true
unicode-xid.workspace = true
url.workspace = true
Expand Down
103 changes: 65 additions & 38 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use flate2::{Compression, GzBuilder};
use log::debug;
use serde::Serialize;
use tar::{Archive, Builder, EntryType, Header, HeaderMode};
use unicase::Ascii as UncasedAscii;

pub struct PackageOpts<'cfg> {
pub config: &'cfg Config,
Expand Down Expand Up @@ -227,58 +228,84 @@ fn build_ar_list(
src_files: Vec<PathBuf>,
vcs_info: Option<VcsInfo>,
) -> CargoResult<Vec<ArchiveFile>> {
let mut result = Vec::new();
let mut result = HashMap::new();
let root = pkg.root();
for src_file in src_files {
let rel_path = src_file.strip_prefix(&root)?.to_path_buf();
check_filename(&rel_path, &mut ws.config().shell())?;
let rel_str = rel_path
.to_str()
.ok_or_else(|| {
anyhow::format_err!("non-utf8 path in source directory: {}", rel_path.display())
})?
.to_string();

for src_file in &src_files {
let rel_path = src_file.strip_prefix(&root)?;
check_filename(rel_path, &mut ws.config().shell())?;
let rel_str = rel_path.to_str().ok_or_else(|| {
anyhow::format_err!("non-utf8 path in source directory: {}", rel_path.display())
})?;
match rel_str.as_ref() {
"Cargo.toml" => {
result.push(ArchiveFile {
rel_path: PathBuf::from(ORIGINAL_MANIFEST_FILE),
rel_str: ORIGINAL_MANIFEST_FILE.to_string(),
contents: FileContents::OnDisk(src_file),
});
result.push(ArchiveFile {
rel_path,
rel_str,
contents: FileContents::Generated(GeneratedFile::Manifest),
});
}
"Cargo.lock" => continue,
VCS_INFO_FILE | ORIGINAL_MANIFEST_FILE => anyhow::bail!(
"invalid inclusion of reserved file name {} in package source",
rel_str
),
_ => {
result.push(ArchiveFile {
rel_path,
rel_str,
contents: FileContents::OnDisk(src_file),
});
result
.entry(UncasedAscii::new(rel_str))
.or_insert_with(Vec::new)
.push(ArchiveFile {
rel_path: rel_path.to_owned(),
rel_str: rel_str.to_owned(),
contents: FileContents::OnDisk(src_file.clone()),
});
}
}
}

// Ensure we normalize for case insensitive filesystems (like on Windows) by removing the
// existing entry, regardless of case, and adding in with the correct case
if result.remove(&UncasedAscii::new("Cargo.toml")).is_some() {
result
.entry(UncasedAscii::new(ORIGINAL_MANIFEST_FILE))
.or_insert_with(Vec::new)
.push(ArchiveFile {
rel_path: PathBuf::from(ORIGINAL_MANIFEST_FILE),
rel_str: ORIGINAL_MANIFEST_FILE.to_string(),
contents: FileContents::OnDisk(pkg.manifest_path().to_owned()),
});
result
.entry(UncasedAscii::new("Cargo.toml"))
.or_insert_with(Vec::new)
.push(ArchiveFile {
rel_path: PathBuf::from("Cargo.toml"),
rel_str: "Cargo.toml".to_string(),
contents: FileContents::Generated(GeneratedFile::Manifest),
});
} else {
ws.config().shell().warn(&format!(
"no `Cargo.toml` file found when packaging `{}` (note the case of the file name).",
pkg.name()
))?;
}

if pkg.include_lockfile() {
result.push(ArchiveFile {
rel_path: PathBuf::from("Cargo.lock"),
rel_str: "Cargo.lock".to_string(),
contents: FileContents::Generated(GeneratedFile::Lockfile),
});
let rel_str = "Cargo.lock";
result
.entry(UncasedAscii::new(rel_str))
.or_insert_with(Vec::new)
.push(ArchiveFile {
rel_path: PathBuf::from(rel_str),
rel_str: rel_str.to_string(),
contents: FileContents::Generated(GeneratedFile::Lockfile),
});
}
if let Some(vcs_info) = vcs_info {
result.push(ArchiveFile {
rel_path: PathBuf::from(VCS_INFO_FILE),
rel_str: VCS_INFO_FILE.to_string(),
contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
});
}
let rel_str = VCS_INFO_FILE;
result
.entry(UncasedAscii::new(rel_str))
.or_insert_with(Vec::new)
.push(ArchiveFile {
rel_path: PathBuf::from(rel_str),
rel_str: rel_str.to_string(),
contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
});
}

let mut result = result.into_values().flatten().collect();
if let Some(license_file) = &pkg.manifest().metadata().license_file {
let license_path = Path::new(license_file);
let abs_file_path = paths::normalize_path(&pkg.root().join(license_path));
Expand Down
112 changes: 112 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2983,3 +2983,115 @@ src/main.rs.bak
],
);
}

#[cargo_test]
#[cfg(windows)] // windows is the platform that is most consistently configured for case insensitive filesystems
fn normalize_case() {
let p = project()
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
.file("src/bar.txt", "") // should be ignored when packaging
.build();
// Workaround `project()` making a `Cargo.toml` on our behalf
std::fs::remove_file(p.root().join("Cargo.toml")).unwrap();
std::fs::write(
p.root().join("cargo.toml"),
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
exclude = ["*.txt"]
license = "MIT"
description = "foo"
"#,
Comment on lines +2999 to +3006
Copy link
Member

Choose a reason for hiding this comment

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

nit: One less indentation?

)
.unwrap();

p.cargo("package")
.with_stderr(
"\
[WARNING] manifest has no documentation[..]
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[VERIFYING] foo v0.0.1 ([CWD])
[COMPILING] foo v0.0.1 ([CWD][..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[PACKAGED] 4 files, [..] ([..] compressed)
",
)
.run();
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
p.cargo("package -l")
.with_stdout(
"\
Cargo.lock
Cargo.toml
Cargo.toml.orig
src/main.rs
",
)
.run();
p.cargo("package").with_stdout("").run();

let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
validate_crate_contents(
f,
"foo-0.0.1.crate",
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
&[],
);
}

#[cargo_test]
#[cfg(target_os = "linux")] // linux is generally configured to be case sensitive
fn mixed_case() {
let manifest = r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
exclude = ["*.txt"]
license = "MIT"
description = "foo"
"#;
let p = project()
.file("Cargo.toml", manifest)
.file("cargo.toml", manifest)
weihanglo marked this conversation as resolved.
Show resolved Hide resolved
.file("src/main.rs", r#"fn main() { println!("hello"); }"#)
.file("src/bar.txt", "") // should be ignored when packaging
.build();

p.cargo("package")
.with_stderr(
"\
[WARNING] manifest has no documentation[..]
See [..]
[PACKAGING] foo v0.0.1 ([CWD])
[VERIFYING] foo v0.0.1 ([CWD])
[COMPILING] foo v0.0.1 ([CWD][..])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
[PACKAGED] 4 files, [..] ([..] compressed)
",
)
.run();
assert!(p.root().join("target/package/foo-0.0.1.crate").is_file());
p.cargo("package -l")
.with_stdout(
"\
Cargo.lock
Cargo.toml
Cargo.toml.orig
src/main.rs
",
)
.run();
p.cargo("package").with_stdout("").run();

let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
validate_crate_contents(
f,
"foo-0.0.1.crate",
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
&[],
);
}