Auto merge of #3060 - rillian:package_path_deps, r=alexcrichton

Reject path-based dependencies in `cargo package`

`cargo publish` will complain if a package manifest contains a path, rather than registry+version-based dependency. Make `cargo package` do the same so that issue is caught sooner in developer workflow.

Author: bors

src/cargo/ops/cargo_package.rs

@@ -35,6 +35,8 @@ pub fn package(ws: &Workspace,
         try!(check_metadata(pkg, config));
     }
 
+    try!(verify_dependencies(&pkg));
+
     if opts.list {
         let root = pkg.root();
         let mut list: Vec<_> = try!(src.list_files(&pkg)).iter().map(|file| {
@@ -112,13 +114,27 @@ fn check_metadata(pkg: &Package, config: &Config) -> CargoResult<()> {
         things.push_str(&missing.last().unwrap());
 
         try!(config.shell().warn(
-            &format!("manifest has no {things}. \
+            &format!("manifest has no {things}.\n\
                     See http://doc.crates.io/manifest.html#package-metadata for more info.",
                     things = things)))
     }
     Ok(())
 }
 
+// check that the package dependencies are safe to deploy.
+fn verify_dependencies(pkg: &Package) -> CargoResult<()> {
+    for dep in pkg.dependencies() {
+        if dep.source_id().is_path() {
+            if !dep.specified_req() {
+                bail!("all path dependencies must have a version specified \
+                       when packaging.\ndependency `{}` does not specify \
+                       a version.", dep.name())
+            }
+        }
+    }
+    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() {

tests/package.rs

@@ -37,6 +37,7 @@ fn simple() {
     assert_that(p.cargo_process("package"),
                 execs().with_status(0).with_stderr(&format!("\
 [WARNING] manifest has no documentation[..]
+See [..]
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 ({dir}[..])
@@ -82,8 +83,8 @@ fn metadata_warning() {
     assert_that(p.cargo_process("package"),
                 execs().with_status(0).with_stderr(&format!("\
 warning: manifest has no description, license, license-file, documentation, \
-homepage or repository. See \
-http://doc.crates.io/manifest.html#package-metadata for more info.
+homepage or repository.
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 ({dir}[..])
@@ -104,8 +105,8 @@ http://doc.crates.io/manifest.html#package-metadata for more info.
         "#);
     assert_that(p.cargo_process("package"),
                 execs().with_status(0).with_stderr(&format!("\
-warning: manifest has no description, documentation, homepage or repository. See \
-http://doc.crates.io/manifest.html#package-metadata for more info.
+warning: manifest has no description, documentation, homepage or repository.
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 ({dir}[..])
@@ -165,6 +166,7 @@ fn package_verbose() {
     assert_that(cargo.clone().arg("package").arg("-v").arg("--no-verify"),
                 execs().with_status(0).with_stderr("\
 [WARNING] manifest has no description[..]
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ([..])
 [ARCHIVING] [..]
 [ARCHIVING] [..]
@@ -175,6 +177,7 @@ fn package_verbose() {
                      .cwd(p.root().join("a")),
                 execs().with_status(0).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] [..]
@@ -198,6 +201,7 @@ fn package_verification() {
     assert_that(p.cargo("package"),
                 execs().with_status(0).with_stderr(&format!("\
 [WARNING] manifest has no description[..]
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 ({dir}[..])
@@ -206,6 +210,38 @@ fn package_verification() {
         dir = p.url())));
 }
 
+#[test]
+fn path_dependency_no_version() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+
+            [dependencies.bar]
+            path = "bar"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("bar/src/lib.rs", "");
+
+    assert_that(p.cargo_process("package"),
+                execs().with_status(101).with_stderr("\
+[WARNING] manifest has no documentation, homepage or repository.
+See http://doc.crates.io/manifest.html#package-metadata for more info.
+[ERROR] all path dependencies must have a version specified when packaging.
+dependency `bar` does not specify a version.
+"));
+}
+
 #[test]
 fn exclude() {
     let p = project("foo")
@@ -225,6 +261,7 @@ fn exclude() {
     assert_that(p.cargo_process("package").arg("--no-verify").arg("-v"),
                 execs().with_status(0).with_stderr("\
 [WARNING] manifest has no description[..]
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ([..])
 [ARCHIVING] [..]
 [ARCHIVING] [..]
@@ -251,6 +288,7 @@ fn include() {
     assert_that(p.cargo_process("package").arg("--no-verify").arg("-v"),
                 execs().with_status(0).with_stderr("\
 [WARNING] manifest has no description[..]
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] foo v0.0.1 ([..])
 [ARCHIVING] [..]
 [ARCHIVING] [..]
@@ -360,6 +398,7 @@ fn ignore_nested() {
     assert_that(p.cargo_process("package"),
                 execs().with_status(0).with_stderr(&format!("\
 [WARNING] manifest has no documentation[..]
+See http://doc.crates.io/manifest.html#package-metadata for more info.
 [PACKAGING] nested v0.0.1 ({dir})
 [VERIFYING] nested v0.0.1 ({dir})
 [COMPILING] nested v0.0.1 ({dir}[..])
@@ -408,6 +447,7 @@ fn package_weird_characters() {
     assert_that(p.cargo_process("package"),
                 execs().with_status(101).with_stderr("\
 warning: [..]
+See [..]
 [PACKAGING] foo [..]
 [ERROR] failed to prepare local package for uploading
 
@@ -448,6 +488,7 @@ fn repackage_on_source_change() {
     // Check that cargo rebuilds the tarball
     assert_that(pro, execs().with_status(0).with_stderr(&format!("\
 [WARNING] [..]
+See [..]
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 ({dir}[..])

tests/publish.rs

@@ -60,6 +60,7 @@ fn simple() {
                 execs().with_status(0).with_stderr(&format!("\
 [UPDATING] registry `{reg}`
 [WARNING] manifest has no documentation, [..]
+See [..]
 [PACKAGING] foo v0.0.1 ({dir})
 [UPLOADING] foo v0.0.1 ({dir})
 ",
@@ -357,6 +358,7 @@ fn dry_run() {
                 execs().with_status(0).with_stderr(&format!("\
 [UPDATING] registry `[..]`
 [WARNING] manifest has no documentation, [..]
+See [..]
 [PACKAGING] foo v0.0.1 ({dir})
 [VERIFYING] foo v0.0.1 ({dir})
 [COMPILING] foo v0.0.1 [..]