Skip to content

Commit 6d1bd93

Browse files
authored
fix: preserve dep_name for build script metadata (rust-lang#16494)
### What does this PR try to resolve? rust-lang#16493 regressed because we looked up the manifest dependency name by matching SourceId, which fails when `[patch]` changes the source. This fix ensures the synthetic RunCustomBuild-RunCustomBuild edges carry the manifest dep name, so `CARGO_DEP_*` environment variable uses the correct prefix even with patched sources and renamed dependencies. ### How to test and review this PR? Run repro in rust-lang#16493. Fixes rust-lang#16493
2 parents 186bb00 + 09ea79d commit 6d1bd93

File tree

4 files changed

+69
-18
lines changed

4 files changed

+69
-18
lines changed

src/cargo/core/compiler/custom_build.rs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -469,20 +469,10 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
469469
if dep.unit.mode.is_run_custom_build() {
470470
let dep_metadata = build_runner.get_run_build_script_metadata(&dep.unit);
471471

472-
let Some(dependency) = unit.pkg.dependencies().iter().find(|d| {
473-
d.package_name() == dep.unit.pkg.name()
474-
&& d.source_id() == dep.unit.pkg.package_id().source_id()
475-
&& d.version_req().matches(dep.unit.pkg.version())
476-
}) else {
477-
panic!(
478-
"Dependency `{}` not found in `{}`s dependencies",
479-
dep.unit.pkg.name(),
480-
unit.pkg.name()
481-
)
482-
};
472+
let dep_name = dep.dep_name.unwrap_or(dep.unit.pkg.name());
483473

484474
Some((
485-
dependency.name_in_toml(),
475+
dep_name,
486476
dep.unit
487477
.pkg
488478
.manifest()

src/cargo/core/compiler/unit_dependencies.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,16 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) {
10051005
state.unit_dependencies[&other.unit]
10061006
.iter()
10071007
.find(|other_dep| other_dep.unit.mode == CompileMode::RunCustomBuild)
1008-
.cloned()
1008+
.map(|other_dep| {
1009+
let mut dep = other_dep.clone();
1010+
let dep_name = other.dep_name.unwrap_or(other.unit.pkg.name());
1011+
// Propagate the manifest dep name from the sibling edge.
1012+
// The RunCustomBuild-RustCustomBuild edge is synthetic
1013+
// and doesn't carry a usable dep name, but build script
1014+
// metadata needs one for `CARGO_DEP_<dep_name>_*` env var
1015+
dep.dep_name = Some(dep_name);
1016+
dep
1017+
})
10091018
})
10101019
.collect::<HashSet<_>>();
10111020

src/cargo/core/compiler/unit_graph.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,15 @@ pub struct UnitDep {
2626
pub unit_for: UnitFor,
2727
/// The name the parent uses to refer to this dependency.
2828
pub extern_crate_name: InternedString,
29-
/// If `Some`, the name of the dependency if renamed in toml.
30-
/// It's particularly interesting to artifact dependencies which rely on it
31-
/// for naming their environment variables. Note that the `extern_crate_name`
32-
/// cannot be used for this as it also may be the build target itself,
33-
/// which isn't always the renamed dependency name.
29+
/// The dependency name as written in the manifest (including a rename).
30+
///
31+
/// `None` means this edge does not carry a manifest dep name. For example,
32+
/// std edges in build-std or synthetic edges for build script executions.
33+
/// When `None`, the package name is typically used by callers as a fallback.
34+
///
35+
/// This is mainly for Cargo-synthesized outputs
36+
/// (artifact env vars and `CARGO_DEP_*` metadata env)
37+
/// and is distinct from `extern_crate_name`.
3438
pub dep_name: Option<InternedString>,
3539
/// Whether or not this is a public dependency.
3640
pub public: bool,

tests/testsuite/build_script.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,6 +1722,54 @@ fn non_links_can_pass_env_vars_direct_deps_only() {
17221722
.run();
17231723
}
17241724

1725+
/// Regression test for https://github.com/rust-lang/cargo/issues/16493
1726+
#[cargo_test]
1727+
fn with_patch() {
1728+
Package::new("cxx", "1.0.0").publish();
1729+
1730+
let p = project()
1731+
.file(
1732+
"Cargo.toml",
1733+
r#"
1734+
[package]
1735+
name = "foo"
1736+
edition = "2021"
1737+
1738+
[dependencies]
1739+
cxx = "1.0.0"
1740+
1741+
[patch.crates-io]
1742+
cxx = { path = "cxx" }
1743+
"#,
1744+
)
1745+
.file("src/lib.rs", "")
1746+
.file("build.rs", "fn main() {}")
1747+
.file(
1748+
"cxx/Cargo.toml",
1749+
r#"
1750+
[package]
1751+
name = "cxx"
1752+
version = "1.0.0"
1753+
edition = "2021"
1754+
links = "cxx"
1755+
"#,
1756+
)
1757+
.file("cxx/src/lib.rs", "")
1758+
.file("cxx/build.rs", "fn main() {}")
1759+
.build();
1760+
1761+
p.cargo("check")
1762+
.with_stderr_data(str![[r#"
1763+
[UPDATING] `dummy-registry` index
1764+
[LOCKING] 1 package to latest compatible version
1765+
[COMPILING] cxx v1.0.0 ([ROOT]/foo/cxx)
1766+
[COMPILING] foo v0.0.0 ([ROOT]/foo)
1767+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
1768+
1769+
"#]])
1770+
.run();
1771+
}
1772+
17251773
#[cargo_test]
17261774
fn only_rerun_build_script() {
17271775
let p = project()

0 commit comments

Comments
 (0)