]> git.proxmox.com Git - cargo.git/commitdiff
Prevent packaging a crate if any files are dirty
authorAlex Crichton <alex@alexcrichton.com>
Fri, 10 Jun 2016 13:43:34 +0000 (06:43 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Fri, 10 Jun 2016 14:18:01 +0000 (07:18 -0700)
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 subsequently...

Closes #2063

src/bin/package.rs
src/bin/publish.rs
src/cargo/ops/cargo_package.rs
src/cargo/ops/mod.rs
src/cargo/ops/registry.rs
tests/package.rs
tests/publish.rs

index 37dd9a122d402d71952bc4d7bcb4a4bc1885215a..be4a94cad6c0402c468a94c9508f794ead9fea73 100644 (file)
@@ -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)
 }
index dac40fae3e2c367ca3aff462e42701b4859c91af..73466c27fbbd1aff94c7a911357b818598a0da67 100644 (file)
@@ -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)
 }
index a4758e737a081c8cddceb654bd5e94e87fffa3cd..77113795bc31dd5c8f149688fade532e14e34833 100644 (file)
@@ -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,
index be772fe8c04dd8d872af10447faf7a45c769b232..d6e41a2d35faffe522b3aa83985f1ac79cbbb729 100644 (file)
@@ -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};
index 49764c67482b169377934510872f3afb5cd5917a..a802165f728b436140379f8fceeaaa5072e69dc7 100644 (file)
@@ -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(())
index 7bc7dcdfe94ab59362a4d6775488eb3bd8db3eb5..03ccb85115757c2f79cab3b92aea3e7a7d8d0045 100644 (file)
@@ -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| {
index 37299bc0d663e6d6068a2e0d51848be2a4457254..b823672f93360498cf591bd7ace39f972f3cd845 100644 (file)
@@ -1,3 +1,4 @@
+#[macro_use]
 extern crate cargotest;
 extern crate flate2;
 extern crate hamcrest;
@@ -172,3 +173,157 @@ fn unpublishable_crate() {
 `foo` is marked as unpublishable
 "));
 }
+
+#[test]
+fn dont_publish_dirty() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    let p = project("foo");
+    t!(File::create(p.root().join("bar")));
+    assert_that(p.cargo("publish"),
+                execs().with_status(101).with_stderr("\
+[UPDATING] registry `[..]`
+error: 1 dirty files found in the working directory:
+
+bar
+
+to publish despite this, pass `--allow-dirty` to `cargo publish`
+"));
+}
+
+#[test]
+fn publish_clean() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    let p = project("foo");
+    assert_that(p.cargo("publish"),
+                execs().with_status(0));
+}
+
+#[test]
+fn publish_in_sub_repo() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .file("bar/Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .file("bar/src/main.rs", "fn main() {}")
+        .build();
+
+    let p = project("foo");
+    t!(File::create(p.root().join("baz")));
+    assert_that(p.cargo("publish").cwd(p.root().join("bar")),
+                execs().with_status(0));
+}
+
+#[test]
+fn publish_when_ignored() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file(".gitignore", "baz")
+        .build();
+
+    let p = project("foo");
+    t!(File::create(p.root().join("baz")));
+    assert_that(p.cargo("publish"),
+                execs().with_status(0));
+}
+
+#[test]
+fn ignore_when_crate_ignored() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .file(".gitignore", "bar")
+        .nocommit_file("bar/Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .nocommit_file("bar/src/main.rs", "fn main() {}");
+    let p = project("foo");
+    t!(File::create(p.root().join("bar/baz")));
+    assert_that(p.cargo("publish").cwd(p.root().join("bar")),
+                execs().with_status(0));
+}
+
+#[test]
+fn new_crate_rejected() {
+    setup();
+
+    repo(&paths::root().join("foo"))
+        .nocommit_file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+            documentation = "foo"
+            homepage = "foo"
+            repository = "foo"
+        "#)
+        .nocommit_file("src/main.rs", "fn main() {}");
+    let p = project("foo");
+    t!(File::create(p.root().join("baz")));
+    assert_that(p.cargo("publish"),
+                execs().with_status(101));
+}