From 010db83cf5396a4cb56774a64856f621e5f443a9 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Thu, 1 Aug 2024 18:03:54 +0700 Subject: [PATCH 1/2] Add misbehaving tests. --- tests/testsuite/package.rs | 353 +++++++++++++++++++++++++++++++++++++ 1 file changed, 353 insertions(+) diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 92a91d41732..f00eedfac63 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -5943,3 +5943,356 @@ fn workspace_with_local_and_remote_deps() { ) .run(); } + +#[cargo_test] +fn registry_not_in_publish_list() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + edition = "2015" + authors = [] + license = "MIT" + description = "foo" + publish = [ + "test" + ] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("package --registry alternative -Zpackage-workspace") + .masquerade_as_nightly_cargo(&["package-workspace"]) + .with_status(101) + .with_stderr_data(str![[r#" +[ERROR] registry index was not found in any configuration: `alternative` + +"#]]) + .run(); +} + +#[cargo_test] +fn registry_inferred_from_unique_option() { + let _registry = 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 = ["alternative"] + + [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_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(); +} + +#[cargo_test] +fn registry_not_inferred_because_of_conflict() { + 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 = ["alternative"] + + [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 = ["alternative2"] + "#, + ) + .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#" +[ERROR] `dep` cannot be packaged. +The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. + +"#]]) + .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. + +"#]]) + .run(); + + p.cargo(&format!( + "package --index {} -Zpackage-workspace", + 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. + +"#]]) + .run(); +} + +#[cargo_test] +fn registry_not_inferred_because_of_multiple_options() { + 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 = ["alternative", "alternative2"] + + [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", "alternative2"] + "#, + ) + .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#" +[ERROR] `dep` cannot be packaged. +The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. + +"#]]) + .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 +[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_mismatch() { + 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 = ["alternative"] + + [dependencies] + dep = { path = "../dep", version = "0.1.0", registry = "alternative" } + "#, + ) + .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#" + [package] + name = "dep" + version = "0.1.0" + edition = "2015" + authors = [] + license = "MIT" + description = "dep" + repository = "bar" + "#, + ) + .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#" +[ERROR] `main` cannot be packaged. +The registry `crates-io` is not listed in the `package.publish` value in Cargo.toml. + +"#]]) + .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 +[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(); +} From 275d3b6e5489ee24be1f2d083f772829bae16a15 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Wed, 7 Aug 2024 21:55:11 +0700 Subject: [PATCH 2/2] 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 | 43 ++++++++++++-------- 2 files changed, 79 insertions(+), 36 deletions(-) diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index ab4ceeb4199..fa9f7687be7 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 f00eedfac63..d8de24c5ee2 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -6028,18 +6028,21 @@ 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(); @@ -6100,8 +6103,7 @@ 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] 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(); @@ -6185,8 +6198,7 @@ fn registry_not_inferred_because_of_multiple_options() { .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(); @@ -6269,8 +6281,7 @@ fn registry_not_inferred_because_of_mismatch() { .masquerade_as_nightly_cargo(&["package-workspace"]) .with_status(101) .with_stderr_data(str![[r#" -[ERROR] `main` 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();