Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/cargo/core/compiler/build_runner/mod.rs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I just got another idea: the current deps_output map collects every dylib output dir from every unit in the compilation session, which is not ideal. Some of the unit only need a subtree of it, and would be basically zero dylib path.

Anyway, I believe we don't need to put that much effort on this becuase dylibs are not that common as dependencies.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm yeah, good point. I probably added unit_graph.iter() to cast a wide net as possible.
I think we could construct lists per Unit instead of CompileKind.

Though, like you mentioned, dylibs usually don't make up a large percent of the crate graph so not sure if the effort for that optimization would be worth it

Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,18 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
}
if self.bcx.gctx.cli_unstable().build_dir_new_layout {
for (unit, _) in self.bcx.unit_graph.iter() {
Comment thread
weihanglo marked this conversation as resolved.
if kind != unit.kind {
continue;
}
let dep_dir = self.files().deps_dir(unit);
paths::create_dir_all(&dep_dir)?;
self.compilation
.deps_output
.entry(kind)
.or_default()
.insert(dep_dir);
if unit.target.is_dylib() {
Comment thread
weihanglo marked this conversation as resolved.
Comment thread
weihanglo marked this conversation as resolved.
self.compilation
.deps_output
.entry(kind)
.or_default()
.insert(dep_dir);
}
}
} else {
self.compilation
Expand Down
8 changes: 6 additions & 2 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,9 @@ impl<'gctx> Compilation<'gctx> {
&self.root_output[&CompileKind::Host],
));
}
search_path.extend(self.deps_output[&CompileKind::Host].clone());
if let Some(paths) = self.deps_output.get(&CompileKind::Host) {
search_path.extend(paths.clone());
}
} else {
if let Some(path) = self.root_output.get(&kind) {
search_path.extend(super::filter_dynamic_search_path(
Expand All @@ -385,7 +387,9 @@ impl<'gctx> Compilation<'gctx> {
));
search_path.push(path.clone());
}
search_path.extend(self.deps_output[&kind].clone());
if let Some(paths) = self.deps_output.get(&kind) {
search_path.extend(paths.clone());
}
// For build-std, we don't want to accidentally pull in any shared
// libs from the sysroot that ships with rustc. This may not be
// required (at least I cannot craft a situation where it
Expand Down
121 changes: 121 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Tests for the `cargo build` command.

use std::env;
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
use std::fs;
use std::io::Read;
use std::process::Stdio;
Expand Down Expand Up @@ -6598,3 +6599,123 @@ fn should_not_include_build_script_out_dir_path_in_rustc_args() {
"#]])
.run();
}

#[cargo_test(
nightly,
reason = "Depends on https://github.com/rust-lang/rust/pull/155439/changes/61f3e086acc1c187bb262ab43cac71f44018c397"
)]
fn should_only_include_dylibs_on_lib_search_path() {
Comment thread
weihanglo marked this conversation as resolved.
let envvar = dylib_path_envvar();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
edition = "2021"
authors = []
resolver = "2"

[dependencies]
bar = { path = "bar" }
baz = { path = "baz" }
qux = { path = "qux" }
"#,
)
.file(
"src/main.rs",
&format!(
r#"
fn main() {{
let search_path = std::env::var_os("{envvar}").unwrap();
let paths: Vec<_> = std::env::split_paths(&search_path).collect();

for (name, kind) in [("bar", "dylib"), ("baz", "rlib"), ("qux", "cdylib")] {{
let found = paths.iter().any(|dir| artifact_exists(dir, name, kind));
println!("{{name}} ({{kind}}) on search path: {{found}}");
}}
}}

fn artifact_exists(dir: &std::path::Path, name: &str, kind: &str) -> bool {{
let (prefix, suffix) = if kind == "rlib" {{
(format!("lib{{name}}-"), ".rlib".to_string())
}} else {{
(format!("{DLL_PREFIX}{{name}}"), "{DLL_SUFFIX}".to_string())
}};
std::fs::read_dir(dir)
.into_iter()
.flatten()
.flatten()
.any(|e| {{
let f = e.file_name();
let f = f.to_string_lossy();
f.starts_with(&prefix) && f.ends_with(&suffix)
}})
}}
"#
),
)
.file(
"bar/Cargo.toml",
r#"
[package]
name = "bar"
version = "0.1.0"
edition = "2021"
authors = []

[lib]
crate-type = ["dylib"]
"#,
)
.file("bar/src/lib.rs", r#"pub fn bar_value() -> i32 { 100 }"#)
.file(
"baz/Cargo.toml",
r#"
[package]
name = "baz"
version = "0.1.0"
edition = "2021"
authors = []
"#,
)
.file("baz/src/lib.rs", r#"pub fn baz_value() -> i32 { 200 }"#)
.file(
"qux/Cargo.toml",
r#"
[package]
name = "qux"
version = "0.1.0"
edition = "2021"
authors = []
Comment thread
weihanglo marked this conversation as resolved.

[lib]
crate-type = ["cdylib"]
"#,
)
.file("qux/src/lib.rs", r#"pub fn qux_value() -> i32 { 200 }"#)
.build();

p.cargo("-Zbuild-dir-new-layout -v run")
.masquerade_as_nightly_cargo(&["new build-dir layout"])
.enable_mac_dsym()
.with_stdout_data(str![[r#"
bar (dylib) on search path: true
baz (rlib) on search path: false
qux (cdylib) on search path: false

"#]])
.run();

// Legacy layout
p.cargo("-v run")
.enable_mac_dsym()
.with_stdout_data(str![[r#"
bar (dylib) on search path: true
baz (rlib) on search path: true
qux (cdylib) on search path: true

"#]])
.run();
}
99 changes: 99 additions & 0 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Tests for build.rs scripts.

use std::env;
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
use std::fs;
use std::io;
use std::thread;
Expand All @@ -17,6 +18,7 @@ use cargo_test_support::registry::Package;
use cargo_test_support::str;
use cargo_test_support::{basic_manifest, cross_compile, is_coarse_mtime, project, project_in};
use cargo_test_support::{git, rustc_host, sleep_ms, slow_cpu_multiplier, symlink_supported};
use cargo_util::paths::dylib_path_envvar;
use cargo_util::paths::{self, remove_dir_all};

#[cargo_test]
Expand Down Expand Up @@ -6870,3 +6872,100 @@ fn target_linker_does_not_apply_to_build_script_with_host_config() {
"#]])
.run();
}

#[cargo_test(
nightly,
reason = "Depends on https://github.com/rust-lang/rust/pull/155439/changes/61f3e086acc1c187bb262ab43cac71f44018c397"
)]
fn build_script_dylib_search_path_excludes_target_dylibs() {
if cross_compile_disabled() {
return;
}

let envvar = dylib_path_envvar();
let target = cross_compile::alternate();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
edition = "2021"
authors = []

[dependencies]
bar = { path = "bar" }

[build-dependencies]
hostbar = { path = "hostbar" }
"#,
)
.file(
"build.rs",
&format!(
r#"
fn main() {{
let search_path = std::env::var_os("{envvar}").unwrap();
let paths: Vec<_> = std::env::split_paths(&search_path).collect();

let found_hostbar = paths.iter()
.any(|dir| dir.join("{DLL_PREFIX}hostbar{DLL_SUFFIX}").exists());
assert!(
found_hostbar,
"hostbar (host-arch build-dependency dylib) should be on \
the build script's search path, but wasn't: {{:?}}",
paths
);

let found_bar = paths.iter()
.any(|dir| dir.join("{DLL_PREFIX}bar{DLL_SUFFIX}").exists());
assert!(
!found_bar,
"bar ({target}-arch regular dependency dylib) must NOT be on \
the host build script's search path, but was: {{:?}}",
paths
);
}}
"#
),
)
.file("src/main.rs", "fn main() {}")
.file(
"bar/Cargo.toml",
r#"
[package]
name = "bar"
version = "0.1.0"
edition = "2021"
authors = []
[lib]
crate-type = ["dylib"]
"#,
)
.file("bar/src/lib.rs", "pub fn bar_value() -> i32 { 100 }")
.file(
"hostbar/Cargo.toml",
r#"
[package]
name = "hostbar"
version = "0.1.0"
edition = "2021"
authors = []
[lib]
crate-type = ["dylib"]
"#,
)
.file(
"hostbar/src/lib.rs",
"pub fn hostbar_value() -> i32 { 200 }",
)
.build();

p.cargo("build -Zbuild-dir-new-layout -v --target")
.arg(&target)
.masquerade_as_nightly_cargo(&["new build-dir layout"])
.enable_mac_dsym()
.run();
}