Auto merge of #14218 - hi-rustin:rustin-patch-refactor, r=epage

refactor: move get_source_id out of registry

Author: bors

src/cargo/ops/registry/login.rs

@@ -26,8 +26,15 @@ pub fn registry_login(
 ) -> CargoResult<()> {
     let source_ids = get_source_id(gctx, reg_or_index)?;
 
-    let login_url = match registry(gctx, token_from_cmdline.clone(), reg_or_index, false, None) {
-        Ok((registry, _)) => Some(format!("{}/me", registry.host())),
+    let login_url = match registry(
+        gctx,
+        &source_ids,
+        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>()
             .unwrap()

src/cargo/ops/registry/mod.rs

@@ -107,6 +107,8 @@ impl RegistryCredentialConfig {
 
 /// Returns the `Registry` and `Source` based on command-line and config settings.
 ///
+/// * `source_ids`: The source IDs for the registry. It contains the original source ID and
+///   the replacement source ID.
 /// * `token_from_cmdline`: The token from the command-line. If not set, uses the token
 ///   from the config.
 /// * `index`: The index URL from the command-line.
@@ -116,13 +118,12 @@ impl RegistryCredentialConfig {
 /// * `token_required`: If `true`, the token will be set.
 fn registry(
     gctx: &GlobalContext,
+    source_ids: &RegistrySourceIds,
     token_from_cmdline: Option<Secret<&str>>,
     reg_or_index: Option<&RegistryOrIndex>,
     force_update: bool,
     token_required: Option<Operation<'_>>,
-) -> CargoResult<(Registry, RegistrySourceIds)> {
-    let source_ids = get_source_id(gctx, reg_or_index)?;
-
+) -> CargoResult<Registry> {
     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");
@@ -165,9 +166,11 @@ fn registry(
         None
     };
     let handle = http_handle(gctx)?;
-    Ok((
-        Registry::new_handle(api_host, token, handle, cfg.auth_required),
-        source_ids,
+    Ok(Registry::new_handle(
+        api_host,
+        token,
+        handle,
+        cfg.auth_required,
     ))
 }
 
@@ -188,25 +191,11 @@ fn get_source_id(
     gctx: &GlobalContext,
     reg_or_index: Option<&RegistryOrIndex>,
 ) -> CargoResult<RegistrySourceIds> {
-    let sid = match reg_or_index {
-        None => SourceId::crates_io(gctx)?,
-        Some(RegistryOrIndex::Index(url)) => SourceId::for_registry(url)?,
-        Some(RegistryOrIndex::Registry(r)) => SourceId::alt_registry(gctx, r)?,
-    };
-    // Load source replacements that are built-in to Cargo.
-    let builtin_replacement_sid = SourceConfigMap::empty(gctx)?
-        .load(sid, &HashSet::new())?
-        .replaced_source_id();
-    let replacement_sid = SourceConfigMap::new(gctx)?
-        .load(sid, &HashSet::new())?
-        .replaced_source_id();
+    let sid = get_initial_source_id(gctx, reg_or_index)?;
+    let (builtin_replacement_sid, replacement_sid) = get_replacement_source_ids(gctx, 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`");
-        } else {
-            bail!("crates-io is replaced with non-remote-registry source {replacement_sid};\ninclude `--registry crates-io` to use crates.io");
-        }
+        bail!(gen_replacement_error(replacement_sid));
     } else {
         Ok(RegistrySourceIds {
             original: sid,
@@ -215,6 +204,56 @@ fn get_source_id(
     }
 }
 
+fn get_initial_source_id(
+    gctx: &GlobalContext,
+    reg_or_index: Option<&RegistryOrIndex>,
+) -> CargoResult<SourceId> {
+    match reg_or_index {
+        None => SourceId::crates_io(gctx),
+        Some(reg_or_index) => get_initial_source_id_from_registry_or_index(gctx, reg_or_index),
+    }
+}
+
+fn get_initial_source_id_from_registry_or_index(
+    gctx: &GlobalContext,
+    reg_or_index: &RegistryOrIndex,
+) -> CargoResult<SourceId> {
+    match reg_or_index {
+        RegistryOrIndex::Index(url) => SourceId::for_registry(url),
+        RegistryOrIndex::Registry(r) => SourceId::alt_registry(gctx, r),
+    }
+}
+
+fn get_replacement_source_ids(
+    gctx: &GlobalContext,
+    sid: SourceId,
+) -> CargoResult<(SourceId, SourceId)> {
+    let builtin_replacement_sid = SourceConfigMap::empty(gctx)?
+        .load(sid, &HashSet::new())?
+        .replaced_source_id();
+    let replacement_sid = SourceConfigMap::new(gctx)?
+        .load(sid, &HashSet::new())?
+        .replaced_source_id();
+    Ok((builtin_replacement_sid, replacement_sid))
+}
+
+fn gen_replacement_error(replacement_sid: SourceId) -> String {
+    // Neither --registry nor --index was passed and the user has configured source-replacement.
+    let error_message = if let Some(replacement_name) = replacement_sid.alt_registry_key() {
+        format!(
+            "crates-io is replaced with remote registry {};\ninclude `--registry {}` or `--registry crates-io`",
+            replacement_name, replacement_name
+        )
+    } else {
+        format!(
+            "crates-io is replaced with non-remote-registry source {};\ninclude `--registry crates-io` to use crates.io",
+            replacement_sid
+        )
+    };
+
+    error_message
+}
+
 struct RegistrySourceIds {
     /// Use when looking up the auth token, or writing out `Cargo.lock`
     original: SourceId,

src/cargo/ops/registry/owner.rs

@@ -35,9 +35,10 @@ pub fn modify_owners(gctx: &GlobalContext, opts: &OwnersOptions) -> CargoResult<
     };
 
     let operation = Operation::Owners { name: &name };
-
-    let (mut registry, _) = super::registry(
+    let source_ids = super::get_source_id(gctx, opts.reg_or_index.as_ref())?;
+    let mut registry = super::registry(
         gctx,
+        &source_ids,
         opts.token.as_ref().map(Secret::as_deref),
         opts.reg_or_index.as_ref(),
         true,

src/cargo/ops/registry/publish.rs

@@ -130,14 +130,16 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
         }
         val => val,
     };
-    let (mut registry, reg_ids) = super::registry(
+    let source_ids = super::get_source_id(opts.gctx, reg_or_index.as_ref())?;
+    let mut registry = super::registry(
         opts.gctx,
+        &source_ids,
         opts.token.as_ref().map(Secret::as_deref),
         reg_or_index.as_ref(),
         true,
         Some(operation).filter(|_| !opts.dry_run),
     )?;
-    verify_dependencies(pkg, &registry, reg_ids.original)?;
+    verify_dependencies(pkg, &registry, source_ids.original)?;
 
     // Prepare a tarball, with a non-suppressible warning if metadata
     // is missing since this is being put online.
@@ -169,7 +171,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
         };
         registry.set_token(Some(auth::auth_token(
             &opts.gctx,
-            &reg_ids.original,
+            &source_ids.original,
             None,
             operation,
             vec![],
@@ -185,7 +187,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
         pkg,
         tarball.file(),
         &mut registry,
-        reg_ids.original,
+        source_ids.original,
         opts.dry_run,
     )?;
     if !opts.dry_run {
@@ -198,7 +200,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
         };
         if 0 < timeout {
             let timeout = Duration::from_secs(timeout);
-            wait_for_publish(opts.gctx, reg_ids.original, pkg, timeout)?;
+            wait_for_publish(opts.gctx, source_ids.original, pkg, timeout)?;
         }
     }
 

src/cargo/ops/registry/search.rs

@@ -20,8 +20,9 @@ pub fn search(
     reg_or_index: Option<RegistryOrIndex>,
     limit: u32,
 ) -> CargoResult<()> {
-    let (mut registry, source_ids) =
-        super::registry(gctx, None, reg_or_index.as_ref(), false, None)?;
+    let source_ids = super::get_source_id(gctx, reg_or_index.as_ref())?;
+    let mut registry =
+        super::registry(gctx, &source_ids, 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 {}",

src/cargo/ops/registry/yank.rs

@@ -46,9 +46,10 @@ pub fn yank(
             vers: &version,
         }
     };
-
-    let (mut registry, _) = super::registry(
+    let source_ids = super::get_source_id(gctx, reg_or_index.as_ref())?;
+    let mut registry = super::registry(
         gctx,
+        &source_ids,
         token.as_ref().map(Secret::as_deref),
         reg_or_index.as_ref(),
         true,