Allow unpublishable crates to be packaged

jneem · jneem · commit 591993fdb0c7 · 2024-08-19T12:07:40.000+07:00
diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
@@ -13,7 +13,7 @@ use crate::core::resolver::CliFeatures;
 use crate::core::resolver::HasDevUnits;
 use crate::core::{Feature, PackageIdSpecQuery, Shell, Verbosity, Workspace};
 use crate::core::{Package, PackageId, PackageSet, Resolve, SourceId};
-use crate::ops::registry::{infer_registry, RegistryOrIndex};
+use crate::ops::registry::infer_registry;
 use crate::sources::registry::index::{IndexPackage, RegistryDependency};
 use crate::sources::{PathSource, CRATES_IO_REGISTRY};
 use crate::util::cache_lock::CacheLockMode;
@@ -33,6 +33,8 @@ use tar::{Archive, Builder, EntryType, Header, HeaderMode};
 use tracing::debug;
 use unicase::Ascii as UncasedAscii;
 
+use super::RegistryOrIndex;
+
 #[derive(Clone)]
 pub struct PackageOpts<'gctx> {
     pub gctx: &'gctx GlobalContext,
@@ -173,6 +175,46 @@ fn create_package(
     return Ok(dst);
 }
 
+/// Determine which registry the packages are for.
+///
+/// The registry only affects the built packages if there are dependencies within the
+/// packages that we're packaging: if we're packaging foo-bin and foo-lib, and foo-bin
+/// depends on foo-lib, then the foo-lib entry in foo-bin's lockfile will depend on the
+/// registry that we're building packages for.
+fn get_registry(
+    gctx: &GlobalContext,
+    pkgs: &[&Package],
+    reg_or_index: Option<RegistryOrIndex>,
+) -> CargoResult<SourceId> {
+    let reg_or_index = match reg_or_index.clone() {
+        Some(r) => Some(r),
+        None => infer_registry(pkgs)?,
+    };
+
+    // Validate the registry against the packages' allow-lists.
+    let reg = reg_or_index
+        .clone()
+        .unwrap_or_else(|| RegistryOrIndex::Registry(CRATES_IO_REGISTRY.to_owned()));
+    if let RegistryOrIndex::Registry(reg_name) = reg {
+        for pkg in pkgs {
+            if let Some(allowed) = pkg.publish().as_ref() {
+                // If allowed is empty (i.e. package.publish is false), we let it slide.
+                // This allows packaging unpublishable packages (although packaging might
+                // fail later if the unpublishable package is a dependency of something else).
+                if !allowed.is_empty() && !allowed.iter().any(|a| a == &reg_name) {
+                    bail!(
+                        "`{}` cannot be packaged.\n\
+                         The registry `{}` is not listed in the `package.publish` value in Cargo.toml.",
+                        pkg.name(),
+                        reg_name
+                    );
+                }
+            }
+        }
+    }
+    Ok(ops::registry::get_source_id(gctx, reg_or_index.as_ref())?.replacement)
+}
+
 /// Packages an entire workspace.
 ///
 /// Returns the generated package files. If `opts.list` is true, skips
@@ -243,7 +285,9 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
         } else {
             let tarball = create_package(ws, &pkg, ar_files, local_reg.as_ref())?;
             if let Some(local_reg) = local_reg.as_mut() {
-                local_reg.add_package(ws, &pkg, &tarball)?;
+                if pkg.publish() != &Some(Vec::new()) {
+                    local_reg.add_package(ws, &pkg, &tarball)?;
+                }
             }
             outputs.push((pkg, opts, tarball));
         }
@@ -261,43 +305,6 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
     Ok(outputs.into_iter().map(|x| x.2).collect())
 }
 
-/// Determine which registry the packages are for.
-///
-/// The registry only affects the built packages if there are dependencies within the
-/// packages that we're packaging: if we're packaging foo-bin and foo-lib, and foo-bin
-/// depends on foo-lib, then the foo-lib entry in foo-bin's lockfile will depend on the
-/// registry that we're building packages for.
-fn get_registry(
-    gctx: &GlobalContext,
-    pkgs: &[&Package],
-    reg_or_index: Option<RegistryOrIndex>,
-) -> CargoResult<SourceId> {
-    let reg_or_index = match reg_or_index.clone() {
-        Some(r) => Some(r),
-        None => infer_registry(pkgs)?,
-    };
-
-    // Validate the registry against the packages' allow-lists.
-    let reg = reg_or_index
-        .clone()
-        .unwrap_or_else(|| RegistryOrIndex::Registry(CRATES_IO_REGISTRY.to_owned()));
-    if let RegistryOrIndex::Registry(reg_name) = reg {
-        for pkg in pkgs {
-            if let Some(allowed) = pkg.publish().as_ref() {
-                if !allowed.iter().any(|a| a == &reg_name) {
-                    bail!(
-                        "`{}` cannot be packaged.\n\
-                         The registry `{}` is not listed in the `package.publish` value in Cargo.toml.",
-                        pkg.name(),
-                        reg_name
-                    );
-                }
-            }
-        }
-    }
-    Ok(ops::registry::get_source_id(gctx, reg_or_index.as_ref())?.replacement)
-}
-
 /// Just the part of the dependency graph that's between the packages we're packaging.
 /// (Is the package name a good key? Does it uniquely identify packages?)
 #[derive(Clone, Debug, Default)]
diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs
@@ -325,39 +325,49 @@ pub(crate) struct RegistrySourceIds {
 
 /// If this set of packages has an unambiguous publish registry, find it.
 pub(crate) fn infer_registry(pkgs: &[&Package]) -> CargoResult<Option<RegistryOrIndex>> {
-    if pkgs[1..].iter().all(|p| p.publish() == pkgs[0].publish()) {
+    // Ignore "publish = false" packages while inferring the registry.
+    let publishable_pkgs: Vec<_> = pkgs
+        .iter()
+        .filter(|p| p.publish() != &Some(Vec::new()))
+        .collect();
+
+    if let Some((first, rest)) = publishable_pkgs.split_first() {
         // If all packages have the same publish settings, we take that as the default.
-        match pkgs[0].publish().as_deref() {
-            Some([unique_pkg_reg]) => {
-                Ok(Some(RegistryOrIndex::Registry(unique_pkg_reg.to_owned())))
+        if rest.iter().all(|p| p.publish() == first.publish()) {
+            match publishable_pkgs[0].publish().as_deref() {
+                Some([unique_pkg_reg]) => {
+                    Ok(Some(RegistryOrIndex::Registry(unique_pkg_reg.to_owned())))
+                }
+                None | Some([]) => Ok(None),
+                Some(regs) => {
+                    let mut regs: Vec<_> = regs.iter().map(|s| format!("\"{}\"", s)).collect();
+                    regs.sort();
+                    regs.dedup();
+                    // unwrap: the match block ensures that there's more than one reg.
+                    let (last_reg, regs) = regs.split_last().unwrap();
+                    bail!(
+                        "--registry is required to disambiguate between {} or {} registries",
+                        regs.join(", "),
+                        last_reg
+                    )
+                }
             }
-            None | Some([]) => Ok(None),
-            Some(regs) => {
-                let mut regs: Vec<_> = regs.iter().map(|s| format!("\"{}\"", s)).collect();
-                regs.sort();
-                regs.dedup();
-                // unwrap: the match block ensures that there's more than one reg.
-                let (last_reg, regs) = regs.split_last().unwrap();
-                bail!(
-                    "--registry is required to disambiguate between {} or {} registries",
-                    regs.join(", "),
-                    last_reg
-                )
+        } else {
+            let common_regs = publishable_pkgs
+                .iter()
+                // `None` means "all registries", so drop them instead of including them
+                // in the intersection.
+                .filter_map(|p| p.publish().as_deref())
+                .map(|p| p.iter().collect::<HashSet<_>>())
+                .reduce(|xs, ys| xs.intersection(&ys).cloned().collect())
+                .unwrap_or_default();
+            if common_regs.is_empty() {
+                bail!("conflicts between `package.publish` fields in the selected packages");
+            } else {
+                bail!("--registry is required because not all `package.publish` settings agree",);
             }
         }
     } else {
-        let common_regs = pkgs
-            .iter()
-            // `None` means "all registries", so drop them instead of including them
-            // in the intersection.
-            .filter_map(|p| p.publish().as_deref())
-            .map(|p| p.iter().collect::<HashSet<_>>())
-            .reduce(|xs, ys| xs.intersection(&ys).cloned().collect())
-            .unwrap_or_default();
-        if common_regs.is_empty() {
-            bail!("conflicts between `package.publish` fields in the selected packages");
-        } else {
-            bail!("--registry is required because not all `package.publish` settings agree",);
-        }
+        Ok(None)
     }
 }
diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs
@@ -6227,19 +6227,41 @@ fn registry_inference_ignores_unpublishable() {
 
     p.cargo("package -Zpackage-workspace")
         .masquerade_as_nightly_cargo(&["package-workspace"])
-        .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] conflicts between `package.publish` fields in the selected packages
+[PACKAGING] dep v0.1.0 ([ROOT]/foo/dep)
+[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
+[PACKAGING] main v0.0.1 ([ROOT]/foo/main)
+[UPDATING] `alternative` index
+[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
+[VERIFYING] dep v0.1.0 ([ROOT]/foo/dep)
+[COMPILING] dep v0.1.0 ([ROOT]/foo/target/package/dep-0.1.0)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+[VERIFYING] main v0.0.1 ([ROOT]/foo/main)
+[UPDATING] `alternative` index
+[UNPACKING] dep v0.1.0 (registry `[ROOT]/foo/target/package/tmp-registry`)
+[COMPILING] dep v0.1.0 (registry `alternative`)
+[COMPILING] main v0.0.1 ([ROOT]/foo/target/package/main-0.0.1)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 
 "#]])
         .run();
 
     p.cargo("package -Zpackage-workspace --registry=alternative")
         .masquerade_as_nightly_cargo(&["package-workspace"])
-        .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] `main` cannot be packaged.
-The registry `alternative` is not listed in the `package.publish` value in Cargo.toml.
+[PACKAGING] dep v0.1.0 ([ROOT]/foo/dep)
+[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
+[PACKAGING] main v0.0.1 ([ROOT]/foo/main)
+[UPDATING] `alternative` index
+[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
+[VERIFYING] dep v0.1.0 ([ROOT]/foo/dep)
+[COMPILING] dep v0.1.0 ([ROOT]/foo/target/package/dep-0.1.0)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+[VERIFYING] main v0.0.1 ([ROOT]/foo/main)
+[UPDATING] `alternative` index
+[COMPILING] dep v0.1.0 (registry `alternative`)
+[COMPILING] main v0.0.1 ([ROOT]/foo/target/package/main-0.0.1)
+[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
 
 "#]])
         .run();
@@ -6464,7 +6486,16 @@ fn unpublishable_dependency() {
         .masquerade_as_nightly_cargo(&["package-workspace"])
         .with_status(101)
         .with_stderr_data(str![[r#"
-[ERROR] conflicts between `package.publish` fields in the selected packages
+[PACKAGING] dep v0.1.0 ([ROOT]/foo/dep)
+[PACKAGED] 3 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
+[PACKAGING] main v0.0.1 ([ROOT]/foo/main)
+[UPDATING] `alternative` index
+[ERROR] failed to prepare local package for uploading
+
+Caused by:
+  no matching package named `dep` found
+  location searched: registry `alternative`
+  required by package `main v0.0.1 ([ROOT]/foo/main)`
 
 "#]])
         .run();
