diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 17ef0ecbba3..aa5aca3c9e3 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap}; use std::fs::{self, File}; use std::io::prelude::*; use std::io::SeekFrom; @@ -6,7 +6,6 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::task::Poll; -use super::RegistryOrIndex; use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor}; use crate::core::dependency::DepKind; use crate::core::manifest::Target; @@ -15,8 +14,9 @@ use crate::core::resolver::HasDevUnits; use crate::core::{Feature, PackageIdSpecQuery, Shell, Verbosity, Workspace}; use crate::core::{Package, PackageId, PackageSet, Resolve, SourceId}; use crate::ops::lockfile::LOCKFILE_NAME; +use crate::ops::registry::{infer_registry, RegistryOrIndex}; use crate::sources::registry::index::{IndexPackage, RegistryDependency}; -use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_REGISTRY}; +use crate::sources::{PathSource, CRATES_IO_REGISTRY}; use crate::util::cache_lock::CacheLockMode; use crate::util::context::JobsConfig; use crate::util::errors::CargoResult; @@ -174,97 +174,6 @@ 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 infer_registry( - gctx: &GlobalContext, - pkgs: &[&Package], - reg_or_index: Option, -) -> CargoResult { - let reg_or_index = match reg_or_index { - Some(r) => r, - None => { - if pkgs[1..].iter().all(|p| p.publish() == pkgs[0].publish()) { - // If all packages have the same publish settings, we take that as the default. - match pkgs[0].publish().as_deref() { - Some([unique_pkg_reg]) => RegistryOrIndex::Registry(unique_pkg_reg.to_owned()), - None | Some([]) => RegistryOrIndex::Registry(CRATES_IO_REGISTRY.to_owned()), - Some([reg, ..]) if pkgs.len() == 1 => { - // For backwards compatibility, avoid erroring if there's only one package. - // The registry doesn't affect packaging in this case. - RegistryOrIndex::Registry(reg.to_owned()) - } - 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 = 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::>()) - .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", - ); - } - } - } - }; - - // Validate the registry against the packages' allow-lists. For backwards compatibility, we - // skip this if only a single package is being published (because in that case the registry - // doesn't affect the packaging step). - if pkgs.len() > 1 { - if let RegistryOrIndex::Registry(reg_name) = ®_or_index { - 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 - ); - } - } - } - } - } - - let sid = match reg_or_index { - RegistryOrIndex::Index(url) => SourceId::for_registry(&url)?, - RegistryOrIndex::Registry(reg) if reg == CRATES_IO_REGISTRY => SourceId::crates_io(gctx)?, - RegistryOrIndex::Registry(reg) => SourceId::alt_registry(gctx, ®)?, - }; - - // Load source replacements that are built-in to Cargo. - let sid = SourceConfigMap::empty(gctx)? - .load(sid, &HashSet::new())? - .replaced_source_id(); - - Ok(sid) -} - /// Packages an entire workspace. /// /// Returns the generated package files. If `opts.list` is true, skips @@ -293,19 +202,28 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult = pkgs.iter().map(|p| p.0).collect(); - let publish_reg = infer_registry(ws.gctx(), &just_pkgs, opts.reg_or_index.clone())?; - debug!("packaging for registry {publish_reg}"); + + // The publish registry doesn't matter unless there are local dependencies, + // so only try to get one if we need it. If they explicitly passed a + // registry on the CLI, we check it no matter what. + let sid = if deps.has_no_dependencies() && opts.reg_or_index.is_none() { + None + } else { + let sid = get_registry(ws.gctx(), &just_pkgs, opts.reg_or_index.clone())?; + debug!("packaging for registry {}", sid); + Some(sid) + }; let mut local_reg = if ws.gctx().cli_unstable().package_workspace { let reg_dir = ws.target_dir().join("package").join("tmp-registry"); - Some(TmpRegistry::new(ws.gctx(), reg_dir, publish_reg)?) + sid.map(|sid| TmpRegistry::new(ws.gctx(), reg_dir, sid)) + .transpose()? } else { None }; - let deps = local_deps(pkgs.iter().map(|(p, f)| ((*p).clone(), f.clone()))); - // Packages need to be created in dependency order, because dependencies must // be added to our local overlay before we can create lockfiles that depend on them. let sorted_pkgs = deps.sort(); @@ -325,7 +243,9 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult, opts: &PackageOpts<'_>) -> CargoResult, +) -> CargoResult { + 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 == ®_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)] @@ -359,6 +319,12 @@ impl LocalDependencies { .map(|name| self.packages[&name].clone()) .collect() } + + pub fn has_no_dependencies(&self) -> bool { + self.graph + .iter() + .all(|node| self.graph.edges(node).next().is_none()) + } } /// Build just the part of the dependency graph that's between the given packages, diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs index 016d4cebb77..93b48b6f6f6 100644 --- a/src/cargo/ops/registry/mod.rs +++ b/src/cargo/ops/registry/mod.rs @@ -19,7 +19,7 @@ use cargo_credential::{Operation, Secret}; use crates_io::Registry; use url::Url; -use crate::core::{PackageId, SourceId}; +use crate::core::{Package, PackageId, SourceId}; use crate::sources::source::Source; use crate::sources::{RegistrySource, SourceConfigMap}; use crate::util::auth; @@ -191,7 +191,7 @@ fn registry( /// /// The return value is a pair of `SourceId`s: The first may be a built-in replacement of /// crates.io (such as index.crates.io), while the second is always the original source. -fn get_source_id( +pub(crate) fn get_source_id( gctx: &GlobalContext, reg_or_index: Option<&RegistryOrIndex>, ) -> CargoResult { @@ -322,3 +322,52 @@ pub(crate) struct RegistrySourceIds { /// User-defined source replacement is not applied. pub(crate) replacement: SourceId, } + +/// If this set of packages has an unambiguous publish registry, find it. +pub(crate) fn infer_registry(pkgs: &[&Package]) -> CargoResult> { + // Ignore "publish = false" packages while inferring the registry. + let publishable_pkgs: Vec<_> = pkgs + .iter() + .filter(|p| p.publish() != &Some(Vec::new())) + .collect(); + + let Some((first, rest)) = publishable_pkgs.split_first() else { + return Ok(None); + }; + + // If all packages have the same publish settings, we take that as the default. + 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 + ) + } + } + } 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::>()) + .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",); + } + } +} diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 79954062381..7b937ca53b9 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -5999,7 +5999,8 @@ fn registry_not_in_publish_list() { .masquerade_as_nightly_cargo(&["package-workspace"]) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] registry index was not found in any configuration: `alternative` +[ERROR] `foo` cannot be packaged. +The registry `alternative` is not listed in the `package.publish` value in Cargo.toml. "#]]) .run(); @@ -6173,6 +6174,99 @@ The registry `alternative` is not listed in the `package.publish` value in Cargo .run(); } +#[cargo_test] +fn registry_inference_ignores_unpublishable() { + let _alt_reg = registry::RegistryBuilder::new() + .http_api() + .http_index() + .alternative() + .build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["dep", "main"] + "#, + ) + .file( + "main/Cargo.toml", + r#" + [package] + name = "main" + version = "0.0.1" + edition = "2015" + authors = [] + license = "MIT" + description = "main" + repository = "bar" + publish = false + + [dependencies] + dep = { path = "../dep", version = "0.1.0", registry = "alternative" } + "#, + ) + .file("main/src/main.rs", "fn main() {}") + .file( + "dep/Cargo.toml", + r#" + [package] + name = "dep" + version = "0.1.0" + edition = "2015" + authors = [] + license = "MIT" + description = "dep" + repository = "bar" + publish = ["alternative"] + "#, + ) + .file("dep/src/lib.rs", "") + .build(); + + p.cargo("package -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .with_stderr_data(str![[r#" +[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_stderr_data(str![[r#" +[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(); +} + #[cargo_test] fn registry_not_inferred_because_of_multiple_options() { let _alt_reg = registry::RegistryBuilder::new() @@ -6337,3 +6431,72 @@ fn registry_not_inferred_because_of_mismatch() { "#]]) .run(); } + +#[cargo_test] +fn unpublishable_dependency() { + let _alt_reg = registry::RegistryBuilder::new() + .http_api() + .http_index() + .alternative() + .build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["dep", "main"] + "#, + ) + .file( + "main/Cargo.toml", + r#" + [package] + name = "main" + version = "0.0.1" + edition = "2015" + authors = [] + license = "MIT" + description = "main" + repository = "bar" + + [dependencies] + dep = { path = "../dep", version = "0.1.0", registry = "alternative" } + "#, + ) + .file("main/src/main.rs", "fn main() {}") + .file( + "dep/Cargo.toml", + r#" + [package] + name = "dep" + version = "0.1.0" + edition = "2015" + authors = [] + license = "MIT" + description = "dep" + repository = "bar" + publish = false + "#, + ) + .file("dep/src/lib.rs", "") + .build(); + + p.cargo("package -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .with_status(101) + .with_stderr_data(str![[r#" +[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(); +}