Skip to content

Commit

Permalink
Change registry inference rules when packaging multiple packages
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jneem committed Aug 7, 2024
1 parent 010db83 commit 275d3b6
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 36 deletions.
72 changes: 52 additions & 20 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,44 +185,76 @@ fn infer_registry(
pkgs: &[&Package],
reg_or_index: Option<RegistryOrIndex>,
) -> CargoResult<SourceId> {
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::<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 {
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) = &reg_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, &reg)?,
};

// Load source replacements that are built-in to Cargo.
Expand Down
43 changes: 27 additions & 16 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 275d3b6

Please sign in to comment.