Auto merge of #12677 - weihanglo:registry-or-index, r=hi-rustin

refactor: use `RegistryOrIndex` enum to replace two booleans
rust-lang · Sep 17, 2023 · 41cef47 · 41cef47
2 parents 4c10811 + 11754e9
commit 41cef47
Show file tree

Hide file tree

Showing 24 changed files with 186 additions and 95 deletions.
diff --git 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 `<cyan,bold>cargo help init</>` for more detailed information.\n"

diff --git 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::<String>("index") {
-        SourceId::for_registry(&index.into_url()?)?
-    } else if let Some(registry) = args.registry(config)? {
-        SourceId::alt_registry(config, &registry)?
+    } 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
@@ -1,12 +1,13 @@
-use crate::command_prelude::*;
-
 use cargo::ops;
+use cargo::ops::RegistryOrIndex;
+
+use crate::command_prelude::*;
 
 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)")
@@ -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::<String>("args")
         .unwrap_or_default()
@@ -29,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     ops::registry_login(
         config,
         args.get_one::<String>("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
@@ -1,18 +1,25 @@
-use crate::command_prelude::*;
 use cargo::ops;
+use cargo::ops::RegistryOrIndex;
+
+use crate::command_prelude::*;
 
 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 `<cyan,bold>cargo help logout</>` for more detailed information.\n"
         ))
 }
 
 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/new.rs b/src/bin/cargo/commands/new.rs
@@ -7,7 +7,7 @@ pub fn cli() -> Command {
         .about("Create a new cargo package at <path>")
         .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 `<cyan,bold>cargo help new</>` for more detailed information.\n"

diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs
@@ -24,29 +24,27 @@ 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 `<cyan,bold>cargo help owner</>` for more detailed information.\n"
         ))
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let registry = args.registry(config)?;
     let opts = OwnersOptions {
         krate: args.get_one::<String>("crate").cloned(),
         token: args.get_one::<String>("token").cloned().map(Secret::from),
-        index: args.get_one::<String>("index").cloned(),
+        reg_or_index: args.registry_or_index(config)?,
         to_add: args
             .get_many::<String>("add")
             .map(|xs| xs.cloned().collect()),
         to_remove: args
             .get_many::<String>("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
@@ -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",
@@ -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,15 +47,14 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             token: args
                 .get_one::<String>("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()?,
             targets: args.targets()?,
             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
@@ -15,17 +15,16 @@ 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 `<cyan,bold>cargo help search</>` for more detailed information.\n"
         ))
 }
 
 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
@@ -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!(
@@ -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::<String>("crate").map(String::as_str),
         args.get_one::<String>("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::<String>("token").cloned().map(Secret::from),
-        args.get_one::<String>("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
@@ -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
@@ -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<Secret<&str>>,
-    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::<AuthorizationError>() => e
             .downcast::<AuthorizationError>()

diff --git 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<RegistryOrIndex>) -> 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
@@ -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<Secret<&str>>,
-    index: Option<&str>,
-    registry: Option<&str>,
+    reg_or_index: Option<&RegistryOrIndex>,
     force_update: bool,
     token_required: Option<Operation<'_>>,
 ) -> 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<RegistrySourceIds> {
-    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`");