Auto merge of #7751 - ehuss:multiple-source-definition, r=alexcrichton

Check for a source defined multiple times.

There is a bug where if a source is defined in multiple `[source]` tables, it causes a key collision in `SourceConfigMap::id2name`.  This can result in random behavior depending on which key is inserted first.

I decided to just make it an error.  I can't think of a way to make it work since the `replace-with` chain walking depends on unique sourceid->name mappings.  If anyone has ideas how to support it, I can try, but I don't immediately see how.

Closes #7692

src/cargo/sources/config.rs

@@ -90,7 +90,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
                 id: SourceId::crates_io(config)?,
                 replace_with: None,
             },
-        );
+        )?;
         Ok(base)
     }
 
@@ -185,9 +185,22 @@ restore the source replacement configuration to continue the build
         Ok(Box::new(ReplacedSource::new(id, new_id, new_src)))
     }
 
-    fn add(&mut self, name: &str, cfg: SourceConfig) {
-        self.id2name.insert(cfg.id, name.to_string());
+    fn add(&mut self, name: &str, cfg: SourceConfig) -> CargoResult<()> {
+        if let Some(old_name) = self.id2name.insert(cfg.id, name.to_string()) {
+            // The user is allowed to redefine the built-in crates-io
+            // definition from `empty()`.
+            if name != CRATES_IO_REGISTRY {
+                bail!(
+                    "source `{}` defines source {}, but that source is already defined by `{}`\n\
+                     note: Sources are not allowed to be defined multiple times.",
+                    name,
+                    cfg.id,
+                    old_name
+                );
+            }
+        }
         self.cfgs.insert(name.to_string(), cfg);
+        Ok(())
     }
 
     fn add_config(&mut self, name: String, def: SourceConfigDef) -> CargoResult<()> {
@@ -262,7 +275,7 @@ restore the source replacement configuration to continue the build
                 id: src,
                 replace_with,
             },
-        );
+        )?;
 
         return Ok(());
 

tests/testsuite/bad_config.rs

@@ -974,7 +974,6 @@ fn bad_source_config4() {
             ".cargo/config",
             r#"
             [source.crates-io]
-            registry = 'https://example.com'
             replace-with = 'bar'
 
             [source.bar]
@@ -1394,3 +1393,52 @@ fn bad_target_links_overrides() {
         .with_stderr("[ERROR] `warning` is not supported in build script overrides")
         .run();
 }
+
+#[cargo_test]
+fn redefined_sources() {
+    // Cannot define a source multiple times.
+    let p = project()
+        .file(
+            ".cargo/config",
+            r#"
+            [source.foo]
+            registry = "https://github.com/rust-lang/crates.io-index"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] source `foo` defines source registry `https://github.com/rust-lang/crates.io-index`, \
+    but that source is already defined by `crates-io`
+note: Sources are not allowed to be defined multiple times.
+",
+        )
+        .run();
+
+    p.change_file(
+        ".cargo/config",
+        r#"
+        [source.one]
+        directory = "index"
+
+        [source.two]
+        directory = "index"
+        "#,
+    );
+
+    // Name is `[..]` because we can't guarantee the order.
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+[ERROR] source `[..]` defines source dir [..]/foo/index, \
+    but that source is already defined by `[..]`
+note: Sources are not allowed to be defined multiple times.
+",
+        )
+        .run();
+}