Implement RFC 3289: source replacement ambiguity

Author: arlosi

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

@@ -818,6 +818,20 @@ impl Execs {
         self
     }
 
+    /// Allows replacement of crates-io source without specifying --registry <NAME>
+    ///
+    /// Can be used for testing crates-io specific items where alt registries
+    /// cannot be used.
+    pub fn allow_silent_crates_io_replacement(&mut self) -> &mut Self {
+        if let Some(ref mut p) = self.process_builder {
+            p.env(
+                "__CARGO_TEST_ALLOW_CRATES_IO_REPLACEMENT_DO_NOT_USE",
+                "true",
+            );
+        }
+        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

@@ -250,8 +250,8 @@ impl RegistryBuilder {
                     [source.crates-io]
                     replace-with = 'dummy-registry'
 
-                    [source.dummy-registry]
-                    registry = '{}'",
+                    [registries.dummy-registry]
+                    index = '{}'",
                         registry.index_url
                     )
                     .as_bytes(),
@@ -282,6 +282,9 @@ impl RegistryBuilder {
                         r#"
                     [registry]
                     token = "{token}"
+
+                    [registries.dummy-registry]
+                    token = "{token}"
                 "#
                     )
                     .as_bytes(),

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)
     }
@@ -228,6 +233,7 @@ impl SourceId {
             url,
             precise: None,
             name: Some(key.to_string()),
+            alt_registry_key: Some(key.to_string()),
         }))
     }
 
@@ -264,6 +270,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

src/cargo/ops/registry.rs

@@ -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.
@@ -943,16 +901,41 @@ 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 (except when testing Cargo).
+///
+/// If `reg` is set, source replacement is not followed.
+fn get_source_id(
+    config: &Config,
+    index: Option<&str>,
+    reg: Option<&str>,
+) -> CargoResult<(SourceId, SourceId)> {
+    let map = SourceConfigMap::new(config)?;
+    let sid = match (reg, index) {
+        (None, None) | (Some("crates-io"), 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")
         }
+    };
+    let src = map.load(sid, &HashSet::new())?;
+    if src.is_replaced() && reg.is_none() && index.is_none() {
+        // Neither --registry nor --index was passed and source replacement is configured.
+        let replacement_sid = src.replaced_source_id();
+        if replacement_sid.is_default_registry()
+            || std::env::var("__CARGO_TEST_ALLOW_CRATES_IO_REPLACEMENT_DO_NOT_USE").is_ok()
+        {
+            // Allow replacement of crates.io for the sparse crates.io endpoint or for Cargo's
+            // testing framework.
+            Ok((replacement_sid, sid))
+        } else 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 publish to crates.io");
+        }
+    } else {
+        Ok((sid.clone(), sid))
     }
 }
 

src/cargo/sources/config.rs

@@ -121,18 +121,24 @@ impl<'cfg> SourceConfigMap<'cfg> {
         };
         let mut cfg_loc = "";
         let orig_name = name;
-        let new_id;
-        loop {
+        let new_id = loop {
             let cfg = match self.cfgs.get(name) {
                 Some(cfg) => cfg,
-                None => bail!(
-                    "could not find a configured source with the \
+                None => {
+                    // Attempt to interpret the source name as an alt registry name
+                    if let Ok(alt_id) = SourceId::alt_registry(self.config, name) {
+                        debug!("following pointer to registry {}", name);
+                        break alt_id.with_precise(id.precise().map(str::to_string));
+                    }
+                    bail!(
+                        "could not find a configured source with the \
                      name `{}` when attempting to lookup `{}` \
                      (configuration in `{}`)",
-                    name,
-                    orig_name,
-                    cfg_loc
-                ),
+                        name,
+                        orig_name,
+                        cfg_loc
+                    );
+                }
             };
             match &cfg.replace_with {
                 Some((s, c)) => {
@@ -141,8 +147,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
                 }
                 None if id == cfg.id => return id.load(self.config, yanked_whitelist),
                 None => {
-                    new_id = cfg.id.with_precise(id.precise().map(|s| s.to_string()));
-                    break;
+                    break cfg.id.with_precise(id.precise().map(|s| s.to_string()));
                 }
             }
             debug!("following pointer to {}", name);
@@ -155,7 +160,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
                     cfg_loc
                 )
             }
-        }
+        };
 
         let new_src = new_id.load(
             self.config,

src/doc/src/reference/source-replacement.md

@@ -50,6 +50,9 @@ directory = "vendor"
 # The crates.io default source for crates is available under the name
 # "crates-io", and here we use the `replace-with` key to indicate that it's
 # replaced with our source above.
+#
+# The `replace-with` key can also reference an alternative registry name
+# defined in the `[registries]` table.
 [source.crates-io]
 replace-with = "my-vendor-source"
 

tests/testsuite/artifact_dep.rs

@@ -1901,7 +1901,7 @@ fn publish_artifact_dep() {
         .file("src/lib.rs", "")
         .build();
 
-    p.cargo("publish -Z bindeps --no-verify --token sekrit")
+    p.cargo("publish --registry dummy-registry -Z bindeps --no-verify --token sekrit")
         .masquerade_as_nightly_cargo(&["bindeps"])
         .with_stderr(
             "\

tests/testsuite/bad_config.rs

@@ -74,7 +74,7 @@ fn bad3() {
         .build();
     Package::new("foo", "1.0.0").publish();
 
-    p.cargo("publish -v")
+    p.cargo("publish --registry dummy-registry -v")
         .with_status(101)
         .with_stderr(
             "\
@@ -125,7 +125,7 @@ fn bad6() {
         .build();
     Package::new("foo", "1.0.0").publish();
 
-    p.cargo("publish -v")
+    p.cargo("publish --registry dummy-registry -v")
         .with_status(101)
         .with_stderr(
             "\

tests/testsuite/cargo_features.rs

@@ -626,7 +626,7 @@ fn publish_allowed() {
         )
         .file("src/lib.rs", "")
         .build();
-    p.cargo("publish --token sekrit")
+    p.cargo("publish --registry dummy-registry --token sekrit")
         .masquerade_as_nightly_cargo(&["test-dummy-unstable"])
         .run();
 }