Auto merge of rust-lang#8175 - ehuss:allow-package-list, r=alexcrichton

Allow `cargo package --list` even for things that don't package.

`cargo package --list` was changed in rust-lang#7905 to generate `Cargo.lock` earlier. If there is a problem, then it would fail where previously it would succeed. This changes it so that file generation is deferred until after `--list`.

This also changes it so that the "dependencies must have a version" check is deferred until after `--list` as well.

Closes rust-lang#8151

src/cargo/ops/cargo_package.rs

@@ -50,8 +50,17 @@ struct ArchiveFile {
 enum FileContents {
     /// Absolute path to the file on disk to add to the archive.
     OnDisk(PathBuf),
-    /// Contents of a file generated in memory.
-    Generated(String),
+    /// Generates a file.
+    Generated(GeneratedFile),
+}
+
+enum GeneratedFile {
+    /// Generates `Cargo.toml` by rewriting the original.
+    Manifest,
+    /// Generates `Cargo.lock` in some cases (like if there is a binary).
+    Lockfile,
+    /// Adds a `.cargo-vcs_info.json` file if in a (clean) git repo.
+    VcsInfo(String),
 }
 
 pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option<FileLock>> {
@@ -71,8 +80,6 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
         check_metadata(pkg, config)?;
     }
 
-    verify_dependencies(pkg)?;
-
     if !pkg.manifest().exclude().is_empty() && !pkg.manifest().include().is_empty() {
         config.shell().warn(
             "both package.include and package.exclude are specified; \
@@ -100,6 +107,8 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
         return Ok(None);
     }
 
+    verify_dependencies(pkg)?;
+
     let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
     let dir = ws.target_dir().join("package");
     let mut dst = {
@@ -156,11 +165,10 @@ fn build_ar_list(
                     rel_str: "Cargo.toml.orig".to_string(),
                     contents: FileContents::OnDisk(src_file),
                 });
-                let generated = pkg.to_registry_toml(ws)?;
                 result.push(ArchiveFile {
                     rel_path,
                     rel_str,
-                    contents: FileContents::Generated(generated),
+                    contents: FileContents::Generated(GeneratedFile::Manifest),
                 });
             }
             "Cargo.lock" => continue,
@@ -179,18 +187,17 @@ fn build_ar_list(
         }
     }
     if pkg.include_lockfile() {
-        let new_lock = build_lock(ws)?;
         result.push(ArchiveFile {
             rel_path: PathBuf::from("Cargo.lock"),
             rel_str: "Cargo.lock".to_string(),
-            contents: FileContents::Generated(new_lock),
+            contents: FileContents::Generated(GeneratedFile::Lockfile),
         });
     }
     if let Some(vcs_info) = vcs_info {
         result.push(ArchiveFile {
             rel_path: PathBuf::from(VCS_INFO_FILE),
             rel_str: VCS_INFO_FILE.to_string(),
-            contents: FileContents::Generated(vcs_info),
+            contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
         });
     }
     if let Some(license_file) = &pkg.manifest().metadata().license_file {
@@ -530,7 +537,12 @@ fn tar(
                     format!("could not archive source file `{}`", disk_path.display())
                 })?;
             }
-            FileContents::Generated(contents) => {
+            FileContents::Generated(generated_kind) => {
+                let contents = match generated_kind {
+                    GeneratedFile::Manifest => pkg.to_registry_toml(ws)?,
+                    GeneratedFile::Lockfile => build_lock(ws)?,
+                    GeneratedFile::VcsInfo(s) => s,
+                };
                 header.set_entry_type(EntryType::file());
                 header.set_mode(0o644);
                 header.set_mtime(

tests/testsuite/package.rs

@@ -1,10 +1,10 @@
 //! Tests for the `cargo package` command.
 
 use cargo_test_support::paths::CargoPathExt;
-use cargo_test_support::registry::Package;
+use cargo_test_support::publish::validate_crate_contents;
+use cargo_test_support::registry::{self, Package};
 use cargo_test_support::{
-    basic_manifest, cargo_process, git, path2url, paths, project, publish::validate_crate_contents,
-    registry, symlink_supported, t,
+    basic_manifest, cargo_process, git, path2url, paths, project, symlink_supported, t,
 };
 use std::fs::{self, read_to_string, File};
 use std::path::Path;
@@ -1796,3 +1796,52 @@ Caused by:
         )
         .run();
 }
+
+#[cargo_test]
+fn list_with_path_and_lock() {
+    // Allow --list even for something that isn't packageable.
+
+    // Init an empty registry because a versionless path dep will search for
+    // the package on crates.io.
+    registry::init();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            license = "MIT"
+            description = "foo"
+            homepage = "foo"
+
+            [dependencies]
+            bar = {path="bar"}
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
+        .file("bar/src/lib.rs", "")
+        .build();
+
+    p.cargo("package --list")
+        .with_stdout(
+            "\
+Cargo.lock
+Cargo.toml
+Cargo.toml.orig
+src/main.rs
+",
+        )
+        .run();
+
+    p.cargo("package")
+        .with_status(101)
+        .with_stderr(
+            "\
+error: all path dependencies must have a version specified when packaging.
+dependency `bar` does not specify a version.
+",
+        )
+        .run();
+}

tests/testsuite/publish_lockfile.rs

@@ -297,8 +297,8 @@ fn no_warn_workspace_extras() {
         .cwd("a")
         .with_stderr(
             "\
-[UPDATING] `[..]` index
 [PACKAGING] a v0.1.0 ([..])
+[UPDATING] `[..]` index
 ",
         )
         .run();
@@ -328,10 +328,10 @@ fn warn_package_with_yanked() {
     p.cargo("package --no-verify")
         .with_stderr(
             "\
+[PACKAGING] foo v0.0.1 ([..])
 [UPDATING] `[..]` index
 [WARNING] package `bar v0.1.0` in Cargo.lock is yanked in registry \
     `crates.io`, consider updating to a version that is not yanked
-[PACKAGING] foo v0.0.1 ([..])
 ",
         )
         .run();

tests/testsuite/registry.rs

@@ -359,13 +359,18 @@ fn package_with_path_deps() {
         .file("notyet/src/lib.rs", "")
         .build();
 
-    p.cargo("package -v")
+    p.cargo("package")
         .with_status(101)
         .with_stderr_contains(
             "\
-[ERROR] no matching package named `notyet` found
-location searched: registry [..]
-required by package `foo v0.0.1 ([..])`
+[PACKAGING] foo [..]
+[UPDATING] [..]
+[ERROR] failed to prepare local package for uploading
+
+Caused by:
+  no matching package named `notyet` found
+location searched: registry `https://github.com/rust-lang/crates.io-index`
+required by package `foo v0.0.1 [..]`
 ",
         )
         .run();
@@ -375,8 +380,8 @@ required by package `foo v0.0.1 ([..])`
     p.cargo("package")
         .with_stderr(
             "\
-[UPDATING] `[..]` index
 [PACKAGING] foo v0.0.1 ([CWD])
+[UPDATING] `[..]` index
 [VERIFYING] foo v0.0.1 ([CWD])
 [DOWNLOADING] crates ...
 [DOWNLOADED] notyet v0.0.1 (registry `[ROOT][..]`)