buck2: make interaction of cwd & relative exe consistent between UNIX…

… & Windows

Summary:
If you do `Command::new("a").current_dir("b")` with your cwd being `c`, on UNIX
you run `c/b/a`, on Windows you run `c/a`.

This is a bit inconsistent, and it means that running Buck2 with a cwd that
is not the project root does not work on Windows.

This diff fixes that by making the behavior consistent on both platforms. When
we run a command, we now check if the executable does exist relative to the
cwd, and if it does, we use it. We know we always provide a cwd and it's always
absolute, so this is fairly robust.

Reviewed By: KapJI

Differential Revision: D46440477

fbshipit-source-id: f2c3bf6877de02fe5e783d3a434e417aa6f46f1f

app/buck2_execute_impl/src/executors/local.rs

@@ -60,6 +60,7 @@ use buck2_execute::materialize::materializer::MaterializationError;
 use buck2_execute::materialize::materializer::Materializer;
 use buck2_forkserver::client::ForkserverClient;
 use buck2_forkserver::run::gather_output;
+use buck2_forkserver::run::maybe_absolutize_exe;
 use buck2_forkserver::run::timeout_into_cancellation;
 use buck2_forkserver::run::GatherOutputStatus;
 use buck2_util::process::background_command;
@@ -174,7 +175,8 @@ impl LocalExecutor {
                 }
 
                 None => {
-                    let mut cmd = background_command(exe);
+                    let exe = maybe_absolutize_exe(exe, &working_directory)?;
+                    let mut cmd = background_command(exe.as_ref());
                     cmd.current_dir(working_directory.as_path());
                     cmd.args(args);
                     apply_local_execution_environment(

app/buck2_execute_impl/src/executors/worker.rs

@@ -24,6 +24,7 @@ use buck2_core::fs::paths::file_name::FileName;
 use buck2_events::dispatch::get_dispatcher_opt;
 use buck2_execute::execute::request::WorkerId;
 use buck2_execute::execute::request::WorkerSpec;
+use buck2_forkserver::run::maybe_absolutize_exe;
 use buck2_forkserver::run::prepare_command;
 use buck2_forkserver::run::GatherOutputStatus;
 use buck2_util::process::background_command;
@@ -46,7 +47,8 @@ async fn exec_spawn(
     stderr_path: &AbsNormPathBuf,
 ) -> anyhow::Result<Child> {
     // TODO(ctolliday) spawn using forkserver T153604128
-    let mut cmd = background_command(exe);
+    let exe = maybe_absolutize_exe(exe, root)?;
+    let mut cmd = background_command(exe.as_ref());
     cmd.current_dir(root);
     cmd.args(args);
     apply_local_execution_environment(&mut cmd, root, env, None);

app/buck2_forkserver/src/run/mod.rs

@@ -10,7 +10,9 @@
 mod interruptible_async_read;
 pub mod status_decoder;
 
+use std::borrow::Cow;
 use std::io;
+use std::path::Path;
 use std::pin::Pin;
 use std::process::Command;
 use std::process::ExitStatus;
@@ -20,6 +22,8 @@ use std::task::Poll;
 use std::time::Duration;
 
 use anyhow::Context as _;
+use buck2_core::fs::fs_util;
+use buck2_core::fs::paths::abs_path::AbsPath;
 use bytes::Bytes;
 use futures::future::Future;
 use futures::future::FutureExt;
@@ -337,6 +341,26 @@ fn kill_process_impl(pid: u32) -> anyhow::Result<()> {
         .with_context(|| format!("Failed to kill process {}", pid))
 }
 
+/// Unify the the behavior of using a relative path for the executable between Unix and Windows. On
+/// UNIX, the path is understood to be relative to the cwd of the *spawned process*, whereas on
+/// Windows, it's relative ot the cwd of the *spawning* process.
+///
+/// Here, we unify the two behaviors since we always run our subprocesses with a known cwd: we
+/// check if the executable actually exists relative to said cwd, and if it does, we use that.
+pub fn maybe_absolutize_exe<'a>(
+    exe: &'a (impl AsRef<Path> + ?Sized),
+    spawned_process_cwd: &'_ AbsPath,
+) -> anyhow::Result<Cow<'a, Path>> {
+    let exe = exe.as_ref();
+
+    let abs = spawned_process_cwd.join(exe);
+    if fs_util::try_exists(&abs).context("Error absolute-izing executable")? {
+        return Ok(abs.into_path_buf().into());
+    }
+
+    Ok(exe.into())
+}
+
 pub fn prepare_command(mut cmd: Command) -> tokio::process::Command {
     #[cfg(unix)]
     {

app/buck2_forkserver/src/unix/service.rs

@@ -42,6 +42,7 @@ use tonic::Status;
 use tonic::Streaming;
 
 use crate::convert::encode_event_stream;
+use crate::run::maybe_absolutize_exe;
 use crate::run::prepare_command;
 use crate::run::status_decoder::DefaultStatusDecoder;
 use crate::run::status_decoder::MiniperfStatusDecoder;
@@ -122,15 +123,17 @@ impl Forkserver for UnixForkserverService {
                 .transpose()
                 .context("Invalid timeout")?;
 
+            let exe = maybe_absolutize_exe(exe, cwd)?;
+
             let (mut cmd, miniperf_output) = match (enable_miniperf, &self.miniperf) {
                 (true, Some(miniperf)) => {
                     let mut cmd = background_command(miniperf.miniperf.as_path());
                     let output_path = miniperf.allocate_output_path();
                     cmd.arg(output_path.as_path());
-                    cmd.arg(exe);
+                    cmd.arg(exe.as_ref());
                     (cmd, Some(output_path))
                 }
-                _ => (background_command(exe), None),
+                _ => (background_command(exe.as_ref()), None),
             };
 
             cmd.current_dir(cwd);