From 6b9961a382d0dae2bbd773599b6d7a7973b77e0f Mon Sep 17 00:00:00 2001 From: David Kellum Date: Mon, 23 Jul 2018 16:55:06 -0700 Subject: [PATCH 1/4] Add generated .cargo_vcs_info.json (git hash) file to `cargo package` --- src/cargo/ops/cargo_package.rs | 133 +++++++++++++++++++++++++++------ tests/testsuite/package.rs | 45 ++++++++--- tests/testsuite/support/git.rs | 4 + 3 files changed, 150 insertions(+), 32 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 822b519d7ca..770093434fe 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -1,12 +1,13 @@ use std::fs::{self, File}; -use std::io::SeekFrom; +use std::io::{Cursor, SeekFrom, Write}; use std::io::prelude::*; -use std::path::{self, Path}; +use std::path::{self, Path, PathBuf}; use std::sync::Arc; use flate2::read::GzDecoder; use flate2::{Compression, GzBuilder}; use git2; +use serde_json; use tar::{Archive, Builder, EntryType, Header}; use core::{Package, Source, SourceId, Workspace}; @@ -28,6 +29,8 @@ pub struct PackageOpts<'cfg> { pub registry: Option, } +static VCS_INFO_FILE: &'static str = ".cargo_vcs_info.json"; + pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult> { ops::resolve_ws(ws)?; let pkg = ws.current()?; @@ -42,6 +45,19 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult = src.list_files(pkg)? @@ -51,6 +67,9 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult CargoResult CargoResult CargoResult<()> { Ok(()) } -fn check_not_dirty(p: &Package, src: &PathSource, config: &Config) -> CargoResult<()> { +// Check if the package source is in a *git* DVCS repository. If *git*, and +// the source is *dirty* (e.g. has uncommited changes) and not `allow_dirty` +// then `bail!` with an informative message. Otherwise return the sha1 hash of +// the current *HEAD* commit, or `None` if *dirty*. +fn check_repo_state( + p: &Package, + src_files: &[PathBuf], + config: &Config, + allow_dirty: bool +) -> CargoResult> { if let Ok(repo) = git2::Repository::discover(p.root()) { if let Some(workdir) = repo.workdir() { debug!("found a git repo at {:?}", workdir); @@ -164,7 +188,7 @@ fn check_not_dirty(p: &Package, src: &PathSource, config: &Config) -> CargoResul "found (git) Cargo.toml at {:?} in workdir {:?}", path, workdir ); - return git(p, src, &repo); + return git(p, src_files, &repo, allow_dirty); } } config.shell().verbose(|shell| { @@ -182,11 +206,16 @@ fn check_not_dirty(p: &Package, src: &PathSource, config: &Config) -> CargoResul // No VCS with a checked in Cargo.toml found. so we don't know if the // directory is dirty or not, so we have to assume that it's clean. - return Ok(()); - - fn git(p: &Package, src: &PathSource, repo: &git2::Repository) -> CargoResult<()> { + return Ok(None); + + fn git( + p: &Package, + src_files: &[PathBuf], + repo: &git2::Repository, + allow_dirty: bool + ) -> CargoResult> { let workdir = repo.workdir().unwrap(); - let dirty = src.list_files(p)? + let dirty = src_files .iter() .filter(|file| { let relative = file.strip_prefix(workdir).unwrap(); @@ -204,20 +233,46 @@ fn check_not_dirty(p: &Package, src: &PathSource, config: &Config) -> CargoResul }) .collect::>(); if dirty.is_empty() { - Ok(()) + let rev_obj = repo.revparse_single("HEAD")?; + Ok(Some(rev_obj.id().to_string())) } else { - bail!( - "{} files in the working directory contain changes that were \ - not yet committed into git:\n\n{}\n\n\ - to proceed despite this, pass the `--allow-dirty` flag", - dirty.len(), - dirty.join("\n") - ) + if !allow_dirty { + bail!( + "{} files in the working directory contain changes that were \ + not yet committed into git:\n\n{}\n\n\ + to proceed despite this, pass the `--allow-dirty` flag", + dirty.len(), + dirty.join("\n") + ) + } + Ok(None) } } } -fn tar(ws: &Workspace, src: &PathSource, dst: &File, filename: &str) -> CargoResult<()> { +// Check for and `bail!` if a source file matches ROOT/VCS_INFO_FILE, since +// this is now a cargo reserved file name, and we don't want to allow +// forgery. +fn check_vcs_file_collision(pkg: &Package, src_files: &[PathBuf]) -> CargoResult<()> { + let root = pkg.root(); + let vcs_info_path = Path::new(VCS_INFO_FILE); + let collision = src_files.iter().find(|&p| { + util::without_prefix(&p, root).unwrap() == vcs_info_path + }); + if collision.is_some() { + bail!("Invalid inclusion of reserved file name \ + {} in package source", VCS_INFO_FILE); + } + Ok(()) +} + +fn tar( + ws: &Workspace, + src_files: &[PathBuf], + vcs_info: Option, + dst: &File, + filename: &str +) -> CargoResult<()> { // Prepare the encoder and its header let filename = Path::new(filename); let encoder = GzBuilder::new() @@ -229,7 +284,7 @@ fn tar(ws: &Workspace, src: &PathSource, dst: &File, filename: &str) -> CargoRes let pkg = ws.current()?; let config = ws.config(); let root = pkg.root(); - for file in src.list_files(pkg)?.iter() { + for file in src_files.iter() { let relative = util::without_prefix(file, root).unwrap(); check_filename(relative)?; let relative = relative.to_str().ok_or_else(|| { @@ -297,6 +352,38 @@ fn tar(ws: &Workspace, src: &PathSource, dst: &File, filename: &str) -> CargoRes } } + if let Some(ref json) = vcs_info { + let filename: PathBuf = Path::new(VCS_INFO_FILE).into(); + debug_assert!(check_filename(&filename).is_ok()); + let fnd = filename.display(); + config + .shell() + .verbose(|shell| shell.status("Archiving", &fnd))?; + let path = format!( + "{}-{}{}{}", + pkg.name(), + pkg.version(), + path::MAIN_SEPARATOR, + fnd + ); + let mut header = Header::new_ustar(); + header.set_path(&path).chain_err(|| { + format!("failed to add to archive: `{}`", fnd) + })?; + let mut buff = Cursor::new(Vec::with_capacity(256)); + writeln!(buff, "{}", serde_json::to_string_pretty(json)?)?; + let mut header = Header::new_ustar(); + header.set_path(&path)?; + header.set_entry_type(EntryType::file()); + header.set_mode(0o644); + header.set_size(buff.position() as u64); + header.set_cksum(); + buff.seek(SeekFrom::Start(0))?; + ar.append(&header, &mut buff).chain_err(|| { + internal(format!("could not archive source file `{}`", fnd)) + })?; + } + if include_lockfile(pkg) { let toml = paths::read(&ws.root().join("Cargo.lock"))?; let path = format!( diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 55fe71eb423..a66aa06e4fa 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -155,17 +155,17 @@ See http://doc.crates.io/manifest.html#package-metadata for more info. #[test] fn package_verbose() { let root = paths::root().join("all"); - let p = git::repo(&root) + let repo = git::repo(&root) .file("Cargo.toml", &basic_manifest("foo", "0.0.1")) .file("src/main.rs", "fn main() {}") .file("a/Cargo.toml", &basic_manifest("a", "0.0.1")) .file("a/src/lib.rs", "") .build(); - assert_that(cargo_process("build").cwd(p.root()), execs()); + assert_that(cargo_process("build").cwd(repo.root()), execs()); println!("package main repo"); assert_that( - cargo_process("package -v --no-verify").cwd(p.root()), + cargo_process("package -v --no-verify").cwd(repo.root()), execs().with_stderr( "\ [WARNING] manifest has no description[..] @@ -173,20 +173,46 @@ See http://doc.crates.io/manifest.html#package-metadata for more info. [PACKAGING] foo v0.0.1 ([..]) [ARCHIVING] [..] [ARCHIVING] [..] +[ARCHIVING] .cargo_vcs_info.json ", ), ); + let f = File::open(&repo.root().join("target/package/foo-0.0.1.crate")).unwrap(); + let mut rdr = GzDecoder::new(f); + let mut contents = Vec::new(); + rdr.read_to_end(&mut contents).unwrap(); + let mut ar = Archive::new(&contents[..]); + let mut entry = ar.entries().unwrap() + .map(|f| f.unwrap()) + .find(|e| e.path().unwrap().ends_with(".cargo_vcs_info.json")) + .unwrap(); + let mut contents = String::new(); + entry.read_to_string(&mut contents).unwrap(); + assert_eq!( + &contents[..], + &*format!( + r#"{{ + "git": {{ + "sha1": "{}" + }} +}} +"#, + repo.revparse_head() + ) + ); + println!("package sub-repo"); assert_that( - cargo_process("package -v --no-verify").cwd(p.root().join("a")), + cargo_process("package -v --no-verify").cwd(repo.root().join("a")), execs().with_stderr( "\ [WARNING] manifest has no description[..] See http://doc.crates.io/manifest.html#package-metadata for more info. [PACKAGING] a v0.0.1 ([..]) -[ARCHIVING] [..] -[ARCHIVING] [..] +[ARCHIVING] Cargo.toml +[ARCHIVING] src/lib.rs +[ARCHIVING] .cargo_vcs_info.json ", ), ); @@ -320,8 +346,6 @@ fn exclude() { "\ [WARNING] manifest has no description[..] See http://doc.crates.io/manifest.html#package-metadata for more info. -[WARNING] No (git) Cargo.toml found at `[..]` in workdir `[..]` -[PACKAGING] foo v0.0.1 ([..]) [WARNING] [..] file `dir_root_1/some_dir/file` WILL be excluded [..] See [..] [WARNING] [..] file `dir_root_2/some_dir/file` WILL be excluded [..] @@ -334,6 +358,8 @@ See [..] See [..] [WARNING] [..] file `some_dir/file_deep_1` WILL be excluded [..] See [..] +[WARNING] No (git) Cargo.toml found at `[..]` in workdir `[..]` +[PACKAGING] foo v0.0.1 ([..]) [ARCHIVING] [..] [ARCHIVING] [..] [ARCHIVING] [..] @@ -483,7 +509,7 @@ fn no_duplicates_from_modified_tracked_files() { .unwrap(); assert_that(cargo_process("build").cwd(p.root()), execs()); assert_that( - cargo_process("package --list").cwd(p.root()), + cargo_process("package --list --allow-dirty").cwd(p.root()), execs().with_stdout( "\ Cargo.toml @@ -1164,6 +1190,7 @@ fn package_lockfile_git_repo() { p.cargo("package").arg("-l").masquerade_as_nightly_cargo(), execs().with_stdout( "\ +.cargo_vcs_info.json Cargo.lock Cargo.toml src/main.rs diff --git a/tests/testsuite/support/git.rs b/tests/testsuite/support/git.rs index a35f65f3ad9..74c9e481e33 100644 --- a/tests/testsuite/support/git.rs +++ b/tests/testsuite/support/git.rs @@ -74,6 +74,10 @@ impl Repository { pub fn url(&self) -> Url { path2url(self.0.workdir().unwrap().to_path_buf()) } + + pub fn revparse_head(&self) -> String { + self.0.revparse_single("HEAD").expect("revparse HEAD").id().to_string() + } } pub fn new(name: &str, callback: F) -> Result From ab7877205f6127e8d123fae226420649bca60f96 Mon Sep 17 00:00:00 2001 From: David Kellum Date: Tue, 24 Jul 2018 14:53:30 -0700 Subject: [PATCH 2/4] Add test package::vcs_file_collision --- tests/testsuite/package.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index a66aa06e4fa..d7e3eb5f2b4 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -240,6 +240,42 @@ See http://doc.crates.io/manifest.html#package-metadata for more info. ); } +#[test] +fn vcs_file_collision() { + let p = project().build(); + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + description = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + documentation = "foo" + homepage = "foo" + repository = "foo" + exclude = ["*.no-existe"] + "#) + .file( + "src/main.rs", + r#" + fn main() {} + "#) + .file(".cargo_vcs_info.json", "foo") + .build(); + assert_that( + p.cargo("package").arg("--no-verify"), + execs().with_status(101).with_stderr(&format!( + "\ +[ERROR] Invalid inclusion of reserved file name .cargo_vcs_info.json \ +in package source +", + )), + ); +} + #[test] fn path_dependency_no_version() { let p = project() From d64c1687d9bbfba71d0a9ee0dba137e11d43dc7a Mon Sep 17 00:00:00 2001 From: David Kellum Date: Thu, 16 Aug 2018 12:33:32 -0700 Subject: [PATCH 3/4] Simplify json production and append to archive Per github review. --- src/cargo/ops/cargo_package.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 770093434fe..4dcca88b566 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -1,6 +1,6 @@ use std::fs::{self, File}; -use std::io::{Cursor, SeekFrom, Write}; use std::io::prelude::*; +use std::io::SeekFrom; use std::path::{self, Path, PathBuf}; use std::sync::Arc; @@ -370,16 +370,14 @@ fn tar( header.set_path(&path).chain_err(|| { format!("failed to add to archive: `{}`", fnd) })?; - let mut buff = Cursor::new(Vec::with_capacity(256)); - writeln!(buff, "{}", serde_json::to_string_pretty(json)?)?; + let json = format!("{}\n", serde_json::to_string_pretty(json)?); let mut header = Header::new_ustar(); header.set_path(&path)?; header.set_entry_type(EntryType::file()); header.set_mode(0o644); - header.set_size(buff.position() as u64); + header.set_size(json.len() as u64); header.set_cksum(); - buff.seek(SeekFrom::Start(0))?; - ar.append(&header, &mut buff).chain_err(|| { + ar.append(&header, json.as_bytes()).chain_err(|| { internal(format!("could not archive source file `{}`", fnd)) })?; } From 06dcc200d2c734fd8f079c378f054712e9ac8379 Mon Sep 17 00:00:00 2001 From: David Kellum Date: Thu, 16 Aug 2018 12:34:37 -0700 Subject: [PATCH 4/4] Fix a clippy lint --- src/cargo/ops/cargo_package.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 4dcca88b566..5967b6be978 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -92,7 +92,7 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult CargoResult fn tar( ws: &Workspace, src_files: &[PathBuf], - vcs_info: Option, + vcs_info: Option<&serde_json::Value>, dst: &File, filename: &str ) -> CargoResult<()> {