Auto merge of #9612 - Bryysen:master, r=alexcrichton

Error when packaging with git dependencies without version

If `cargo package` is run on a package that specifies a git dependency
without a version, cargo will error. This should help with clarifying
that git dependencies will be stripped when packaging, and that the
dependency has to be fetched from a registry.
Closes #9442

src/cargo/ops/cargo_package.rs

@@ -105,7 +105,10 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
         return Ok(None);
     }
 
-    verify_dependencies(pkg)?;
+    // Check that the package dependencies are safe to deploy.
+    for dep in pkg.dependencies() {
+        super::check_dep_has_version(dep, false)?;
+    }
 
     let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
     let dir = ws.target_dir().join("package");
@@ -333,21 +336,6 @@ fn check_metadata(pkg: &Package, config: &Config) -> CargoResult<()> {
     Ok(())
 }
 
-// Checks 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() && !dep.specified_req() && dep.is_transitive() {
-            anyhow::bail!(
-                "all path dependencies must have a version specified \
-                 when packaging.\ndependency `{}` does not specify \
-                 a version.",
-                dep.name_in_toml()
-            )
-        }
-    }
-    Ok(())
-}
-
 /// Checks if the package source is in a *git* DVCS repository. If *git*, and
 /// the source is *dirty* (e.g., has uncommitted changes) then `bail!` with an
 /// informative message. Otherwise return the sha1 hash of the current *HEAD*

src/cargo/ops/mod.rs

@@ -51,3 +51,35 @@ mod registry;
 mod resolve;
 pub mod tree;
 mod vendor;
+
+/// Returns true if the dependency is either git or path, false otherwise
+/// Error if a git/path dep is transitive, but has no version (registry source).
+/// This check is performed on dependencies before publishing or packaging
+fn check_dep_has_version(dep: &crate::core::Dependency, publish: bool) -> crate::CargoResult<bool> {
+    let which = if dep.source_id().is_path() {
+        "path"
+    } else if dep.source_id().is_git() {
+        "git"
+    } else {
+        return Ok(false);
+    };
+
+    if !dep.specified_req() && dep.is_transitive() {
+        let dep_version_source = dep.registry_id().map_or_else(
+            || "crates.io".to_string(),
+            |registry_id| registry_id.display_registry_name(),
+        );
+        anyhow::bail!(
+            "all dependencies must have a version specified when {}.\n\
+             dependency `{}` does not specify a version\n\
+             Note: The {} dependency will use the version from {},\n\
+             the `{}` specification will be removed from the dependency declaration.",
+            if publish { "publishing" } else { "packaging" },
+            dep.package_name(),
+            if publish { "published" } else { "packaged" },
+            dep_version_source,
+            which,
+        )
+    }
+    Ok(true)
+}

src/cargo/ops/registry.rs

@@ -138,34 +138,12 @@ fn verify_dependencies(
     registry_src: SourceId,
 ) -> CargoResult<()> {
     for dep in pkg.dependencies().iter() {
-        if dep.source_id().is_path() || dep.source_id().is_git() {
-            if !dep.specified_req() {
-                if !dep.is_transitive() {
-                    // dev-dependencies will be stripped in TomlManifest::prepare_for_publish
-                    continue;
-                }
-                let which = if dep.source_id().is_path() {
-                    "path"
-                } else {
-                    "git"
-                };
-                let dep_version_source = dep.registry_id().map_or_else(
-                    || "crates.io".to_string(),
-                    |registry_id| registry_id.display_registry_name(),
-                );
-                bail!(
-                    "all dependencies must have a version specified when publishing.\n\
-                     dependency `{}` does not specify a version\n\
-                     Note: The published dependency will use the version from {},\n\
-                     the `{}` specification will be removed from the dependency declaration.",
-                    dep.package_name(),
-                    dep_version_source,
-                    which,
-                )
-            }
+        if super::check_dep_has_version(dep, true)? {
+            continue;
+        }
         // TomlManifest::prepare_for_publish will rewrite the dependency
         // to be just the `version` field.
-        } else if dep.source_id() != registry_src {
+        if dep.source_id() != registry_src {
             if !dep.source_id().is_registry() {
                 // Consider making SourceId::kind a public type that we can
                 // exhaustively match on. Using match can help ensure that

tests/testsuite/package.rs

@@ -287,8 +287,47 @@ fn path_dependency_no_version() {
             "\
 [WARNING] manifest has no documentation, homepage or repository.
 See https://doc.rust-lang.org/cargo/reference/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.
+[ERROR] all dependencies must have a version specified when packaging.
+dependency `bar` does not specify a version\n\
+Note: The packaged dependency will use the version from crates.io,
+the `path` specification will be removed from the dependency declaration.
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn git_dependency_no_version() {
+    registry::init();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+                license = "MIT"
+                description = "foo"
+
+                [dependencies.foo]
+                git = "git://path/to/nowhere"
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("package")
+        .with_status(101)
+        .with_stderr(
+            "\
+[WARNING] manifest has no documentation, homepage or repository.
+See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
+[ERROR] all dependencies must have a version specified when packaging.
+dependency `foo` does not specify a version
+Note: The packaged dependency will use the version from crates.io,
+the `git` specification will be removed from the dependency declaration.
 ",
         )
         .run();
@@ -1895,8 +1934,10 @@ src/main.rs
         .with_status(101)
         .with_stderr(
             "\
-error: all path dependencies must have a version specified when packaging.
-dependency `bar` does not specify a version.
+[ERROR] all dependencies must have a version specified when packaging.
+dependency `bar` does not specify a version
+Note: The packaged dependency will use the version from crates.io,
+the `path` specification will be removed from the dependency declaration.
 ",
         )
         .run();