Skip to content

Commit 3bb95ae

Browse files
committed
Search cargo build directories before system for libraries
Regardless of crate search paths emitted, always prefer searching search paths pointing into the artifacts directory to those pointing outside. This way libraries built by Cargo are preferred even if the same library name exists in the system & a crate earlier in the build process emitted a system library path for searching.
1 parent 8cfdba9 commit 3bb95ae

File tree

5 files changed

+118
-18
lines changed

5 files changed

+118
-18
lines changed

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,9 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
301301
.extend(output.env.iter().cloned());
302302

303303
for dir in output.library_paths.iter() {
304-
self.compilation.native_dirs.insert(dir.clone());
304+
self.compilation
305+
.native_dirs
306+
.insert(dir.clone().into_path_buf());
305307
}
306308
}
307309
Ok(self.compilation)

src/cargo/core/compiler/custom_build.rs

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
//! [`CompileMode::RunCustomBuild`]: super::CompileMode
3232
//! [instructions]: https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script
3333
34-
use super::{fingerprint, BuildRunner, Job, Unit, Work};
34+
use super::{fingerprint, get_dynamic_search_path, BuildRunner, Job, Unit, Work};
3535
use crate::core::compiler::artifact;
3636
use crate::core::compiler::build_runner::UnitHash;
3737
use crate::core::compiler::fingerprint::DirtyReason;
@@ -74,11 +74,76 @@ pub enum Severity {
7474

7575
pub type LogMessage = (Severity, String);
7676

77+
/// Represents a path added to the library search path.
78+
///
79+
/// We need to keep track of requests to add search paths within the cargo build directory
80+
/// separately from paths outside of Cargo. The reason is that we want to give precedence to linking
81+
/// against libraries within the Cargo build directory even if a similar library exists in the
82+
/// system (e.g. crate A adds `/usr/lib` to the search path and then a later build of crate B adds
83+
/// `target/debug/...` to satisfy its request to link against the library B that it built, but B is
84+
/// also found in `/usr/lib`).
85+
///
86+
/// There's some nuance here because we want to preserve relative order of paths of the same type.
87+
/// For example, if the build process would in declaration order emit the following linker line:
88+
/// ```bash
89+
/// -L/usr/lib -Ltarget/debug/build/crate1/libs -L/lib -Ltarget/debug/build/crate2/libs)
90+
/// ```
91+
///
92+
/// we want the linker to actually receive:
93+
/// ```bash
94+
/// -Ltarget/debug/build/crate1/libs -Ltarget/debug/build/crate2/libs) -L/usr/lib -L/lib
95+
/// ```
96+
///
97+
/// so that the library search paths within the crate artifacts directory come first but retain
98+
/// relative ordering while the system library paths come after while still retaining relative
99+
/// ordering among them; ordering is the order they are emitted within the build process,
100+
/// not lexicographic order.
101+
///
102+
/// WARNING: Even though this type implements PartialOrd + Ord, this is a lexicographic ordering.
103+
/// The linker line will require an explicit sorting algorithm. PartialOrd + Ord is derived because
104+
/// BuildOutput requires it but that ordering is different from the one for the linker search path,
105+
/// at least today. It may be worth reconsidering & perhaps it's ok if BuildOutput doesn't have
106+
/// a lexicographic ordering for the library_paths? I'm not sure the consequence of that.
107+
#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
108+
pub enum LibraryPath {
109+
/// The path is pointing within the output folder of the crate and takes priority over
110+
/// external paths when passed to the linker.
111+
CargoArtifact(PathBuf),
112+
/// The path is pointing outside of the crate's build location. The linker will always
113+
/// receive such paths after `CargoArtifact`.
114+
External(PathBuf),
115+
}
116+
117+
impl LibraryPath {
118+
fn new(p: PathBuf, script_out_dir: &Path) -> Self {
119+
let search_path = get_dynamic_search_path(&p);
120+
if search_path.starts_with(script_out_dir) {
121+
Self::CargoArtifact(p)
122+
} else {
123+
Self::External(p)
124+
}
125+
}
126+
127+
pub fn into_path_buf(self) -> PathBuf {
128+
match self {
129+
LibraryPath::CargoArtifact(p) | LibraryPath::External(p) => p,
130+
}
131+
}
132+
}
133+
134+
impl AsRef<PathBuf> for LibraryPath {
135+
fn as_ref(&self) -> &PathBuf {
136+
match self {
137+
LibraryPath::CargoArtifact(p) | LibraryPath::External(p) => p,
138+
}
139+
}
140+
}
141+
77142
/// Contains the parsed output of a custom build script.
78143
#[derive(Clone, Debug, Hash, Default, PartialEq, Eq, PartialOrd, Ord)]
79144
pub struct BuildOutput {
80145
/// Paths to pass to rustc with the `-L` flag.
81-
pub library_paths: Vec<PathBuf>,
146+
pub library_paths: Vec<LibraryPath>,
82147
/// Names and link kinds of libraries, suitable for the `-l` flag.
83148
pub library_links: Vec<String>,
84149
/// Linker arguments suitable to be passed to `-C link-arg=<args>`
@@ -237,7 +302,7 @@ fn emit_build_output(
237302
let library_paths = output
238303
.library_paths
239304
.iter()
240-
.map(|l| l.display().to_string())
305+
.map(|l| l.as_ref().display().to_string())
241306
.collect::<Vec<_>>();
242307

243308
let msg = machine_message::BuildScript {
@@ -884,10 +949,16 @@ impl BuildOutput {
884949
"rustc-flags" => {
885950
let (paths, links) = BuildOutput::parse_rustc_flags(&value, &whence)?;
886951
library_links.extend(links.into_iter());
887-
library_paths.extend(paths.into_iter());
952+
library_paths.extend(
953+
paths
954+
.into_iter()
955+
.map(|p| LibraryPath::new(p, script_out_dir)),
956+
);
888957
}
889958
"rustc-link-lib" => library_links.push(value.to_string()),
890-
"rustc-link-search" => library_paths.push(PathBuf::from(value)),
959+
"rustc-link-search" => {
960+
library_paths.push(LibraryPath::new(PathBuf::from(value), script_out_dir))
961+
}
891962
"rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => {
892963
if !targets.iter().any(|target| target.is_cdylib()) {
893964
log_messages.push((

src/cargo/core/compiler/mod.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ pub use self::compilation::{Compilation, Doctest, UnitOutput};
7979
pub use self::compile_kind::{CompileKind, CompileTarget};
8080
pub use self::crate_type::CrateType;
8181
pub use self::custom_build::LinkArgTarget;
82-
pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts};
82+
pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts, LibraryPath};
8383
pub(crate) use self::fingerprint::DirtyReason;
8484
pub use self::job_queue::Freshness;
8585
use self::job_queue::{Job, JobQueue, JobState, Work};
@@ -495,16 +495,39 @@ fn rustc(
495495
target: &Target,
496496
current_id: PackageId,
497497
) -> CargoResult<()> {
498+
let mut library_paths = vec![];
499+
500+
for key in build_scripts.to_link.iter() {
501+
let output = build_script_outputs.get(key.1).ok_or_else(|| {
502+
internal(format!(
503+
"couldn't find build script output for {}/{}",
504+
key.0, key.1
505+
))
506+
})?;
507+
library_paths.extend(output.library_paths.iter());
508+
}
509+
510+
// NOTE: This very intentionally does not use the derived ord from LibraryPath because we need to
511+
// retain relative ordering within the same type (i.e. not lexicographic). The use of a stable sort
512+
// is also important here because it ensures that paths of the same type retain the same relative
513+
// ordering (for an unstable sort to work here, the list would need to retain the idx of each element
514+
// and then sort by that idx when the type is equivalent.
515+
library_paths.sort_by_key(|p| match p {
516+
LibraryPath::CargoArtifact(_) => 0,
517+
LibraryPath::External(_) => 1,
518+
});
519+
520+
for path in library_paths.iter() {
521+
rustc.arg("-L").arg(path.as_ref());
522+
}
523+
498524
for key in build_scripts.to_link.iter() {
499525
let output = build_script_outputs.get(key.1).ok_or_else(|| {
500526
internal(format!(
501527
"couldn't find build script output for {}/{}",
502528
key.0, key.1
503529
))
504530
})?;
505-
for path in output.library_paths.iter() {
506-
rustc.arg("-L").arg(path);
507-
}
508531

509532
if key.0 == current_id {
510533
if pass_l_flag {
@@ -650,7 +673,7 @@ fn add_plugin_deps(
650673
.get(*metadata)
651674
.ok_or_else(|| internal(format!("couldn't find libs for plugin dep {}", pkg_id)))?;
652675
search_path.append(&mut filter_dynamic_search_path(
653-
output.library_paths.iter(),
676+
output.library_paths.iter().map(AsRef::as_ref),
654677
root_output,
655678
));
656679
}

src/cargo/util/context/target.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{ConfigKey, ConfigRelativePath, GlobalContext, OptValue, PathAndArgs, StringList, CV};
2-
use crate::core::compiler::{BuildOutput, LinkArgTarget};
2+
use crate::core::compiler::{BuildOutput, LibraryPath, LinkArgTarget};
33
use crate::util::CargoResult;
44
use serde::Deserialize;
55
use std::collections::{BTreeMap, HashMap};
@@ -167,7 +167,9 @@ fn parse_links_overrides(
167167
let flags = value.string(key)?;
168168
let whence = format!("target config `{}.{}` (in {})", target_key, key, flags.1);
169169
let (paths, links) = BuildOutput::parse_rustc_flags(flags.0, &whence)?;
170-
output.library_paths.extend(paths);
170+
output
171+
.library_paths
172+
.extend(paths.into_iter().map(LibraryPath::External));
171173
output.library_links.extend(links);
172174
}
173175
"rustc-link-lib" => {
@@ -178,9 +180,11 @@ fn parse_links_overrides(
178180
}
179181
"rustc-link-search" => {
180182
let list = value.list(key)?;
181-
output
182-
.library_paths
183-
.extend(list.iter().map(|v| PathBuf::from(&v.0)));
183+
output.library_paths.extend(
184+
list.iter()
185+
.map(|v| PathBuf::from(&v.0))
186+
.map(LibraryPath::External),
187+
);
184188
}
185189
"rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => {
186190
let args = extra_link_args(LinkArgTarget::Cdylib, key, value)?;

tests/testsuite/build_script.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6171,7 +6171,7 @@ fn linker_search_path_preference() {
61716171
// -L /usr/lib2 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsB.1 -L /lib2 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsB.2
61726172
p.cargo("build -v").with_stderr_data(str![[r#"
61736173
...
6174-
[RUNNING] `rustc --crate-name foo [..] -L /usr/lib -L [ROOT]/foo/target/debug/build/foo-[HASH]/out/libs2 -L /lib -L [ROOT]/foo/target/debug/build/foo-[HASH]/out/libs1 -L /usr/lib3 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsA.2 -L /lib3 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsA.1 -L /usr/lib2 -L [ROOT]/foo/target/debug/build/b-[HASH]/out/libsB.1 -L /lib2 -L [ROOT]/foo/target/debug/build/b-[HASH]/out/libsB.2`
6174+
[RUNNING] `rustc --crate-name foo [..] -L [ROOT]/foo/target/debug/build/foo-[HASH]/out/libs2 -L [ROOT]/foo/target/debug/build/foo-[HASH]/out/libs1 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsA.2 -L [ROOT]/foo/target/debug/build/a-[HASH]/out/libsA.1 -L [ROOT]/foo/target/debug/build/b-[HASH]/out/libsB.1 -L [ROOT]/foo/target/debug/build/b-[HASH]/out/libsB.2 -L /usr/lib -L /lib -L /usr/lib3 -L /lib3 -L /usr/lib2 -L /lib2`
61756175
...
61766176
"#]]).run();
6177-
}
6177+
}

0 commit comments

Comments
 (0)