Skip to content

Commit 801942b

Browse files
committed
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
2 parents feb2103 + 30cc7ce commit 801942b

File tree

2 files changed

+66
-5
lines changed

2 files changed

+66
-5
lines changed

src/cargo/sources/config.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
9090
id: SourceId::crates_io(config)?,
9191
replace_with: None,
9292
},
93-
);
93+
)?;
9494
Ok(base)
9595
}
9696

@@ -185,9 +185,22 @@ restore the source replacement configuration to continue the build
185185
Ok(Box::new(ReplacedSource::new(id, new_id, new_src)))
186186
}
187187

188-
fn add(&mut self, name: &str, cfg: SourceConfig) {
189-
self.id2name.insert(cfg.id, name.to_string());
188+
fn add(&mut self, name: &str, cfg: SourceConfig) -> CargoResult<()> {
189+
if let Some(old_name) = self.id2name.insert(cfg.id, name.to_string()) {
190+
// The user is allowed to redefine the built-in crates-io
191+
// definition from `empty()`.
192+
if name != CRATES_IO_REGISTRY {
193+
bail!(
194+
"source `{}` defines source {}, but that source is already defined by `{}`\n\
195+
note: Sources are not allowed to be defined multiple times.",
196+
name,
197+
cfg.id,
198+
old_name
199+
);
200+
}
201+
}
190202
self.cfgs.insert(name.to_string(), cfg);
203+
Ok(())
191204
}
192205

193206
fn add_config(&mut self, name: String, def: SourceConfigDef) -> CargoResult<()> {
@@ -262,7 +275,7 @@ restore the source replacement configuration to continue the build
262275
id: src,
263276
replace_with,
264277
},
265-
);
278+
)?;
266279

267280
return Ok(());
268281

tests/testsuite/bad_config.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,6 @@ fn bad_source_config4() {
974974
".cargo/config",
975975
r#"
976976
[source.crates-io]
977-
registry = 'https://example.com'
978977
replace-with = 'bar'
979978
980979
[source.bar]
@@ -1394,3 +1393,52 @@ fn bad_target_links_overrides() {
13941393
.with_stderr("[ERROR] `warning` is not supported in build script overrides")
13951394
.run();
13961395
}
1396+
1397+
#[cargo_test]
1398+
fn redefined_sources() {
1399+
// Cannot define a source multiple times.
1400+
let p = project()
1401+
.file(
1402+
".cargo/config",
1403+
r#"
1404+
[source.foo]
1405+
registry = "https://github.com/rust-lang/crates.io-index"
1406+
"#,
1407+
)
1408+
.file("src/lib.rs", "")
1409+
.build();
1410+
1411+
p.cargo("check")
1412+
.with_status(101)
1413+
.with_stderr(
1414+
"\
1415+
[ERROR] source `foo` defines source registry `https://github.com/rust-lang/crates.io-index`, \
1416+
but that source is already defined by `crates-io`
1417+
note: Sources are not allowed to be defined multiple times.
1418+
",
1419+
)
1420+
.run();
1421+
1422+
p.change_file(
1423+
".cargo/config",
1424+
r#"
1425+
[source.one]
1426+
directory = "index"
1427+
1428+
[source.two]
1429+
directory = "index"
1430+
"#,
1431+
);
1432+
1433+
// Name is `[..]` because we can't guarantee the order.
1434+
p.cargo("check")
1435+
.with_status(101)
1436+
.with_stderr(
1437+
"\
1438+
[ERROR] source `[..]` defines source dir [..]/foo/index, \
1439+
but that source is already defined by `[..]`
1440+
note: Sources are not allowed to be defined multiple times.
1441+
",
1442+
)
1443+
.run();
1444+
}

0 commit comments

Comments
 (0)