diff --git a/src/cargo/core/compiler/build_runner/mod.rs b/src/cargo/core/compiler/build_runner/mod.rs index ec8d6bd1e..6daa35465 100644 --- a/src/cargo/core/compiler/build_runner/mod.rs +++ b/src/cargo/core/compiler/build_runner/mod.rs @@ -301,7 +301,9 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> { .extend(output.env.iter().cloned()); for dir in output.library_paths.iter() { - self.compilation.native_dirs.insert(dir.clone()); + self.compilation + .native_dirs + .insert(dir.clone().into_path_buf()); } } Ok(self.compilation) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 5483eabc8..16702f0c5 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -31,7 +31,7 @@ //! [`CompileMode::RunCustomBuild`]: super::CompileMode //! [instructions]: https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script -use super::{fingerprint, BuildRunner, Job, Unit, Work}; +use super::{fingerprint, get_dynamic_search_path, BuildRunner, Job, Unit, Work}; use crate::core::compiler::artifact; use crate::core::compiler::build_runner::UnitHash; use crate::core::compiler::fingerprint::DirtyReason; @@ -75,11 +75,76 @@ pub enum Severity { pub type LogMessage = (Severity, String); +/// Represents a path added to the library search path. +/// +/// We need to keep track of requests to add search paths within the cargo build directory +/// separately from paths outside of Cargo. The reason is that we want to give precedence to linking +/// against libraries within the Cargo build directory even if a similar library exists in the +/// system (e.g. crate A adds `/usr/lib` to the search path and then a later build of crate B adds +/// `target/debug/...` to satisfy its request to link against the library B that it built, but B is +/// also found in `/usr/lib`). +/// +/// There's some nuance here because we want to preserve relative order of paths of the same type. +/// For example, if the build process would in declaration order emit the following linker line: +/// ```bash +/// -L/usr/lib -Ltarget/debug/build/crate1/libs -L/lib -Ltarget/debug/build/crate2/libs) +/// ``` +/// +/// we want the linker to actually receive: +/// ```bash +/// -Ltarget/debug/build/crate1/libs -Ltarget/debug/build/crate2/libs) -L/usr/lib -L/lib +/// ``` +/// +/// so that the library search paths within the crate artifacts directory come first but retain +/// relative ordering while the system library paths come after while still retaining relative +/// ordering among them; ordering is the order they are emitted within the build process, +/// not lexicographic order. +/// +/// WARNING: Even though this type implements PartialOrd + Ord, this is a lexicographic ordering. +/// The linker line will require an explicit sorting algorithm. PartialOrd + Ord is derived because +/// BuildOutput requires it but that ordering is different from the one for the linker search path, +/// at least today. It may be worth reconsidering & perhaps it's ok if BuildOutput doesn't have +/// a lexicographic ordering for the library_paths? I'm not sure the consequence of that. +#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub enum LibraryPath { + /// The path is pointing within the output folder of the crate and takes priority over + /// external paths when passed to the linker. + CargoArtifact(PathBuf), + /// The path is pointing outside of the crate's build location. The linker will always + /// receive such paths after `CargoArtifact`. + External(PathBuf), +} + +impl LibraryPath { + fn new(p: PathBuf, script_out_dir: &Path) -> Self { + let search_path = get_dynamic_search_path(&p); + if search_path.starts_with(script_out_dir) { + Self::CargoArtifact(p) + } else { + Self::External(p) + } + } + + pub fn into_path_buf(self) -> PathBuf { + match self { + LibraryPath::CargoArtifact(p) | LibraryPath::External(p) => p, + } + } +} + +impl AsRef for LibraryPath { + fn as_ref(&self) -> &PathBuf { + match self { + LibraryPath::CargoArtifact(p) | LibraryPath::External(p) => p, + } + } +} + /// Contains the parsed output of a custom build script. #[derive(Clone, Debug, Hash, Default, PartialEq, Eq, PartialOrd, Ord)] pub struct BuildOutput { /// Paths to pass to rustc with the `-L` flag. - pub library_paths: Vec, + pub library_paths: Vec, /// Names and link kinds of libraries, suitable for the `-l` flag. pub library_links: Vec, /// Linker arguments suitable to be passed to `-C link-arg=` @@ -239,7 +304,7 @@ fn emit_build_output( let library_paths = output .library_paths .iter() - .map(|l| l.display().to_string()) + .map(|l| l.as_ref().display().to_string()) .collect::>(); let msg = machine_message::BuildScript { @@ -886,10 +951,16 @@ impl BuildOutput { "rustc-flags" => { let (paths, links) = BuildOutput::parse_rustc_flags(&value, &whence)?; library_links.extend(links.into_iter()); - library_paths.extend(paths.into_iter()); + library_paths.extend( + paths + .into_iter() + .map(|p| LibraryPath::new(p, script_out_dir)), + ); } "rustc-link-lib" => library_links.push(value.to_string()), - "rustc-link-search" => library_paths.push(PathBuf::from(value)), + "rustc-link-search" => { + library_paths.push(LibraryPath::new(PathBuf::from(value), script_out_dir)) + } "rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => { if !targets.iter().any(|target| target.is_cdylib()) { log_messages.push(( diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 7825a30fe..2a0fd8b40 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -79,7 +79,7 @@ pub use self::compilation::{Compilation, Doctest, UnitOutput}; pub use self::compile_kind::{CompileKind, CompileKindFallback, CompileTarget}; pub use self::crate_type::CrateType; pub use self::custom_build::LinkArgTarget; -pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts}; +pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts, LibraryPath}; pub(crate) use self::fingerprint::DirtyReason; pub use self::job_queue::Freshness; use self::job_queue::{Job, JobQueue, JobState, Work}; @@ -497,6 +497,32 @@ fn rustc( current_id: PackageId, mode: CompileMode, ) -> CargoResult<()> { + let mut library_paths = vec![]; + + for key in build_scripts.to_link.iter() { + let output = build_script_outputs.get(key.1).ok_or_else(|| { + internal(format!( + "couldn't find build script output for {}/{}", + key.0, key.1 + )) + })?; + library_paths.extend(output.library_paths.iter()); + } + + // NOTE: This very intentionally does not use the derived ord from LibraryPath because we need to + // retain relative ordering within the same type (i.e. not lexicographic). The use of a stable sort + // is also important here because it ensures that paths of the same type retain the same relative + // ordering (for an unstable sort to work here, the list would need to retain the idx of each element + // and then sort by that idx when the type is equivalent. + library_paths.sort_by_key(|p| match p { + LibraryPath::CargoArtifact(_) => 0, + LibraryPath::External(_) => 1, + }); + + for path in library_paths.iter() { + rustc.arg("-L").arg(path.as_ref()); + } + for key in build_scripts.to_link.iter() { let output = build_script_outputs.get(key.1).ok_or_else(|| { internal(format!( @@ -504,9 +530,6 @@ fn rustc( key.0, key.1 )) })?; - for path in output.library_paths.iter() { - rustc.arg("-L").arg(path); - } if key.0 == current_id { if pass_l_flag { @@ -654,7 +677,7 @@ fn add_plugin_deps( .get(*metadata) .ok_or_else(|| internal(format!("couldn't find libs for plugin dep {}", pkg_id)))?; search_path.append(&mut filter_dynamic_search_path( - output.library_paths.iter(), + output.library_paths.iter().map(AsRef::as_ref), root_output, )); } @@ -663,6 +686,13 @@ fn add_plugin_deps( Ok(()) } +fn get_dynamic_search_path(path: &Path) -> &Path { + match path.to_str().and_then(|s| s.split_once("=")) { + Some(("native" | "crate" | "dependency" | "framework" | "all", path)) => Path::new(path), + _ => path, + } +} + // Determine paths to add to the dynamic search path from -L entries // // Strip off prefixes like "native=" or "framework=" and filter out directories @@ -674,12 +704,9 @@ where { let mut search_path = vec![]; for dir in paths { - let dir = match dir.to_str().and_then(|s| s.split_once("=")) { - Some(("native" | "crate" | "dependency" | "framework" | "all", path)) => path.into(), - _ => dir.clone(), - }; + let dir = get_dynamic_search_path(dir); if dir.starts_with(&root_output) { - search_path.push(dir); + search_path.push(dir.to_path_buf()); } else { debug!( "Not including path {} in runtime library search path because it is \ diff --git a/src/cargo/util/context/target.rs b/src/cargo/util/context/target.rs index 3de3d826a..0dd5cfc4c 100644 --- a/src/cargo/util/context/target.rs +++ b/src/cargo/util/context/target.rs @@ -1,5 +1,5 @@ use super::{ConfigKey, ConfigRelativePath, GlobalContext, OptValue, PathAndArgs, StringList, CV}; -use crate::core::compiler::{BuildOutput, LinkArgTarget}; +use crate::core::compiler::{BuildOutput, LibraryPath, LinkArgTarget}; use crate::util::CargoResult; use serde::Deserialize; use std::collections::{BTreeMap, HashMap}; @@ -167,7 +167,9 @@ fn parse_links_overrides( let flags = value.string(key)?; let whence = format!("target config `{}.{}` (in {})", target_key, key, flags.1); let (paths, links) = BuildOutput::parse_rustc_flags(flags.0, &whence)?; - output.library_paths.extend(paths); + output + .library_paths + .extend(paths.into_iter().map(LibraryPath::External)); output.library_links.extend(links); } "rustc-link-lib" => { @@ -178,9 +180,11 @@ fn parse_links_overrides( } "rustc-link-search" => { let list = value.list(key)?; - output - .library_paths - .extend(list.iter().map(|v| PathBuf::from(&v.0))); + output.library_paths.extend( + list.iter() + .map(|v| PathBuf::from(&v.0)) + .map(LibraryPath::External), + ); } "rustc-link-arg-cdylib" | "rustc-cdylib-link-arg" => { let args = extra_link_args(LinkArgTarget::Cdylib, key, value)?; diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index f93cbe04e..17d8228bb 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -6084,3 +6084,90 @@ fn directory_with_leading_underscore() { .with_status(0) .run(); } + +#[cargo_test] +fn linker_search_path_preference() { + // This isn't strictly the exact scenario that causes the issue, but it's the shortest demonstration + // of the issue. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2024" + build = "build.rs" + + [dependencies] + a = { path = "a" } + b = { path = "b" } + "#, + ) + .file( + "build.rs", + r#" + fn main() { + let out_dir = std::env::var("OUT_DIR").unwrap(); + println!("cargo::rustc-link-search=/usr/lib"); + println!("cargo::rustc-link-search={}/libs2", out_dir); + println!("cargo::rustc-link-search=/lib"); + println!("cargo::rustc-link-search={}/libs1", out_dir); + } + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + edition = "2024" + build = "build.rs" + "#, + ) + .file("a/src/lib.rs", "") + .file( + "a/build.rs", + r#" + fn main() { + let out_dir = std::env::var("OUT_DIR").unwrap(); + println!("cargo::rustc-link-search=/usr/lib3"); + println!("cargo::rustc-link-search={}/libsA.2", out_dir); + println!("cargo::rustc-link-search=/lib3"); + println!("cargo::rustc-link-search={}/libsA.1", out_dir); + } + "#, + ) + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + edition = "2024" + build = "build.rs" + "#, + ) + .file("b/src/lib.rs", "") + .file( + "b/build.rs", + r#" + fn main() { + let out_dir = std::env::var("OUT_DIR").unwrap(); + println!("cargo::rustc-link-search=/usr/lib2"); + println!("cargo::rustc-link-search={}/libsB.1", out_dir); + println!("cargo::rustc-link-search=/lib2"); + println!("cargo::rustc-link-search={}/libsB.2", out_dir); + } + "#, + ) + .build(); + + p.cargo("build -v").with_stderr_data(str![[r#" +... +[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` +... +"#]]).run(); +}