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 subsequently...

Closes #2063
This commit is contained in:
Alex Crichton 2016-06-10 06:43:34 -07:00
parent b5479d90e4
commit 088d14ad3d
7 changed files with 262 additions and 48 deletions

View 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)
}

View 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)
}

View 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,

View 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};

View 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(())

View 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| {

View 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));
}