Auto merge of #9566 - ehuss:relative-rustc-path, r=alexcrichton

Fix rustc/rustdoc config values to be config-relative.

The `rustc`, `rustdoc`, `rustc_wrapper`, and `rustc_workspace_wrapper` config values (in the `[build]` table) were being interpreted literally. This caused a problem if you used a relative path like `foo/rustc`.  This would be interpreted as a relative path from whatever cwd cargo launches rustc from, which changes for different scenarios, making it essentially unusuable (since crates.io dependencies wouldn't be buildable).

Additionally, due to rust-lang/rust#37868, it is a bad idea to use relative paths.

This changes it so that those paths are config-relative.  Bare names (like "my-rustc-program") still use PATH as before.

This also includes a commit to centralize the rustc-wrapper program used by several tests so that it isn't built multiple times (and to allow several tests to work on windows).

Fixes #8202

crates/cargo-test-support/src/lib.rs

@@ -39,6 +39,7 @@ pub mod git;
 pub mod paths;
 pub mod publish;
 pub mod registry;
+pub mod tools;
 
 /*
  *

crates/cargo-test-support/src/paths.rs

@@ -1,4 +1,3 @@
-use crate::{basic_manifest, project};
 use filetime::{self, FileTime};
 use lazy_static::lazy_static;
 use std::cell::RefCell;
@@ -296,24 +295,3 @@ pub fn sysroot() -> String {
     let sysroot = String::from_utf8(output.stdout).unwrap();
     sysroot.trim().to_string()
 }
-
-pub fn echo_wrapper() -> std::path::PathBuf {
-    let p = project()
-        .at("rustc-echo-wrapper")
-        .file("Cargo.toml", &basic_manifest("rustc-echo-wrapper", "1.0.0"))
-        .file(
-            "src/main.rs",
-            r#"
-            fn main() {
-                let args = std::env::args().collect::<Vec<_>>();
-                eprintln!("WRAPPER CALLED: {}", args[1..].join(" "));
-                let status = std::process::Command::new(&args[1])
-                    .args(&args[2..]).status().unwrap();
-                std::process::exit(status.code().unwrap_or(1));
-            }
-            "#,
-        )
-        .build();
-    p.cargo("build").run();
-    p.bin("rustc-echo-wrapper")
-}

crates/cargo-test-support/src/tools.rs

@@ -0,0 +1,40 @@
+//! Common executables that can be reused by various tests.
+
+use crate::{basic_manifest, paths, project};
+use lazy_static::lazy_static;
+use std::path::PathBuf;
+use std::sync::Mutex;
+
+lazy_static! {
+    static ref ECHO_WRAPPER: Mutex<Option<PathBuf>> = Mutex::new(None);
+}
+
+/// Returns the path to an executable that works as a wrapper around rustc.
+///
+/// The wrapper will echo the command line it was called with to stderr.
+pub fn echo_wrapper() -> PathBuf {
+    let mut lock = ECHO_WRAPPER.lock().unwrap();
+    if let Some(path) = &*lock {
+        return path.clone();
+    }
+    let p = project()
+        .at(paths::global_root().join("rustc-echo-wrapper"))
+        .file("Cargo.toml", &basic_manifest("rustc-echo-wrapper", "1.0.0"))
+        .file(
+            "src/main.rs",
+            r#"
+            fn main() {
+                let args = std::env::args().collect::<Vec<_>>();
+                eprintln!("WRAPPER CALLED: {}", args[1..].join(" "));
+                let status = std::process::Command::new(&args[1])
+                    .args(&args[2..]).status().unwrap();
+                std::process::exit(status.code().unwrap_or(1));
+            }
+            "#,
+        )
+        .build();
+    p.cargo("build").run();
+    let path = p.bin("rustc-echo-wrapper");
+    *lock = Some(path.clone());
+    path
+}

src/cargo/util/config/mod.rs

@@ -732,7 +732,7 @@ impl Config {
         })
     }
 
-    fn string_to_path(&self, value: String, definition: &Definition) -> PathBuf {
+    fn string_to_path(&self, value: &str, definition: &Definition) -> PathBuf {
         let is_path = value.contains('/') || (cfg!(windows) && value.contains('\\'));
         if is_path {
             definition.root(self).join(value)
@@ -1391,7 +1391,11 @@ impl Config {
 
     /// Looks for a path for `tool` in an environment variable or the given config, and returns
     /// `None` if it's not present.
-    fn maybe_get_tool(&self, tool: &str, from_config: &Option<PathBuf>) -> Option<PathBuf> {
+    fn maybe_get_tool(
+        &self,
+        tool: &str,
+        from_config: &Option<ConfigRelativePath>,
+    ) -> Option<PathBuf> {
         let var = tool.to_uppercase();
 
         match env::var_os(&var) {
@@ -1408,13 +1412,13 @@ impl Config {
                 Some(path)
             }
 
-            None => from_config.clone(),
+            None => from_config.as_ref().map(|p| p.resolve_program(self)),
         }
     }
 
     /// Looks for a path for `tool` in an environment variable or config path, defaulting to `tool`
     /// as a path.
-    fn get_tool(&self, tool: &str, from_config: &Option<PathBuf>) -> PathBuf {
+    fn get_tool(&self, tool: &str, from_config: &Option<ConfigRelativePath>) -> PathBuf {
         self.maybe_get_tool(tool, from_config)
             .unwrap_or_else(|| PathBuf::from(tool))
     }
@@ -2084,10 +2088,10 @@ pub struct CargoBuildConfig {
     pub jobs: Option<u32>,
     pub rustflags: Option<StringList>,
     pub rustdocflags: Option<StringList>,
-    pub rustc_wrapper: Option<PathBuf>,
-    pub rustc_workspace_wrapper: Option<PathBuf>,
-    pub rustc: Option<PathBuf>,
-    pub rustdoc: Option<PathBuf>,
+    pub rustc_wrapper: Option<ConfigRelativePath>,
+    pub rustc_workspace_wrapper: Option<ConfigRelativePath>,
+    pub rustc: Option<ConfigRelativePath>,
+    pub rustdoc: Option<ConfigRelativePath>,
     pub out_dir: Option<ConfigRelativePath>,
 }
 

src/cargo/util/config/path.rs

@@ -34,8 +34,8 @@ impl ConfigRelativePath {
     /// Values which don't look like a filesystem path (don't contain `/` or
     /// `\`) will be returned as-is, and everything else will fall through to an
     /// absolute path.
-    pub fn resolve_program(self, config: &Config) -> PathBuf {
-        config.string_to_path(self.0.val, &self.0.definition)
+    pub fn resolve_program(&self, config: &Config) -> PathBuf {
+        config.string_to_path(&self.0.val, &self.0.definition)
     }
 }
 

tests/testsuite/build.rs

@@ -8,6 +8,7 @@ use cargo::{
 };
 use cargo_test_support::paths::{root, CargoPathExt};
 use cargo_test_support::registry::Package;
+use cargo_test_support::tools;
 use cargo_test_support::{
     basic_bin_manifest, basic_lib_manifest, basic_manifest, cargo_exe, git, is_nightly,
     lines_match_unordered, main_file, paths, process, project, rustc_host, sleep_ms,
@@ -4054,64 +4055,77 @@ fn run_proper_binary_main_rs_as_foo() {
 }
 
 #[cargo_test]
-// NOTE: we don't have `/usr/bin/env` on Windows.
-#[cfg(not(windows))]
 fn rustc_wrapper() {
     let p = project().file("src/lib.rs", "").build();
+    let wrapper = tools::echo_wrapper();
+    let running = format!(
+        "[RUNNING] `{} rustc --crate-name foo [..]",
+        wrapper.display()
+    );
     p.cargo("build -v")
-        .env("RUSTC_WRAPPER", "/usr/bin/env")
-        .with_stderr_contains("[RUNNING] `/usr/bin/env rustc --crate-name foo [..]")
+        .env("RUSTC_WRAPPER", &wrapper)
+        .with_stderr_contains(&running)
         .run();
-}
-
-#[cargo_test]
-#[cfg(not(windows))]
-fn rustc_wrapper_relative() {
-    let p = project().file("src/lib.rs", "").build();
+    p.build_dir().rm_rf();
     p.cargo("build -v")
-        .env("RUSTC_WRAPPER", "./sccache")
-        .with_status(101)
-        .with_stderr_contains("[..]/foo/./sccache rustc[..]")
+        .env("RUSTC_WORKSPACE_WRAPPER", &wrapper)
+        .with_stderr_contains(&running)
         .run();
 }
 
 #[cargo_test]
-#[cfg(not(windows))]
-fn rustc_wrapper_from_path() {
-    let p = project().file("src/lib.rs", "").build();
+fn rustc_wrapper_relative() {
+    Package::new("bar", "1.0.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                bar = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    let wrapper = tools::echo_wrapper();
+    let exe_name = wrapper.file_name().unwrap().to_str().unwrap();
+    let relative_path = format!("./{}", exe_name);
+    fs::hard_link(&wrapper, p.root().join(exe_name)).unwrap();
+    let running = format!("[RUNNING] `[ROOT]/foo/./{} rustc[..]", exe_name);
     p.cargo("build -v")
-        .env("RUSTC_WRAPPER", "wannabe_sccache")
-        .with_status(101)
-        .with_stderr_contains("[..]`wannabe_sccache rustc [..]")
+        .env("RUSTC_WRAPPER", &relative_path)
+        .with_stderr_contains(&running)
         .run();
-}
-
-#[cargo_test]
-// NOTE: we don't have `/usr/bin/env` on Windows.
-#[cfg(not(windows))]
-fn rustc_workspace_wrapper() {
-    let p = project().file("src/lib.rs", "").build();
+    p.build_dir().rm_rf();
     p.cargo("build -v")
-        .env("RUSTC_WORKSPACE_WRAPPER", "/usr/bin/env")
-        .with_stderr_contains("[RUNNING] `/usr/bin/env rustc --crate-name foo [..]")
+        .env("RUSTC_WORKSPACE_WRAPPER", &relative_path)
+        .with_stderr_contains(&running)
         .run();
+    p.build_dir().rm_rf();
+    p.change_file(
+        ".cargo/config.toml",
+        &format!(
+            r#"
+                build.rustc-wrapper = "./{}"
+            "#,
+            exe_name
+        ),
+    );
+    p.cargo("build -v").with_stderr_contains(&running).run();
 }
 
 #[cargo_test]
-#[cfg(not(windows))]
-fn rustc_workspace_wrapper_relative() {
+fn rustc_wrapper_from_path() {
     let p = project().file("src/lib.rs", "").build();
     p.cargo("build -v")
-        .env("RUSTC_WORKSPACE_WRAPPER", "./sccache")
+        .env("RUSTC_WRAPPER", "wannabe_sccache")
         .with_status(101)
-        .with_stderr_contains("[..]/foo/./sccache rustc[..]")
+        .with_stderr_contains("[..]`wannabe_sccache rustc [..]")
         .run();
-}
-
-#[cargo_test]
-#[cfg(not(windows))]
-fn rustc_workspace_wrapper_from_path() {
-    let p = project().file("src/lib.rs", "").build();
+    p.build_dir().rm_rf();
     p.cargo("build -v")
         .env("RUSTC_WORKSPACE_WRAPPER", "wannabe_sccache")
         .with_status(101)

tests/testsuite/cache_messages.rs

@@ -1,5 +1,6 @@
 //! Tests for caching compiler diagnostics.
 
+use cargo_test_support::tools;
 use cargo_test_support::{
     basic_manifest, is_coarse_mtime, process, project, registry::Package, sleep_ms,
 };
@@ -459,8 +460,6 @@ fn caching_large_output() {
 
 #[cargo_test]
 fn rustc_workspace_wrapper() {
-    use cargo_test_support::paths;
-
     let p = project()
         .file(
             "src/lib.rs",
@@ -470,7 +469,7 @@ fn rustc_workspace_wrapper() {
         .build();
 
     p.cargo("check -v")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name foo src/lib.rs [..]")
         .run();
 
@@ -488,7 +487,7 @@ fn rustc_workspace_wrapper() {
 
     // Again, reading from the cache.
     p.cargo("check -v")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("[FRESH] foo [..]")
         .with_stdout_does_not_contain("WRAPPER CALLED: rustc --crate-name foo src/lib.rs [..]")
         .run();

tests/testsuite/check.rs

@@ -5,6 +5,7 @@ use std::fmt::{self, Write};
 use cargo_test_support::install::exe;
 use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::registry::Package;
+use cargo_test_support::tools;
 use cargo_test_support::{basic_manifest, project};
 
 #[cargo_test]
@@ -887,7 +888,6 @@ fn error_from_deep_recursion() -> Result<(), fmt::Error> {
 
 #[cargo_test]
 fn rustc_workspace_wrapper_affects_all_workspace_members() {
-    use cargo_test_support::paths;
     let p = project()
         .file(
             "Cargo.toml",
@@ -903,15 +903,14 @@ fn rustc_workspace_wrapper_affects_all_workspace_members() {
         .build();
 
     p.cargo("check")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name bar [..]")
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name baz [..]")
         .run();
 }
 
 #[cargo_test]
 fn rustc_workspace_wrapper_includes_path_deps() {
-    use cargo_test_support::paths;
     let p = project()
         .file(
             "Cargo.toml",
@@ -936,7 +935,7 @@ fn rustc_workspace_wrapper_includes_path_deps() {
         .build();
 
     p.cargo("check --workspace")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name foo [..]")
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name bar [..]")
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name baz [..]")
@@ -945,7 +944,6 @@ fn rustc_workspace_wrapper_includes_path_deps() {
 
 #[cargo_test]
 fn rustc_workspace_wrapper_respects_primary_units() {
-    use cargo_test_support::paths;
     let p = project()
         .file(
             "Cargo.toml",
@@ -961,15 +959,14 @@ fn rustc_workspace_wrapper_respects_primary_units() {
         .build();
 
     p.cargo("check -p bar")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name bar [..]")
         .with_stdout_does_not_contain("WRAPPER CALLED: rustc --crate-name baz [..]")
         .run();
 }
 
 #[cargo_test]
 fn rustc_workspace_wrapper_excludes_published_deps() {
-    use cargo_test_support::paths;
     let p = project()
         .file(
             "Cargo.toml",
@@ -994,7 +991,7 @@ fn rustc_workspace_wrapper_excludes_published_deps() {
     Package::new("baz", "1.0.0").publish();
 
     p.cargo("check --workspace -v")
-        .env("RUSTC_WORKSPACE_WRAPPER", paths::echo_wrapper())
+        .env("RUSTC_WORKSPACE_WRAPPER", tools::echo_wrapper())
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name foo [..]")
         .with_stderr_contains("WRAPPER CALLED: rustc --crate-name bar [..]")
         .with_stderr_contains("[CHECKING] baz [..]")