From 0d08f19becd32518e47f47d1170094dc9ae3e247 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Mon, 5 Aug 2024 09:47:16 +0700 Subject: [PATCH] Change registry inference rules when packaging multiple packages Infer the package registry only if all packages have the same publish field. If there is confusion, bail out instead of defaulting to crates-io. --- src/cargo/ops/cargo_package.rs | 72 +++++++++++++++++++-------- tests/testsuite/package.rs | 89 ++++++++++++++++++---------------- 2 files changed, 98 insertions(+), 63 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index ab4ceeb4199d..fa9f7687be73 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -185,44 +185,76 @@ fn infer_registry( pkgs: &[&Package], reg_or_index: Option, ) -> CargoResult { - let publish_registry = if let Some(RegistryOrIndex::Registry(registry)) = reg_or_index.as_ref() - { - Some(registry.clone()) - } else if let Some([first_pkg_reg]) = pkgs[0].publish().as_deref() { - // If no registry is specified in the command, but all of the packages - // to publish have the same, unique allowed registry, push to that one. - if pkgs[1..].iter().all(|p| p.publish() == pkgs[0].publish()) { - Some(first_pkg_reg.clone()) - } else { - None + 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", + ); + } + } } - } else { - None }; // 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 { - let reg_name = publish_registry.as_deref().unwrap_or(CRATES_IO_REGISTRY); - for pkg in pkgs { - if let Some(allowed) = pkg.publish().as_ref() { - if !allowed.iter().any(|a| a == reg_name) { - bail!( + 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 { - None => SourceId::crates_io(gctx)?, - Some(RegistryOrIndex::Registry(r)) => SourceId::alt_registry(gctx, &r)?, - Some(RegistryOrIndex::Index(url)) => SourceId::for_registry(&url)?, + 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. diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index df6889b3ef5a..d8de24c5ee24 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -6028,25 +6028,28 @@ fn registry_inferred_from_unique_option() { 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)` +[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(); } #[cargo_test] -fn registry_not_inferred_because_of_missing_option() { +fn registry_not_inferred_because_of_conflict() { let alt_reg = registry::RegistryBuilder::new() .http_api() .http_index() @@ -6100,8 +6103,7 @@ fn registry_not_inferred_because_of_missing_option() { .masquerade_as_nightly_cargo(&["package-workspace"]) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be packaged. -The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. +[ERROR] conflicts between `package.publish` fields in the selected packages "#]]) .run(); @@ -6121,10 +6123,21 @@ The registry `alternative` is not listed in the `package.publish` value in Cargo alt_reg.index_url() )) .masquerade_as_nightly_cargo(&["package-workspace"]) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be packaged. -The registry `crates-io` 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 +[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(); @@ -6143,7 +6156,7 @@ fn registry_not_inferred_because_of_multiple_options() { "Cargo.toml", r#" [workspace] - members = ["dep", "main", "other"] + members = ["dep", "main"] "#, ) .file( @@ -6179,29 +6192,13 @@ fn registry_not_inferred_because_of_multiple_options() { "#, ) .file("dep/src/lib.rs", "") - // No publish field means "publish anywhere" - .file( - "other/Cargo.toml", - r#" - [package] - name = "other" - version = "0.1.0" - edition = "2015" - authors = [] - license = "MIT" - description = "dep" - repository = "bar" - "#, - ) - .file("other/src/lib.rs", "") .build(); p.cargo("package -Zpackage-workspace") .masquerade_as_nightly_cargo(&["package-workspace"]) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be packaged. -The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. +[ERROR] --registry is required to disambiguate between "alternative" or "alternative2" registries "#]]) .run(); @@ -6214,8 +6211,6 @@ The registry `crates-io` is not listed in the `package.publish` value in Cargo.t [PACKAGING] main v0.0.1 ([ROOT]/foo/main) [UPDATING] `alternative` index [PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed) -[PACKAGING] other v0.1.0 ([ROOT]/foo/other) -[PACKAGED] 3 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 @@ -6225,16 +6220,13 @@ The registry `crates-io` is not listed in the `package.publish` value in Cargo.t [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 -[VERIFYING] other v0.1.0 ([ROOT]/foo/other) -[COMPILING] other v0.1.0 ([ROOT]/foo/target/package/other-0.1.0) -[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) .run(); } #[cargo_test] -fn registry_not_inferred_because_of_conflict() { +fn registry_not_inferred_because_of_mismatch() { let _alt_reg = registry::RegistryBuilder::new() .http_api() .http_index() @@ -6267,6 +6259,8 @@ fn registry_not_inferred_because_of_conflict() { "#, ) .file("main/src/main.rs", "fn main() {}") + // No `publish` field means "any registry", but the presence of this package + // will stop us from inferring a registry. .file( "dep/Cargo.toml", r#" @@ -6278,7 +6272,6 @@ fn registry_not_inferred_because_of_conflict() { license = "MIT" description = "dep" repository = "bar" - publish = ["alternative2"] "#, ) .file("dep/src/lib.rs", "") @@ -6288,18 +6281,28 @@ fn registry_not_inferred_because_of_conflict() { .masquerade_as_nightly_cargo(&["package-workspace"]) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` cannot be packaged. -The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. +[ERROR] --registry is required because not all `package.publish` settings agree "#]]) .run(); p.cargo("package -Zpackage-workspace --registry=alternative") .masquerade_as_nightly_cargo(&["package-workspace"]) - .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `dep` 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 +[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();