From 91f75ac4bb23d0b72a805316046513785c8c0e63 Mon Sep 17 00:00:00 2001 From: Weihang Lo <me@weihanglo.tw> Date: Tue, 20 Feb 2024 12:49:19 -0500 Subject: [PATCH 1/3] refactor: enum to represent kinds of tools being run --- src/cargo/core/compiler/compilation.rs | 32 +++++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 018792d2b13..0e768bfcb11 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -13,6 +13,25 @@ use crate::core::compiler::{CompileKind, Metadata, Unit}; use crate::core::Package; use crate::util::{config, CargoResult, GlobalContext}; +/// Represents the kind of process we are creating. +#[derive(Debug)] +enum ToolKind { + /// See [`Compilation::rustc_process`]. + Rustc, + /// See [`Compilation::rustdoc_process`]. + Rustdoc, + /// See [`Compilation::host_process`]. + HostProcess, + /// See [`Compilation::target_process`]. + TargetProcess, +} + +impl ToolKind { + fn is_rustc_tool(&self) -> bool { + matches!(self, ToolKind::Rustc | ToolKind::Rustdoc) + } +} + /// Structure with enough information to run `rustdoc --test`. pub struct Doctest { /// What's being doctested @@ -176,7 +195,7 @@ impl<'gctx> Compilation<'gctx> { }; let cmd = fill_rustc_tool_env(rustc, unit); - self.fill_env(cmd, &unit.pkg, None, unit.kind, true) + self.fill_env(cmd, &unit.pkg, None, unit.kind, ToolKind::Rustc) } /// Returns a [`ProcessBuilder`] for running `rustdoc`. @@ -187,7 +206,7 @@ impl<'gctx> Compilation<'gctx> { ) -> CargoResult<ProcessBuilder> { let rustdoc = ProcessBuilder::new(&*self.gctx.rustdoc()?); let cmd = fill_rustc_tool_env(rustdoc, unit); - let mut cmd = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?; + let mut cmd = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, ToolKind::Rustdoc)?; cmd.retry_with_argfile(true); unit.target.edition().cmd_edition_arg(&mut cmd); @@ -214,7 +233,7 @@ impl<'gctx> Compilation<'gctx> { pkg, None, CompileKind::Host, - false, + ToolKind::HostProcess, ) } @@ -249,7 +268,8 @@ impl<'gctx> Compilation<'gctx> { } else { ProcessBuilder::new(cmd) }; - let mut builder = self.fill_env(builder, pkg, script_meta, kind, false)?; + let tool_kind = ToolKind::TargetProcess; + let mut builder = self.fill_env(builder, pkg, script_meta, kind, tool_kind)?; if let Some(client) = self.gctx.jobserver_from_env() { builder.inherit_jobserver(client); @@ -269,10 +289,10 @@ impl<'gctx> Compilation<'gctx> { pkg: &Package, script_meta: Option<Metadata>, kind: CompileKind, - is_rustc_tool: bool, + tool_kind: ToolKind, ) -> CargoResult<ProcessBuilder> { let mut search_path = Vec::new(); - if is_rustc_tool { + if tool_kind.is_rustc_tool() { search_path.push(self.deps_output[&CompileKind::Host].clone()); } else { search_path.extend(super::filter_dynamic_search_path( From 0e8a67fde3dc2c3bb81dd6d8b8493940ec8bea79 Mon Sep 17 00:00:00 2001 From: Weihang Lo <me@weihanglo.tw> Date: Sun, 25 Feb 2024 19:18:41 -0500 Subject: [PATCH 2/3] test(doctest): doctest doesn't set shared libs to search path This new test demonstrate the current behavior. --- tests/testsuite/test.rs | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index ae79e8df688..f20c707a435 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -7,6 +7,7 @@ use cargo_test_support::{ }; use cargo_test_support::{cross_compile, paths}; use cargo_test_support::{rustc_host, rustc_host_env, sleep_ms}; +use cargo_util::paths::dylib_path_envvar; use std::fs; #[cargo_test] @@ -2767,6 +2768,53 @@ fn only_test_docs() { .run(); } +#[cargo_test] +fn doctest_with_library_paths() { + let p = project(); + // Only link search directories within the target output directory are + // propagated through to dylib_path_envvar() (see #3366). + let dir1 = p.target_debug_dir().join("foo\\backslash"); + let dir2 = p.target_debug_dir().join("dir=containing=equal=signs"); + + let p = p + .file("Cargo.toml", &basic_manifest("foo", "0.0.0")) + .file( + "build.rs", + &format!( + r##" + fn main() {{ + println!(r#"cargo::rustc-link-search=native={}"#); + println!(r#"cargo::rustc-link-search={}"#); + }} + "##, + dir1.display(), + dir2.display() + ), + ) + .file( + "src/lib.rs", + &format!( + r##" + /// ``` + /// foo::assert_search_path(); + /// ``` + pub fn assert_search_path() {{ + let search_path = std::env::var_os("{}").unwrap(); + let paths = std::env::split_paths(&search_path).collect::<Vec<_>>(); + assert!(!paths.contains(&r#"{}"#.into())); + assert!(!paths.contains(&r#"{}"#.into())); + }} + "##, + dylib_path_envvar(), + dir1.display(), + dir2.display() + ), + ) + .build(); + + p.cargo("test --doc").run(); +} + #[cargo_test] fn test_panic_abort_with_dep() { let p = project() From 34215b1b4a5db54ff09526aa8888d51bb458421c Mon Sep 17 00:00:00 2001 From: Weihang Lo <me@weihanglo.tw> Date: Thu, 22 Feb 2024 10:31:48 -0500 Subject: [PATCH 3/3] fix: doctest searches native libs in build script outputs HACK: `rustdoc --test` not only compiles but executes doctests. Ideally only execution phase should have search paths appended, so the executions can find native libs just like other tests. However, there is no way to separate these two phase, so this hack is added for both phases. --- src/cargo/core/compiler/compilation.rs | 12 ++++++++++++ tests/testsuite/test.rs | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 0e768bfcb11..017c41b8afd 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -293,6 +293,18 @@ impl<'gctx> Compilation<'gctx> { ) -> CargoResult<ProcessBuilder> { let mut search_path = Vec::new(); if tool_kind.is_rustc_tool() { + if matches!(tool_kind, ToolKind::Rustdoc) { + // HACK: `rustdoc --test` not only compiles but executes doctests. + // Ideally only execution phase should have search paths appended, + // so the executions can find native libs just like other tests. + // However, there is no way to separate these two phase, so this + // hack is added for both phases. + // TODO: handle doctest-xcompile + search_path.extend(super::filter_dynamic_search_path( + self.native_dirs.iter(), + &self.root_output[&CompileKind::Host], + )); + } search_path.push(self.deps_output[&CompileKind::Host].clone()); } else { search_path.extend(super::filter_dynamic_search_path( diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index f20c707a435..44a830134f5 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -2801,8 +2801,8 @@ fn doctest_with_library_paths() { pub fn assert_search_path() {{ let search_path = std::env::var_os("{}").unwrap(); let paths = std::env::split_paths(&search_path).collect::<Vec<_>>(); - assert!(!paths.contains(&r#"{}"#.into())); - assert!(!paths.contains(&r#"{}"#.into())); + assert!(paths.contains(&r#"{}"#.into())); + assert!(paths.contains(&r#"{}"#.into())); }} "##, dylib_path_envvar(),