From 1f172e4b76d7f900cc6d81235bfea3ba73fbe64c Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 15 Sep 2023 18:30:28 +0800 Subject: [PATCH 1/4] refactor: extract `registry` and `index` arg for reuse --- src/bin/cargo/commands/init.rs | 2 +- src/bin/cargo/commands/login.rs | 2 +- src/bin/cargo/commands/logout.rs | 2 +- src/bin/cargo/commands/new.rs | 2 +- src/bin/cargo/commands/owner.rs | 4 ++-- src/bin/cargo/commands/publish.rs | 4 ++-- src/bin/cargo/commands/search.rs | 4 ++-- src/bin/cargo/commands/yank.rs | 4 ++-- src/cargo/util/command_prelude.rs | 8 ++++++-- tests/testsuite/cargo_owner/help/stdout.log | 4 ++-- tests/testsuite/cargo_publish/help/stdout.log | 2 +- tests/testsuite/cargo_search/help/stdout.log | 4 ++-- tests/testsuite/cargo_yank/help/stdout.log | 4 ++-- 13 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/bin/cargo/commands/init.rs b/src/bin/cargo/commands/init.rs index d699c2c4907..48409d82716 100644 --- a/src/bin/cargo/commands/init.rs +++ b/src/bin/cargo/commands/init.rs @@ -7,7 +7,7 @@ pub fn cli() -> Command { .about("Create a new cargo package in an existing directory") .arg(Arg::new("path").action(ArgAction::Set).default_value(".")) .arg_new_opts() - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_registry("Registry to use") .arg_quiet() .after_help(color_print::cstr!( "Run `cargo help init` for more detailed information.\n" diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index 262e3d6b05f..89ebe855f31 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -6,7 +6,7 @@ pub fn cli() -> Command { subcommand("login") .about("Log in to a registry.") .arg(Arg::new("token").action(ArgAction::Set)) - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_registry("Registry to use") .arg( Arg::new("args") .help("Arguments for the credential provider (unstable)") diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs index a4fae5bd614..40f8d83c73f 100644 --- a/src/bin/cargo/commands/logout.rs +++ b/src/bin/cargo/commands/logout.rs @@ -4,7 +4,7 @@ use cargo::ops; pub fn cli() -> Command { subcommand("logout") .about("Remove an API token from the registry locally") - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_registry("Registry to use") .arg_quiet() .after_help(color_print::cstr!( "Run `cargo help logout` for more detailed information.\n" diff --git a/src/bin/cargo/commands/new.rs b/src/bin/cargo/commands/new.rs index a5dff272108..c8d19218f99 100644 --- a/src/bin/cargo/commands/new.rs +++ b/src/bin/cargo/commands/new.rs @@ -7,7 +7,7 @@ pub fn cli() -> Command { .about("Create a new cargo package at ") .arg(Arg::new("path").action(ArgAction::Set).required(true)) .arg_new_opts() - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_registry("Registry to use") .arg_quiet() .after_help(color_print::cstr!( "Run `cargo help new` for more detailed information.\n" diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs index c9deaef5137..2c3ab0da06e 100644 --- a/src/bin/cargo/commands/owner.rs +++ b/src/bin/cargo/commands/owner.rs @@ -24,9 +24,9 @@ pub fn cli() -> Command { .short('r'), ) .arg(flag("list", "List owners of a crate").short('l')) - .arg(opt("index", "Registry index to modify owners for").value_name("INDEX")) + .arg_index("Registry index URL to modify owners for") + .arg_registry("Registry to modify owners for") .arg(opt("token", "API token to use when authenticating").value_name("TOKEN")) - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) .arg_quiet() .after_help(color_print::cstr!( "Run `cargo help owner` for more detailed information.\n" diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs index 34108d45d25..f8377b82343 100644 --- a/src/bin/cargo/commands/publish.rs +++ b/src/bin/cargo/commands/publish.rs @@ -6,8 +6,8 @@ pub fn cli() -> Command { subcommand("publish") .about("Upload a package to the registry") .arg_dry_run("Perform all checks without uploading") - .arg_index() - .arg(opt("registry", "Registry to publish to").value_name("REGISTRY")) + .arg_index("Registry index URL to upload the package to") + .arg_registry("Registry to upload the package to") .arg(opt("token", "Token to use when uploading").value_name("TOKEN")) .arg(flag( "no-verify", diff --git a/src/bin/cargo/commands/search.rs b/src/bin/cargo/commands/search.rs index 90aadcb274b..199c2bdbaab 100644 --- a/src/bin/cargo/commands/search.rs +++ b/src/bin/cargo/commands/search.rs @@ -15,8 +15,8 @@ pub fn cli() -> Command { ) .value_name("LIMIT"), ) - .arg_index() - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_index("Registry index URL to search packages in") + .arg_registry("Registry to search packages in") .arg_quiet() .after_help(color_print::cstr!( "Run `cargo help search` for more detailed information.\n" diff --git a/src/bin/cargo/commands/yank.rs b/src/bin/cargo/commands/yank.rs index 665683915dd..9097cb31fe9 100644 --- a/src/bin/cargo/commands/yank.rs +++ b/src/bin/cargo/commands/yank.rs @@ -16,8 +16,8 @@ pub fn cli() -> Command { "undo", "Undo a yank, putting a version back into the index", )) - .arg(opt("index", "Registry index to yank from").value_name("INDEX")) - .arg(opt("registry", "Registry to use").value_name("REGISTRY")) + .arg_index("Registry index URL to yank from") + .arg_registry("Registry to yank from") .arg(opt("token", "API token to use when authenticating").value_name("TOKEN")) .arg_quiet() .after_help(color_print::cstr!( diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index f55e44f2c68..f05192f023d 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -286,8 +286,12 @@ pub trait CommandExt: Sized { ) } - fn arg_index(self) -> Self { - self._arg(opt("index", "Registry index URL to upload the package to").value_name("INDEX")) + fn arg_registry(self, help: &'static str) -> Self { + self._arg(opt("registry", help).value_name("REGISTRY")) + } + + fn arg_index(self, help: &'static str) -> Self { + self._arg(opt("index", help).value_name("INDEX")) } fn arg_dry_run(self, dry_run: &'static str) -> Self { diff --git a/tests/testsuite/cargo_owner/help/stdout.log b/tests/testsuite/cargo_owner/help/stdout.log index 3c8495ff02a..580be3c88f9 100644 --- a/tests/testsuite/cargo_owner/help/stdout.log +++ b/tests/testsuite/cargo_owner/help/stdout.log @@ -9,9 +9,9 @@ Options: -a, --add Name of a user or team to invite as an owner -r, --remove Name of a user or team to remove as an owner -l, --list List owners of a crate - --index Registry index to modify owners for + --index Registry index URL to modify owners for + --registry Registry to modify owners for --token API token to use when authenticating - --registry Registry to use -q, --quiet Do not print cargo log messages -v, --verbose... Use verbose output (-vv very verbose/build.rs output) --color Coloring: auto, always, never diff --git a/tests/testsuite/cargo_publish/help/stdout.log b/tests/testsuite/cargo_publish/help/stdout.log index 313b937f91a..db19a168d9b 100644 --- a/tests/testsuite/cargo_publish/help/stdout.log +++ b/tests/testsuite/cargo_publish/help/stdout.log @@ -5,7 +5,7 @@ Usage: cargo[EXE] publish [OPTIONS] Options: --dry-run Perform all checks without uploading --index Registry index URL to upload the package to - --registry Registry to publish to + --registry Registry to upload the package to --token Token to use when uploading --no-verify Don't verify the contents by building them --allow-dirty Allow dirty working directories to be packaged diff --git a/tests/testsuite/cargo_search/help/stdout.log b/tests/testsuite/cargo_search/help/stdout.log index 8572064e357..07170ad7079 100644 --- a/tests/testsuite/cargo_search/help/stdout.log +++ b/tests/testsuite/cargo_search/help/stdout.log @@ -7,8 +7,8 @@ Arguments: Options: --limit Limit the number of results (default: 10, max: 100) - --index Registry index URL to upload the package to - --registry Registry to use + --index Registry index URL to search packages in + --registry Registry to search packages in -q, --quiet Do not print cargo log messages -v, --verbose... Use verbose output (-vv very verbose/build.rs output) --color Coloring: auto, always, never diff --git a/tests/testsuite/cargo_yank/help/stdout.log b/tests/testsuite/cargo_yank/help/stdout.log index 25b04e6c7d0..c6dbfeb9d47 100644 --- a/tests/testsuite/cargo_yank/help/stdout.log +++ b/tests/testsuite/cargo_yank/help/stdout.log @@ -8,8 +8,8 @@ Arguments: Options: --version The version to yank or un-yank --undo Undo a yank, putting a version back into the index - --index Registry index to yank from - --registry Registry to use + --index Registry index URL to yank from + --registry Registry to yank from --token API token to use when authenticating -q, --quiet Do not print cargo log messages -v, --verbose... Use verbose output (-vv very verbose/build.rs output) From e57ce13da8d2fa9d6a84b149365e745a8b22d7a0 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 15 Sep 2023 22:11:10 +0800 Subject: [PATCH 2/4] refactor: `--registry` always conflicts with `--index` Move the validation upfront to clap --- src/cargo/util/command_prelude.rs | 7 ++++++- tests/testsuite/alt_registry.rs | 7 +++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index f05192f023d..6ccbbf231f6 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -291,7 +291,12 @@ pub trait CommandExt: Sized { } fn arg_index(self, help: &'static str) -> Self { - self._arg(opt("index", help).value_name("INDEX")) + // Always conflicts with `--registry`. + self._arg( + opt("index", help) + .value_name("INDEX") + .conflicts_with("registry"), + ) } fn arg_dry_run(self, dry_run: &'static str) -> Self { diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index 91157cd5327..d6d7dd531cc 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -1389,10 +1389,9 @@ fn both_index_and_registry() { p.cargo(cmd) .arg("--registry=foo") .arg("--index=foo") - .with_status(101) - .with_stderr( - "[ERROR] both `--index` and `--registry` \ - should not be set at the same time", + .with_status(1) + .with_stderr_contains( + "error: the argument '--registry ' cannot be used with '--index '", ) .run(); } From e883054c59bd4baf0c381528e7c2cb4c661e185b Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 16 Sep 2023 00:47:59 +0800 Subject: [PATCH 3/4] test: verify publish failed with `--index` and the only implicit allowed registry The message is misleading and shouldn't proceed when `--index` presents. --- tests/testsuite/publish.rs | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 91e212fe259..9842d526fd8 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -921,6 +921,48 @@ You may press ctrl-c [..] ); } +#[cargo_test] +fn publish_failed_with_index_and_only_allowed_registry() { + let registry = RegistryBuilder::new() + .http_api() + .http_index() + .alternative() + .build(); + + let p = project().build(); + + let _ = repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + license = "MIT" + description = "foo" + documentation = "foo" + homepage = "foo" + repository = "foo" + publish = ["alternative"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish") + .arg("--index") + .arg(registry.index_url().as_str()) + .with_status(101) + .with_stderr( + "\ +[NOTE] Found `alternative` as only allowed registry. Publishing to it automatically. +[ERROR] command-line argument --index requires --token to be specified +", + ) + .run(); +} + #[cargo_test] fn publish_fail_with_no_registry_specified() { let p = project().build(); From 11754e9923110ab16b5dcf26f57fc5dbf86d28fc Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sat, 16 Sep 2023 01:10:16 +0800 Subject: [PATCH 4/4] refactor: use `RegistryOrIndex` enum to replace two booleans --- src/bin/cargo/commands/install.rs | 9 ++++---- src/bin/cargo/commands/login.rs | 14 ++++++++---- src/bin/cargo/commands/logout.rs | 13 ++++++++--- src/bin/cargo/commands/owner.rs | 4 +--- src/bin/cargo/commands/publish.rs | 6 ++---- src/bin/cargo/commands/search.rs | 5 ++--- src/bin/cargo/commands/yank.rs | 5 +---- src/cargo/ops/mod.rs | 1 + src/cargo/ops/registry/login.rs | 15 +++++++++---- src/cargo/ops/registry/logout.rs | 5 +++-- src/cargo/ops/registry/mod.rs | 36 ++++++++++++++++++++----------- src/cargo/ops/registry/owner.rs | 8 +++---- src/cargo/ops/registry/publish.rs | 18 +++++++++++----- src/cargo/ops/registry/search.rs | 7 +++--- src/cargo/ops/registry/yank.rs | 8 +++---- src/cargo/util/command_prelude.rs | 27 +++++++++++++---------- 16 files changed, 111 insertions(+), 70 deletions(-) diff --git a/src/bin/cargo/commands/install.rs b/src/bin/cargo/commands/install.rs index e7907f449f5..010a4f48385 100644 --- a/src/bin/cargo/commands/install.rs +++ b/src/bin/cargo/commands/install.rs @@ -154,10 +154,11 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { } else if krates.is_empty() { from_cwd = true; SourceId::for_path(config.cwd())? - } else if let Some(index) = args.get_one::("index") { - SourceId::for_registry(&index.into_url()?)? - } else if let Some(registry) = args.registry(config)? { - SourceId::alt_registry(config, ®istry)? + } else if let Some(reg_or_index) = args.registry_or_index(config)? { + match reg_or_index { + ops::RegistryOrIndex::Registry(r) => SourceId::alt_registry(config, &r)?, + ops::RegistryOrIndex::Index(url) => SourceId::for_registry(&url)?, + } } else { SourceId::crates_io(config)? }; diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index 89ebe855f31..d48bf14dc85 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -1,6 +1,7 @@ -use crate::command_prelude::*; - use cargo::ops; +use cargo::ops::RegistryOrIndex; + +use crate::command_prelude::*; pub fn cli() -> Command { subcommand("login") @@ -20,7 +21,12 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; + let reg = args.registry_or_index(config)?; + assert!( + !matches!(reg, Some(RegistryOrIndex::Index(..))), + "must not be index URL" + ); + let extra_args = args .get_many::("args") .unwrap_or_default() @@ -29,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { ops::registry_login( config, args.get_one::("token").map(|s| s.as_str().into()), - registry.as_deref(), + reg.as_ref(), &extra_args, )?; Ok(()) diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs index 40f8d83c73f..e7bacca863e 100644 --- a/src/bin/cargo/commands/logout.rs +++ b/src/bin/cargo/commands/logout.rs @@ -1,5 +1,7 @@ -use crate::command_prelude::*; use cargo::ops; +use cargo::ops::RegistryOrIndex; + +use crate::command_prelude::*; pub fn cli() -> Command { subcommand("logout") @@ -12,7 +14,12 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; - ops::registry_logout(config, registry.as_deref())?; + let reg = args.registry_or_index(config)?; + assert!( + !matches!(reg, Some(RegistryOrIndex::Index(..))), + "must not be index URL" + ); + + ops::registry_logout(config, reg)?; Ok(()) } diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs index 2c3ab0da06e..00080b8292b 100644 --- a/src/bin/cargo/commands/owner.rs +++ b/src/bin/cargo/commands/owner.rs @@ -34,11 +34,10 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; let opts = OwnersOptions { krate: args.get_one::("crate").cloned(), token: args.get_one::("token").cloned().map(Secret::from), - index: args.get_one::("index").cloned(), + reg_or_index: args.registry_or_index(config)?, to_add: args .get_many::("add") .map(|xs| xs.cloned().collect()), @@ -46,7 +45,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .get_many::("remove") .map(|xs| xs.cloned().collect()), list: args.flag("list"), - registry, }; ops::modify_owners(config, &opts)?; Ok(()) diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs index f8377b82343..8ce2ffc5bcd 100644 --- a/src/bin/cargo/commands/publish.rs +++ b/src/bin/cargo/commands/publish.rs @@ -30,7 +30,7 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; + let reg_or_index = args.registry_or_index(config)?; let ws = args.workspace(config)?; if ws.root_maybe().is_embedded() { return Err(anyhow::format_err!( @@ -39,7 +39,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { ) .into()); } - let index = args.index()?; ops::publish( &ws, @@ -48,7 +47,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { token: args .get_one::("token") .map(|s| s.to_string().into()), - index, + reg_or_index, verify: !args.flag("no-verify"), allow_dirty: args.flag("allow-dirty"), to_publish: args.packages_from_flags()?, @@ -56,7 +55,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { jobs: args.jobs()?, keep_going: args.keep_going(), dry_run: args.dry_run(), - registry, cli_features: args.cli_features()?, }, )?; diff --git a/src/bin/cargo/commands/search.rs b/src/bin/cargo/commands/search.rs index 199c2bdbaab..9cacfc7e806 100644 --- a/src/bin/cargo/commands/search.rs +++ b/src/bin/cargo/commands/search.rs @@ -24,8 +24,7 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; - let index = args.index()?; + let reg_or_index = args.registry_or_index(config)?; let limit = args.value_of_u32("limit")?; let limit = min(100, limit.unwrap_or(10)); let query: Vec<&str> = args @@ -34,6 +33,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .map(String::as_str) .collect(); let query: String = query.join("+"); - ops::search(&query, config, index, limit, registry)?; + ops::search(&query, config, reg_or_index, limit)?; Ok(()) } diff --git a/src/bin/cargo/commands/yank.rs b/src/bin/cargo/commands/yank.rs index 9097cb31fe9..62d1821c3ad 100644 --- a/src/bin/cargo/commands/yank.rs +++ b/src/bin/cargo/commands/yank.rs @@ -26,8 +26,6 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { - let registry = args.registry(config)?; - let (krate, version) = resolve_crate( args.get_one::("crate").map(String::as_str), args.get_one::("version").map(String::as_str), @@ -41,9 +39,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { krate.map(|s| s.to_string()), version.map(|s| s.to_string()), args.get_one::("token").cloned().map(Secret::from), - args.get_one::("index").cloned(), + args.registry_or_index(config)?, args.flag("undo"), - registry, )?; Ok(()) } diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index d4ec442dd91..13613eaf635 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -30,6 +30,7 @@ pub use self::registry::yank; pub use self::registry::OwnersOptions; pub use self::registry::PublishOpts; pub use self::registry::RegistryCredentialConfig; +pub use self::registry::RegistryOrIndex; pub use self::resolve::{ add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_with_opts, WorkspaceResolve, diff --git a/src/cargo/ops/registry/login.rs b/src/cargo/ops/registry/login.rs index e523737340f..e21dfe023dc 100644 --- a/src/cargo/ops/registry/login.rs +++ b/src/cargo/ops/registry/login.rs @@ -16,16 +16,23 @@ use cargo_credential::Secret; use super::get_source_id; use super::registry; +use super::RegistryOrIndex; pub fn registry_login( config: &Config, token_from_cmdline: Option>, - reg: Option<&str>, + reg_or_index: Option<&RegistryOrIndex>, args: &[&str], ) -> CargoResult<()> { - let source_ids = get_source_id(config, None, reg)?; - - let login_url = match registry(config, token_from_cmdline.clone(), None, reg, false, None) { + let source_ids = get_source_id(config, reg_or_index)?; + + let login_url = match registry( + config, + token_from_cmdline.clone(), + reg_or_index, + false, + None, + ) { Ok((registry, _)) => Some(format!("{}/me", registry.host())), Err(e) if e.is::() => e .downcast::() diff --git a/src/cargo/ops/registry/logout.rs b/src/cargo/ops/registry/logout.rs index d1f080baefb..fa0cbc539ab 100644 --- a/src/cargo/ops/registry/logout.rs +++ b/src/cargo/ops/registry/logout.rs @@ -8,9 +8,10 @@ use crate::CargoResult; use crate::Config; use super::get_source_id; +use super::RegistryOrIndex; -pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> { - let source_ids = get_source_id(config, None, reg)?; +pub fn registry_logout(config: &Config, reg_or_index: Option) -> CargoResult<()> { + let source_ids = get_source_id(config, reg_or_index.as_ref())?; auth::logout(config, &source_ids.original)?; Ok(()) } diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs index 7d4e99f6bcb..c72a1c1394b 100644 --- a/src/cargo/ops/registry/mod.rs +++ b/src/cargo/ops/registry/mod.rs @@ -16,6 +16,7 @@ use std::task::Poll; use anyhow::{bail, format_err, Context as _}; use cargo_credential::{Operation, Secret}; use crates_io::{self, Registry}; +use url::Url; use crate::core::SourceId; use crate::sources::source::Source; @@ -24,7 +25,6 @@ use crate::util::auth; use crate::util::config::{Config, PathAndArgs}; use crate::util::errors::CargoResult; use crate::util::network::http::http_handle; -use crate::util::IntoUrl; pub use self::login::registry_login; pub use self::logout::registry_logout; @@ -35,6 +35,19 @@ pub use self::publish::PublishOpts; pub use self::search::search; pub use self::yank::yank; +/// Represents either `--registry` or `--index` argument, which is mutually exclusive. +#[derive(Debug, Clone)] +pub enum RegistryOrIndex { + Registry(String), + Index(Url), +} + +impl RegistryOrIndex { + fn is_index(&self) -> bool { + matches!(self, RegistryOrIndex::Index(..)) + } +} + /// Registry settings loaded from config files. /// /// This is loaded based on the `--registry` flag and the config settings. @@ -103,14 +116,14 @@ impl RegistryCredentialConfig { fn registry( config: &Config, token_from_cmdline: Option>, - index: Option<&str>, - registry: Option<&str>, + reg_or_index: Option<&RegistryOrIndex>, force_update: bool, token_required: Option>, ) -> CargoResult<(Registry, RegistrySourceIds)> { - let source_ids = get_source_id(config, index, registry)?; + let source_ids = get_source_id(config, reg_or_index)?; - if token_required.is_some() && index.is_some() && token_from_cmdline.is_none() { + let is_index = reg_or_index.map(|v| v.is_index()).unwrap_or_default(); + if is_index && token_required.is_some() && token_from_cmdline.is_none() { bail!("command-line argument --index requires --token to be specified"); } if let Some(token) = token_from_cmdline { @@ -171,13 +184,12 @@ fn registry( /// crates.io (such as index.crates.io), while the second is always the original source. fn get_source_id( config: &Config, - index: Option<&str>, - reg: Option<&str>, + reg_or_index: Option<&RegistryOrIndex>, ) -> CargoResult { - let sid = match (reg, index) { - (None, None) => SourceId::crates_io(config)?, - (_, Some(i)) => SourceId::for_registry(&i.into_url()?)?, - (Some(r), None) => SourceId::alt_registry(config, r)?, + let sid = match reg_or_index { + None => SourceId::crates_io(config)?, + Some(RegistryOrIndex::Index(url)) => SourceId::for_registry(url)?, + Some(RegistryOrIndex::Registry(r)) => SourceId::alt_registry(config, r)?, }; // Load source replacements that are built-in to Cargo. let builtin_replacement_sid = SourceConfigMap::empty(config)? @@ -186,7 +198,7 @@ fn get_source_id( let replacement_sid = SourceConfigMap::new(config)? .load(sid, &HashSet::new())? .replaced_source_id(); - if reg.is_none() && index.is_none() && replacement_sid != builtin_replacement_sid { + if reg_or_index.is_none() && replacement_sid != builtin_replacement_sid { // Neither --registry nor --index was passed and the user has configured source-replacement. if let Some(replacement_name) = replacement_sid.alt_registry_key() { bail!("crates-io is replaced with remote registry {replacement_name};\ninclude `--registry {replacement_name}` or `--registry crates-io`"); diff --git a/src/cargo/ops/registry/owner.rs b/src/cargo/ops/registry/owner.rs index e29c6400b6b..3f49bb5bb0b 100644 --- a/src/cargo/ops/registry/owner.rs +++ b/src/cargo/ops/registry/owner.rs @@ -13,14 +13,15 @@ use crate::util::important_paths::find_root_manifest_for_wd; use crate::CargoResult; use crate::Config; +use super::RegistryOrIndex; + pub struct OwnersOptions { pub krate: Option, pub token: Option>, - pub index: Option, + pub reg_or_index: Option, pub to_add: Option>, pub to_remove: Option>, pub list: bool, - pub registry: Option, } pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { @@ -38,8 +39,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { let (mut registry, _) = super::registry( config, opts.token.as_ref().map(Secret::as_deref), - opts.index.as_deref(), - opts.registry.as_deref(), + opts.reg_or_index.as_ref(), true, Some(operation), )?; diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 6e43ca2dbcc..7cff0f0a379 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -37,11 +37,12 @@ use crate::CargoResult; use crate::Config; use super::super::check_dep_has_version; +use super::RegistryOrIndex; pub struct PublishOpts<'cfg> { pub config: &'cfg Config, pub token: Option>, - pub index: Option, + pub reg_or_index: Option, pub verify: bool, pub allow_dirty: bool, pub jobs: Option, @@ -49,7 +50,6 @@ pub struct PublishOpts<'cfg> { pub to_publish: ops::Packages, pub targets: Vec, pub dry_run: bool, - pub registry: Option, pub cli_features: CliFeatures, } @@ -76,7 +76,10 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let (pkg, cli_features) = pkgs.pop().unwrap(); - let mut publish_registry = opts.registry.clone(); + let mut publish_registry = match opts.reg_or_index.as_ref() { + Some(RegistryOrIndex::Registry(registry)) => Some(registry.clone()), + _ => None, + }; if let Some(ref allowed_registries) = *pkg.publish() { if publish_registry.is_none() && allowed_registries.len() == 1 { // If there is only one allowed registry, push to that one directly, @@ -116,11 +119,16 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let ver = pkg.version().to_string(); let operation = Operation::Read; + let reg_or_index = match opts.reg_or_index.clone() { + Some(RegistryOrIndex::Registry(_)) | None => { + publish_registry.map(RegistryOrIndex::Registry) + } + val => val, + }; let (mut registry, reg_ids) = super::registry( opts.config, opts.token.as_ref().map(Secret::as_deref), - opts.index.as_deref(), - publish_registry.as_deref(), + reg_or_index.as_ref(), true, Some(operation).filter(|_| !opts.dry_run), )?; diff --git a/src/cargo/ops/registry/search.rs b/src/cargo/ops/registry/search.rs index 6c4a10324de..10b4d600e6b 100644 --- a/src/cargo/ops/registry/search.rs +++ b/src/cargo/ops/registry/search.rs @@ -13,15 +13,16 @@ use crate::util::truncate_with_ellipsis; use crate::CargoResult; use crate::Config; +use super::RegistryOrIndex; + pub fn search( query: &str, config: &Config, - index: Option, + reg_or_index: Option, limit: u32, - reg: Option, ) -> CargoResult<()> { let (mut registry, source_ids) = - super::registry(config, None, index.as_deref(), reg.as_deref(), false, None)?; + super::registry(config, None, reg_or_index.as_ref(), false, None)?; let (crates, total_crates) = registry.search(query, limit).with_context(|| { format!( "failed to retrieve search results from the registry at {}", diff --git a/src/cargo/ops/registry/yank.rs b/src/cargo/ops/registry/yank.rs index d286093b6fd..2a258ea8177 100644 --- a/src/cargo/ops/registry/yank.rs +++ b/src/cargo/ops/registry/yank.rs @@ -13,14 +13,15 @@ use crate::util::config::Config; use crate::util::errors::CargoResult; use crate::util::important_paths::find_root_manifest_for_wd; +use super::RegistryOrIndex; + pub fn yank( config: &Config, krate: Option, version: Option, token: Option>, - index: Option, + reg_or_index: Option, undo: bool, - reg: Option, ) -> CargoResult<()> { let name = match krate { Some(name) => name, @@ -49,8 +50,7 @@ pub fn yank( let (mut registry, _) = super::registry( config, token.as_ref().map(Secret::as_deref), - index.as_deref(), - reg.as_deref(), + reg_or_index.as_ref(), true, Some(message), )?; diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 6ccbbf231f6..f30833b7aa7 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -1,6 +1,7 @@ use crate::core::compiler::{BuildConfig, MessageFormat, TimingOutput}; use crate::core::resolver::CliFeatures; use crate::core::{Edition, Workspace}; +use crate::ops::registry::RegistryOrIndex; use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl}; use crate::util::important_paths::find_root_manifest_for_wd; use crate::util::interning::InternedString; @@ -27,6 +28,7 @@ pub use clap::{value_parser, Arg, ArgAction, ArgMatches}; pub use clap::Command; use super::config::JobsConfig; +use super::IntoUrl; pub mod heading { pub const PACKAGE_SELECTION: &str = "Package Selection"; @@ -744,29 +746,32 @@ Run `{cmd}` to see possible targets." ) } - fn registry(&self, config: &Config) -> CargoResult> { + fn registry_or_index(&self, config: &Config) -> CargoResult> { let registry = self._value_of("registry"); let index = self._value_of("index"); let result = match (registry, index) { - (None, None) => config.default_registry()?, - (None, Some(_)) => { - // If --index is set, then do not look at registry.default. - None - } + (None, None) => config.default_registry()?.map(RegistryOrIndex::Registry), + (None, Some(i)) => Some(RegistryOrIndex::Index(i.into_url()?)), (Some(r), None) => { validate_package_name(r, "registry name", "")?; - Some(r.to_string()) + Some(RegistryOrIndex::Registry(r.to_string())) } (Some(_), Some(_)) => { - bail!("both `--index` and `--registry` should not be set at the same time") + // Should be guarded by clap + unreachable!("both `--index` and `--registry` should not be set at the same time") } }; Ok(result) } - fn index(&self) -> CargoResult> { - let index = self._value_of("index").map(|s| s.to_string()); - Ok(index) + fn registry(&self, config: &Config) -> CargoResult> { + match self._value_of("registry").map(|s| s.to_string()) { + None => config.default_registry(), + Some(registry) => { + validate_package_name(®istry, "registry name", "")?; + Ok(Some(registry)) + } + } } fn check_optional_opts(