From b64b605236bdc5ef0f5fcd34e533ee14954d8b2f Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Wed, 18 Jan 2023 22:19:56 -0600 Subject: [PATCH] fix(vendor): Make `vendor` use `Manifest`'s "original" `Cargo.toml` --- src/cargo/ops/vendor.rs | 67 ++++++++++++++----- tests/testsuite/vendor.rs | 136 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 17 deletions(-) diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index 1239289d684..23c538e0f52 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -1,5 +1,6 @@ +use crate::core::package::MANIFEST_PREAMBLE; use crate::core::shell::Verbosity; -use crate::core::{GitReference, Workspace}; +use crate::core::{GitReference, Package, Workspace}; use crate::ops; use crate::sources::path::PathSource; use crate::sources::CRATES_IO_REGISTRY; @@ -9,6 +10,7 @@ use cargo_util::{paths, Sha256}; use serde::Serialize; use std::collections::HashSet; use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::ffi::OsStr; use std::fs::{self, File, OpenOptions}; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; @@ -225,7 +227,7 @@ fn sync( let pathsource = PathSource::new(src, id.source_id(), config); let paths = pathsource.list_files(pkg)?; let mut map = BTreeMap::new(); - cp_sources(src, &paths, &dst, &mut map, &mut tmp_buf) + cp_sources(pkg, src, &paths, &dst, &mut map, &mut tmp_buf) .with_context(|| format!("failed to copy over vendored sources for: {}", id))?; // Finally, emit the metadata about this package @@ -315,6 +317,7 @@ fn sync( } fn cp_sources( + pkg: &Package, src: &Path, paths: &[PathBuf], dst: &Path, @@ -354,25 +357,55 @@ fn cp_sources( .fold(dst.to_owned(), |acc, component| acc.join(&component)); paths::create_dir_all(dst.parent().unwrap())?; + let mut dst_opts = OpenOptions::new(); + dst_opts.write(true).create(true).truncate(true); + // When vendoring git dependencies, the manifest has not been normalized like it would be + // when published. This causes issue when the manifest is using workspace inheritance. + // To get around this issue we use the "original" manifest after `{}.workspace = true` + // has been resolved for git dependencies. + let cksum = if dst.file_name() == Some(OsStr::new("Cargo.toml")) + && pkg.package_id().source_id().is_git() + { + let original_toml = toml::to_string_pretty(pkg.manifest().original())?; + let contents = format!("{}\n{}", MANIFEST_PREAMBLE, original_toml); + copy_and_checksum( + &dst, + &mut dst_opts, + &mut contents.as_bytes(), + "Generated Cargo.toml", + tmp_buf, + )? + } else { + let mut src = File::open(&p).with_context(|| format!("failed to open {:?}", &p))?; + #[cfg(unix)] + { + use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; + let src_metadata = src + .metadata() + .with_context(|| format!("failed to stat {:?}", p))?; + dst_opts.mode(src_metadata.mode()); + } + copy_and_checksum( + &dst, + &mut dst_opts, + &mut src, + &p.display().to_string(), + tmp_buf, + )? + }; - let cksum = copy_and_checksum(p, &dst, tmp_buf)?; cksums.insert(relative.to_str().unwrap().replace("\\", "/"), cksum); } Ok(()) } -fn copy_and_checksum(src_path: &Path, dst_path: &Path, buf: &mut [u8]) -> CargoResult { - let mut src = File::open(src_path).with_context(|| format!("failed to open {:?}", src_path))?; - let mut dst_opts = OpenOptions::new(); - dst_opts.write(true).create(true).truncate(true); - #[cfg(unix)] - { - use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; - let src_metadata = src - .metadata() - .with_context(|| format!("failed to stat {:?}", src_path))?; - dst_opts.mode(src_metadata.mode()); - } +fn copy_and_checksum( + dst_path: &Path, + dst_opts: &mut OpenOptions, + contents: &mut T, + contents_path: &str, + buf: &mut [u8], +) -> CargoResult { let mut dst = dst_opts .open(dst_path) .with_context(|| format!("failed to create {:?}", dst_path))?; @@ -380,9 +413,9 @@ fn copy_and_checksum(src_path: &Path, dst_path: &Path, buf: &mut [u8]) -> CargoR // shouldn't be any under normal conditions. let mut cksum = Sha256::new(); loop { - let n = src + let n = contents .read(buf) - .with_context(|| format!("failed to read from {:?}", src_path))?; + .with_context(|| format!("failed to read from {:?}", contents_path))?; if n == 0 { break Ok(cksum.finish_hex()); } diff --git a/tests/testsuite/vendor.rs b/tests/testsuite/vendor.rs index 5a35320962a..9392b29737b 100644 --- a/tests/testsuite/vendor.rs +++ b/tests/testsuite/vendor.rs @@ -789,6 +789,79 @@ Caused by: .run(); } +#[cargo_test] +fn git_complex() { + let git_b = git::new("git_b", |p| { + p.file( + "Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [dependencies] + dep_b = { path = 'dep_b' } + "#, + ) + .file("src/lib.rs", "") + .file("dep_b/Cargo.toml", &basic_lib_manifest("dep_b")) + .file("dep_b/src/lib.rs", "") + }); + + let git_a = git::new("git_a", |p| { + p.file( + "Cargo.toml", + &format!( + r#" + [package] + name = "a" + version = "0.1.0" + + [dependencies] + b = {{ git = '{}' }} + dep_a = {{ path = 'dep_a' }} + "#, + git_b.url() + ), + ) + .file("src/lib.rs", "") + .file("dep_a/Cargo.toml", &basic_lib_manifest("dep_a")) + .file("dep_a/src/lib.rs", "") + }); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + a = {{ git = '{}' }} + "#, + git_a.url() + ), + ) + .file("src/lib.rs", "") + .build(); + + let output = p + .cargo("vendor --respect-source-config") + .exec_with_output() + .unwrap(); + let output = String::from_utf8(output.stdout).unwrap(); + p.change_file(".cargo/config", &output); + + p.cargo("check -v") + .with_stderr_contains("[..]foo/vendor/a/src/lib.rs[..]") + .with_stderr_contains("[..]foo/vendor/dep_a/src/lib.rs[..]") + .with_stderr_contains("[..]foo/vendor/b/src/lib.rs[..]") + .with_stderr_contains("[..]foo/vendor/dep_b/src/lib.rs[..]") + .run(); +} + #[cargo_test] fn depend_on_vendor_dir_not_deleted() { let p = project() @@ -1015,3 +1088,66 @@ fn no_remote_dependency_no_vendor() { .run(); assert!(!p.root().join("vendor").exists()); } + +#[cargo_test] +fn vendor_crate_with_ws_inherit() { + let git = git::new("ws", |p| { + p.file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [workspace.package] + version = "0.1.0" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version.workspace = true + "#, + ) + .file("bar/src/lib.rs", "") + }); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = {{ git = '{}' }} + "#, + git.url() + ), + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("vendor --respect-source-config").run(); + p.change_file( + ".cargo/config", + &format!( + r#" + [source."{}"] + git = "{}" + replace-with = "vendor" + + [source.vendor] + directory = "vendor" + "#, + git.url(), + git.url() + ), + ); + + p.cargo("check -v") + .with_stderr_contains("[..]foo/vendor/bar/src/lib.rs[..]") + .run(); +}