Implement RFC 3289: source replacement ambiguity

Author: arlosi

crates/cargo-test-support/src/lib.rs

@@ -818,6 +818,17 @@ impl Execs {
         self
     }
 
+    /// Overrides the crates.io URL for testing.
+    ///
+    /// Can be used for testing crates-io functionality where alt registries
+    /// cannot be used.
+    pub fn replace_crates_io(&mut self, url: &Url) -> &mut Self {
+        if let Some(ref mut p) = self.process_builder {
+            p.env("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS", url.as_str());
+        }
+        self
+    }
+
     pub fn enable_mac_dsym(&mut self) -> &mut Self {
         if cfg!(target_os = "macos") {
             self.env("CARGO_PROFILE_DEV_SPLIT_DEBUGINFO", "packed")

crates/cargo-test-support/src/registry.rs

@@ -105,7 +105,7 @@ impl RegistryBuilder {
     pub fn new() -> RegistryBuilder {
         RegistryBuilder {
             alternative: None,
-            token: Some("api-token".to_string()),
+            token: None,
             http_api: false,
             http_index: false,
             api: true,
@@ -197,6 +197,7 @@ impl RegistryBuilder {
         let dl_url = generate_url(&format!("{prefix}dl"));
         let dl_path = generate_path(&format!("{prefix}dl"));
         let api_path = generate_path(&format!("{prefix}api"));
+        let token = Some(self.token.unwrap_or_else(|| format!("{prefix}sekrit")));
 
         let (server, index_url, api_url, dl_url) = if !self.http_index && !self.http_api {
             // No need to start the HTTP server.
@@ -205,7 +206,7 @@ impl RegistryBuilder {
             let server = HttpServer::new(
                 registry_path.clone(),
                 dl_path,
-                self.token.clone(),
+                token.clone(),
                 self.custom_responders,
             );
             let index_url = if self.http_index {
@@ -228,7 +229,7 @@ impl RegistryBuilder {
             _server: server,
             dl_url,
             path: registry_path,
-            token: self.token,
+            token,
         };
 
         if self.configure_registry {
@@ -252,8 +253,8 @@ impl RegistryBuilder {
                     [source.crates-io]
                     replace-with = 'dummy-registry'
 
-                    [source.dummy-registry]
-                    registry = '{}'",
+                    [registries.dummy-registry]
+                    index = '{}'",
                         registry.index_url
                     )
                     .as_bytes(),

src/bin/cargo/commands/add.rs

@@ -1,3 +1,4 @@
+use cargo::sources::CRATES_IO_REGISTRY;
 use indexmap::IndexMap;
 use indexmap::IndexSet;
 
@@ -207,7 +208,10 @@ fn parse_dependencies(config: &Config, matches: &ArgMatches) -> CargoResult<Vec<
     let rev = matches.get_one::<String>("rev");
     let tag = matches.get_one::<String>("tag");
     let rename = matches.get_one::<String>("rename");
-    let registry = matches.registry(config)?;
+    let registry = match matches.registry(config)? {
+        Some(reg) if reg == CRATES_IO_REGISTRY => None,
+        reg => reg,
+    };
     let default_features = default_features(matches);
     let optional = optional(matches);
 

src/cargo/core/compiler/rustdoc.rs

@@ -128,7 +128,7 @@ pub fn add_root_urls(
                     if !sid.is_registry() {
                         return false;
                     }
-                    if sid.is_default_registry() {
+                    if sid.is_crates_io() {
                         return registry == CRATES_IO_REGISTRY;
                     }
                     if let Some(index_url) = name2url.get(registry) {

src/cargo/core/package_id.rs

@@ -211,7 +211,7 @@ impl fmt::Display for PackageId {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         write!(f, "{} v{}", self.inner.name, self.inner.version)?;
 
-        if !self.inner.source_id.is_default_registry() {
+        if !self.inner.source_id.is_crates_io() {
             write!(f, " ({})", self.inner.source_id)?;
         }
 

src/cargo/core/source/source_id.rs

@@ -39,6 +39,10 @@ struct SourceIdInner {
     /// WARNING: this is not always set for alt-registries when the name is
     /// not known.
     name: Option<String>,
+    /// Name of the alt registry in the `[registries]` table.
+    /// WARNING: this is not always set for alt-registries when the name is
+    /// not known.
+    alt_registry_key: Option<String>,
 }
 
 /// The possible kinds of code source. Along with `SourceIdInner`, this fully defines the
@@ -81,6 +85,7 @@ impl SourceId {
             url,
             precise: None,
             name: name.map(|n| n.into()),
+            alt_registry_key: None,
         });
         Ok(source_id)
     }
@@ -221,13 +226,17 @@ impl SourceId {
 
     /// Gets the `SourceId` associated with given name of the remote registry.
     pub fn alt_registry(config: &Config, key: &str) -> CargoResult<SourceId> {
+        if key == CRATES_IO_REGISTRY {
+            return Self::crates_io(config);
+        }
         let url = config.get_registry_index(key)?;
         Ok(SourceId::wrap(SourceIdInner {
             kind: SourceKind::Registry,
             canonical_url: CanonicalUrl::new(&url)?,
             url,
             precise: None,
             name: Some(key.to_string()),
+            alt_registry_key: Some(key.to_string()),
         }))
     }
 
@@ -243,15 +252,15 @@ impl SourceId {
     }
 
     pub fn display_index(self) -> String {
-        if self.is_default_registry() {
+        if self.is_crates_io() {
             format!("{} index", CRATES_IO_DOMAIN)
         } else {
             format!("`{}` index", self.display_registry_name())
         }
     }
 
     pub fn display_registry_name(self) -> String {
-        if self.is_default_registry() {
+        if self.is_crates_io() {
             CRATES_IO_REGISTRY.to_string()
         } else if let Some(name) = &self.inner.name {
             name.clone()
@@ -264,6 +273,13 @@ impl SourceId {
         }
     }
 
+    /// Gets the name of the remote registry as defined in the `[registries]` table.
+    /// WARNING: alt registries that come from Cargo.lock, or --index will
+    /// not have a name.
+    pub fn alt_registry_key(&self) -> Option<&str> {
+        self.inner.alt_registry_key.as_deref()
+    }
+
     /// Returns `true` if this source is from a filesystem path.
     pub fn is_path(self) -> bool {
         self.inner.kind == SourceKind::Path
@@ -364,13 +380,15 @@ impl SourceId {
     }
 
     /// Returns `true` if the remote registry is the standard <https://crates.io>.
-    pub fn is_default_registry(self) -> bool {
+    pub fn is_crates_io(self) -> bool {
         match self.inner.kind {
             SourceKind::Registry => {}
             _ => return false,
         }
         let url = self.inner.url.as_str();
-        url == CRATES_IO_INDEX || url == CRATES_IO_HTTP_INDEX
+        url == CRATES_IO_INDEX
+            || url == CRATES_IO_HTTP_INDEX
+            || std::env::var("__CARGO_TEST_CRATES_IO_URL_DO_NOT_USE_THIS").as_deref() == Ok(url)
     }
 
     /// Hashes `self`.

src/cargo/ops/registry.rs

@@ -209,7 +209,7 @@ fn verify_dependencies(
             // This extra hostname check is mostly to assist with testing,
             // but also prevents someone using `--index` to specify
             // something that points to crates.io.
-            if registry_src.is_default_registry() || registry.host_is_crates_io() {
+            if registry_src.is_crates_io() || registry.host_is_crates_io() {
                 bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
                        registries. `{}` needs to be published to crates.io before publishing this crate.\n\
                        (crate `{}` is pulled from {})",
@@ -391,6 +391,22 @@ pub fn registry_configuration(
     };
     // `registry.default` is handled in command-line parsing.
     let (token, process) = match registry {
+        Some("crates-io") | None => {
+            // Use crates.io default.
+            config.check_registry_index_not_set()?;
+            let token = config.get_string("registry.token")?.map(|p| p.val);
+            let process = if config.cli_unstable().credential_process {
+                let process =
+                    config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
+                if token.is_some() && process.is_some() {
+                    return err_both("registry.token", "registry.credential-process");
+                }
+                process
+            } else {
+                None
+            };
+            (token, process)
+        }
         Some(registry) => {
             let token_key = format!("registries.{registry}.token");
             let token = config.get_string(&token_key)?.map(|p| p.val);
@@ -411,22 +427,6 @@ pub fn registry_configuration(
             };
             (token, process)
         }
-        None => {
-            // Use crates.io default.
-            config.check_registry_index_not_set()?;
-            let token = config.get_string("registry.token")?.map(|p| p.val);
-            let process = if config.cli_unstable().credential_process {
-                let process =
-                    config.get::<Option<config::PathAndArgs>>("registry.credential-process")?;
-                if token.is_some() && process.is_some() {
-                    return err_both("registry.token", "registry.credential-process");
-                }
-                process
-            } else {
-                None
-            };
-            (token, process)
-        }
     };
 
     let credential_process =
@@ -444,11 +444,9 @@ pub fn registry_configuration(
 ///
 /// * `token`: The token from the command-line. If not set, uses the token
 ///   from the config.
-/// * `index`: The index URL from the command-line. This is ignored if
-///   `registry` is set.
+/// * `index`: The index URL from the command-line.
 /// * `registry`: The registry name from the command-line. If neither
-///   `registry`, or `index` are set, then uses `crates-io`, honoring
-///   `[source]` replacement if defined.
+///   `registry`, or `index` are set, then uses `crates-io`.
 /// * `force_update`: If `true`, forces the index to be updated.
 /// * `validate_token`: If `true`, the token must be set.
 fn registry(
@@ -459,24 +457,8 @@ fn registry(
     force_update: bool,
     validate_token: bool,
 ) -> CargoResult<(Registry, RegistryConfig, SourceId)> {
-    if index.is_some() && registry.is_some() {
-        // Otherwise we would silently ignore one or the other.
-        bail!("both `--index` and `--registry` should not be set at the same time");
-    }
-    // Parse all configuration options
+    let (sid, sid_no_replacement) = get_source_id(config, index, registry)?;
     let reg_cfg = registry_configuration(config, registry)?;
-    let opt_index = registry
-        .map(|r| config.get_registry_index(r))
-        .transpose()?
-        .map(|u| u.to_string());
-    let sid = get_source_id(config, opt_index.as_deref().or(index), registry)?;
-    if !sid.is_remote_registry() {
-        bail!(
-            "{} does not support API commands.\n\
-             Check for a source-replacement in .cargo/config.",
-            sid
-        );
-    }
     let api_host = {
         let _lock = config.acquire_package_cache_lock()?;
         let mut src = RegistrySource::remote(sid, &HashSet::new(), config)?;
@@ -503,42 +485,18 @@ fn registry(
             }
             token
         } else {
-            // Check `is_default_registry` so that the crates.io index can
-            // change config.json's "api" value, and this won't affect most
-            // people. It will affect those using source replacement, but
-            // hopefully that's a relatively small set of users.
-            if token.is_none()
-                && reg_cfg.is_token()
-                && registry.is_none()
-                && !sid.is_default_registry()
-                && !crates_io::is_url_crates_io(&api_host)
-            {
-                config.shell().warn(
-                    "using `registry.token` config value with source \
-                        replacement is deprecated\n\
-                        This may become a hard error in the future; \
-                        see <https://github.com/rust-lang/cargo/issues/xxx>.\n\
-                        Use the --token command-line flag to remove this warning.",
-                )?;
-                reg_cfg.as_token().map(|t| t.to_owned())
-            } else {
-                let token =
-                    auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
-                Some(token)
-            }
+            let token = auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
+            Some(token)
         }
     } else {
         None
     };
     let handle = http_handle(config)?;
-    // Workaround for the sparse+https://index.crates.io replacement index. Use the non-replaced
-    // source_id so that the original (github) url is used when publishing a crate.
-    let sid = if sid.is_default_registry() {
-        SourceId::crates_io(config)?
-    } else {
-        sid
-    };
-    Ok((Registry::new_handle(api_host, token, handle), reg_cfg, sid))
+    Ok((
+        Registry::new_handle(api_host, token, handle),
+        reg_cfg,
+        sid_no_replacement,
+    ))
 }
 
 /// Creates a new HTTP handle with appropriate global configuration for cargo.
@@ -947,16 +905,45 @@ pub fn yank(
 /// Gets the SourceId for an index or registry setting.
 ///
 /// The `index` and `reg` values are from the command-line or config settings.
-/// If both are None, returns the source for crates.io.
-fn get_source_id(config: &Config, index: Option<&str>, reg: Option<&str>) -> CargoResult<SourceId> {
-    match (reg, index) {
-        (Some(r), _) => SourceId::alt_registry(config, r),
-        (_, Some(i)) => SourceId::for_registry(&i.into_url()?),
-        _ => {
-            let map = SourceConfigMap::new(config)?;
-            let src = map.load(SourceId::crates_io(config)?, &HashSet::new())?;
-            Ok(src.replaced_source_id())
+/// If both are None, and no source-replacement is configured, returns the source for crates.io.
+/// If both are None, and source replacement is configured, returns an error.
+///
+/// The source for crates.io may be GitHub, index.crates.io, or a test-only registry depending
+/// on configuration.
+///
+/// If `reg` is set, source replacement is not followed.
+///
+/// The return value is a pair of `SourceId`s: The first may be a built-in replacement of
+/// 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>,
+) -> CargoResult<(SourceId, SourceId)> {
+    let sid = match (reg, index) {
+        (None, None) => SourceId::crates_io(config)?,
+        (Some(r), None) => SourceId::alt_registry(config, r)?,
+        (None, Some(i)) => SourceId::for_registry(&i.into_url()?)?,
+        (Some(_), Some(_)) => {
+            bail!("both `--index` and `--registry` should not be set at the same time")
+        }
+    };
+    // Load source replacements that are built-in to Cargo.
+    let builtin_replacement_sid = SourceConfigMap::empty(config)?
+        .load(sid, &HashSet::new())?
+        .replaced_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 {
+        // 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");
         }
+    } else {
+        Ok((builtin_replacement_sid, sid))
     }
 }
 
@@ -1024,7 +1011,7 @@ pub fn search(
             &ColorSpec::new(),
         );
     } else if total_crates > limit && limit >= search_max_limit {
-        let extra = if source_id.is_default_registry() {
+        let extra = if source_id.is_crates_io() {
             format!(
                 " (go to https://crates.io/search?q={} to see more)",
                 percent_encode(query.as_bytes(), NON_ALPHANUMERIC)