Auto merge of #2781 - alexcrichton:block-if-dirty, r=brson

Prevent packaging a crate if any files are dirty

This commit alters Cargo's behavior to prevent publishing a crate by default if
any files in that crate are determined to be dirty, that is either modified or
not part of the working tree.

This can prevent common mistakes like many listed in #2063 and enables features like #841.

Closes #1597
Closes #2063

Author: bors

src/bin/package.rs

@@ -11,6 +11,7 @@ pub struct Options {
     flag_no_verify: bool,
     flag_no_metadata: bool,
     flag_list: bool,
+    flag_allow_dirty: bool,
 }
 
 pub const USAGE: &'static str = "
@@ -24,6 +25,7 @@ Options:
     -l, --list              Print files included in a package without making one
     --no-verify             Don't verify the contents by building them
     --no-metadata           Ignore warnings about a lack of human-usable metadata
+    --allow-dirty           Allow dirty working directories to be packaged
     --manifest-path PATH    Path to the manifest to compile
     -v, --verbose           Use verbose output
     -q, --quiet             No output printed to stdout
@@ -36,9 +38,12 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
                                 options.flag_quiet,
                                 &options.flag_color));
     let root = try!(find_root_manifest_for_wd(options.flag_manifest_path, config.cwd()));
-    try!(ops::package(&root, config,
-                      !options.flag_no_verify,
-                      options.flag_list,
-                      !options.flag_no_metadata));
+    try!(ops::package(&root, &ops::PackageOpts {
+        config: config,
+        verify: !options.flag_no_verify,
+        list: options.flag_list,
+        check_metadata: !options.flag_no_metadata,
+        allow_dirty: options.flag_allow_dirty,
+    }));
     Ok(None)
 }

src/bin/publish.rs

@@ -11,6 +11,7 @@ pub struct Options {
     flag_quiet: Option<bool>,
     flag_color: Option<String>,
     flag_no_verify: bool,
+    flag_allow_dirty: bool,
 }
 
 pub const USAGE: &'static str = "
@@ -24,6 +25,7 @@ Options:
     --host HOST              Host to upload the package to
     --token TOKEN            Token to use when uploading
     --no-verify              Don't verify package tarball before publish
+    --allow-dirty            Allow publishing with a dirty source directory
     --manifest-path PATH     Path to the manifest of the package to publish
     -v, --verbose            Use verbose output
     -q, --quiet              No output printed to stdout
@@ -40,10 +42,17 @@ pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
         flag_host: host,
         flag_manifest_path,
         flag_no_verify: no_verify,
+        flag_allow_dirty: allow_dirty,
         ..
     } = options;
 
     let root = try!(find_root_manifest_for_wd(flag_manifest_path.clone(), config.cwd()));
-    try!(ops::publish(&root, config, token, host, !no_verify));
+    try!(ops::publish(&root, &ops::PublishOpts {
+        config: config,
+        token: token,
+        index: host,
+        verify: !no_verify,
+        allow_dirty: allow_dirty,
+    }));
     Ok(None)
 }

src/cargo/ops/cargo_package.rs

@@ -3,30 +3,37 @@ use std::io::SeekFrom;
 use std::io::prelude::*;
 use std::path::{self, Path};
 
-use tar::{Archive, Builder, Header};
-use flate2::{GzBuilder, Compression};
 use flate2::read::GzDecoder;
+use flate2::{GzBuilder, Compression};
+use git2;
+use tar::{Archive, Builder, Header};
 
 use core::{SourceId, Package, PackageId};
 use sources::PathSource;
 use util::{self, CargoResult, human, internal, ChainError, Config, FileLock};
 use ops;
 
+pub struct PackageOpts<'cfg> {
+    pub config: &'cfg Config,
+    pub list: bool,
+    pub check_metadata: bool,
+    pub allow_dirty: bool,
+    pub verify: bool,
+}
+
 pub fn package(manifest_path: &Path,
-               config: &Config,
-               verify: bool,
-               list: bool,
-               metadata: bool) -> CargoResult<Option<FileLock>> {
+               opts: &PackageOpts) -> CargoResult<Option<FileLock>> {
+    let config = opts.config;
     let path = manifest_path.parent().unwrap();
     let id = try!(SourceId::for_path(path));
     let mut src = PathSource::new(path, &id, config);
     let pkg = try!(src.root_package());
 
-    if metadata {
+    if opts.check_metadata {
         try!(check_metadata(&pkg, config));
     }
 
-    if list {
+    if opts.list {
         let root = pkg.root();
         let mut list: Vec<_> = try!(src.list_files(&pkg)).iter().map(|file| {
             util::without_prefix(&file, &root).unwrap().to_path_buf()
@@ -38,6 +45,10 @@ pub fn package(manifest_path: &Path,
         return Ok(None)
     }
 
+    if !opts.allow_dirty {
+        try!(check_not_dirty(&pkg, &src));
+    }
+
     let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
     let dir = config.target_dir(&pkg).join("package");
     let mut dst = match dir.open_ro(&filename, config, "packaged crate") {
@@ -57,7 +68,7 @@ pub fn package(manifest_path: &Path,
     try!(tar(&pkg, &src, config, dst.file(), &filename).chain_error(|| {
         human("failed to prepare local package for uploading")
     }));
-    if verify {
+    if opts.verify {
         try!(dst.seek(SeekFrom::Start(0)));
         try!(run_verify(config, &pkg, dst.file()).chain_error(|| {
             human("failed to verify package tarball")
@@ -109,6 +120,51 @@ fn check_metadata(pkg: &Package, config: &Config) -> CargoResult<()> {
     Ok(())
 }
 
+fn check_not_dirty(p: &Package, src: &PathSource) -> CargoResult<()> {
+    if let Ok(repo) = git2::Repository::discover(p.root()) {
+        if let Some(workdir) = repo.workdir() {
+            debug!("found a git repo at {:?}, checking if index present",
+                   workdir);
+            let path = p.manifest_path();
+            let path = path.strip_prefix(workdir).unwrap_or(path);
+            if let Ok(status) = repo.status_file(path) {
+                if (status & git2::STATUS_IGNORED).is_empty() {
+                    debug!("Cargo.toml found in repo, checking if dirty");
+                    return git(p, src, &repo)
+                }
+            }
+        }
+    }
+
+    // No VCS recognized, 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<()> {
+        let workdir = repo.workdir().unwrap();
+        let dirty = try!(src.list_files(p)).iter().filter(|file| {
+            let relative = file.strip_prefix(workdir).unwrap();
+            if let Ok(status) = repo.status_file(relative) {
+                status != git2::STATUS_CURRENT
+            } else {
+                false
+            }
+        }).map(|path| {
+            path.strip_prefix(p.root()).unwrap_or(path).display().to_string()
+        }).collect::<Vec<_>>();
+        if dirty.is_empty() {
+            Ok(())
+        } else {
+            bail!("{} dirty files found in the working directory:\n\n{}\n\n\
+                   to publish despite this, pass `--allow-dirty` to \
+                   `cargo publish`",
+                  dirty.len(), dirty.join("\n"))
+        }
+    }
+}
+
 fn tar(pkg: &Package,
        src: &PathSource,
        config: &Config,

src/cargo/ops/mod.rs

@@ -15,10 +15,10 @@ pub use self::cargo_generate_lockfile::{update_lockfile};
 pub use self::cargo_generate_lockfile::UpdateOptions;
 pub use self::lockfile::{load_pkg_lockfile, write_pkg_lockfile};
 pub use self::cargo_test::{run_tests, run_benches, TestOptions};
-pub use self::cargo_package::package;
+pub use self::cargo_package::{package, PackageOpts};
 pub use self::registry::{publish, registry_configuration, RegistryConfig};
 pub use self::registry::{registry_login, search, http_proxy_exists, http_handle};
-pub use self::registry::{modify_owners, yank, OwnersOptions};
+pub use self::registry::{modify_owners, yank, OwnersOptions, PublishOpts};
 pub use self::cargo_fetch::{fetch, get_resolved_packages};
 pub use self::cargo_pkgid::pkgid;
 pub use self::resolve::{resolve_pkg, resolve_with_previous};

src/cargo/ops/registry.rs

@@ -29,28 +29,39 @@ pub struct RegistryConfig {
     pub token: Option<String>,
 }
 
-pub fn publish(manifest_path: &Path,
-               config: &Config,
-               token: Option<String>,
-               index: Option<String>,
-               verify: bool) -> CargoResult<()> {
-    let pkg = try!(Package::for_path(&manifest_path, config));
+pub struct PublishOpts<'cfg> {
+    pub config: &'cfg Config,
+    pub token: Option<String>,
+    pub index: Option<String>,
+    pub verify: bool,
+    pub allow_dirty: bool,
+}
+
+pub fn publish(manifest_path: &Path, opts: &PublishOpts) -> CargoResult<()> {
+    let pkg = try!(Package::for_path(&manifest_path, opts.config));
 
     if !pkg.publish() {
         bail!("some crates cannot be published.\n\
                `{}` is marked as unpublishable", pkg.name());
     }
 
-    let (mut registry, reg_id) = try!(registry(config, token, index));
+    let (mut registry, reg_id) = try!(registry(opts.config,
+                                               opts.token.clone(),
+                                               opts.index.clone()));
     try!(verify_dependencies(&pkg, &reg_id));
 
     // Prepare a tarball, with a non-surpressable warning if metadata
     // is missing since this is being put online.
-    let tarball = try!(ops::package(manifest_path, config, verify,
-                                    false, true)).unwrap();
+    let tarball = try!(ops::package(manifest_path, &ops::PackageOpts {
+        config: opts.config,
+        verify: opts.verify,
+        list: false,
+        check_metadata: true,
+        allow_dirty: opts.allow_dirty,
+    })).unwrap();
 
     // Upload said tarball to the specified destination
-    try!(config.shell().status("Uploading", pkg.package_id().to_string()));
+    try!(opts.config.shell().status("Uploading", pkg.package_id().to_string()));
     try!(transmit(&pkg, tarball.file(), &mut registry));
 
     Ok(())

tests/package.rs

@@ -269,28 +269,6 @@ fn package_lib_with_bin() {
                 execs().with_status(0));
 }
 
-#[test]
-fn package_new_git_repo() {
-    let p = project("foo")
-        .file("Cargo.toml", r#"
-            [project]
-            name = "foo"
-            version = "0.0.1"
-        "#)
-        .file("src/main.rs", "fn main() {}");
-    p.build();
-    git2::Repository::init(&p.root()).unwrap();
-
-    assert_that(cargo_process().arg("package").cwd(p.root())
-                 .arg("--no-verify").arg("-v"),
-                execs().with_status(0).with_stderr("\
-[WARNING] manifest has no description[..]
-[PACKAGING] foo v0.0.1 ([..])
-[ARCHIVING] [..]
-[ARCHIVING] [..]
-"));
-}
-
 #[test]
 fn package_git_submodule() {
     let project = git::new("foo", |project| {