Auto merge of #72672 - seritools:remote-test-windows, r=Mark-Simulacrum

Make remote-test-client and remote-test-server compatible with windows

`compiletest` and `remote-test-client`:

The command line for `remote-test-client` was changed slightly to allow cross-platform compatible paths. The old way of supplying the support libs was by joining their paths with the executable path
with `:`. This caused Windows-style paths to be split after the directory letter. Now, the number of support libs is provided as a parameter as well, and the support lib paths are split off from the regular args in the client.

`remote-test-server`:

- Marked Unix-only parts as such and implemented Windows alternatives
- On Windows `LD_LIBRARY_PATH` doesn't exist. Libraries are loaded from `PATH` though, so that's the way around it.
- Tiny cleanup: `Command::args`/`envs` instead of manually looping over them
- The temp path for Windows has to be set via environment variable, since there isn't a global temp directory that would work on every machine (as a static string)

src/bootstrap/test.rs

@@ -1769,7 +1769,7 @@ impl Step for Crate {
         } else if builder.remote_tested(target) {
             cargo.env(
                 format!("CARGO_TARGET_{}_RUNNER", envify(&target)),
-                format!("{} run", builder.tool_exe(Tool::RemoteTestClient).display()),
+                format!("{} run 0", builder.tool_exe(Tool::RemoteTestClient).display()),
             );
         }
 

src/tools/compiletest/src/runtest.rs

@@ -1584,29 +1584,34 @@ impl<'test> TestCx<'test> {
             //
             // into
             //
-            //      remote-test-client run program:support-lib.so arg1 arg2
+            //      remote-test-client run program 2 support-lib.so support-lib2.so arg1 arg2
             //
             // The test-client program will upload `program` to the emulator
             // along with all other support libraries listed (in this case
-            // `support-lib.so`. It will then execute the program on the
-            // emulator with the arguments specified (in the environment we give
-            // the process) and then report back the same result.
+            // `support-lib.so` and `support-lib2.so`. It will then execute
+            // the program on the emulator with the arguments specified
+            // (in the environment we give the process) and then report back
+            // the same result.
             _ if self.config.remote_test_client.is_some() => {
                 let aux_dir = self.aux_output_dir_name();
-                let ProcArgs { mut prog, args } = self.make_run_args();
+                let ProcArgs { prog, args } = self.make_run_args();
+                let mut support_libs = Vec::new();
                 if let Ok(entries) = aux_dir.read_dir() {
                     for entry in entries {
                         let entry = entry.unwrap();
                         if !entry.path().is_file() {
                             continue;
                         }
-                        prog.push_str(":");
-                        prog.push_str(entry.path().to_str().unwrap());
+                        support_libs.push(entry.path());
                     }
                 }
                 let mut test_client =
                     Command::new(self.config.remote_test_client.as_ref().unwrap());
-                test_client.args(&["run", &prog]).args(args).envs(env.clone());
+                test_client
+                    .args(&["run", &support_libs.len().to_string(), &prog])
+                    .args(support_libs)
+                    .args(args)
+                    .envs(env.clone());
                 self.compose_and_run(
                     test_client,
                     self.config.run_lib_path.to_str().unwrap(),

src/tools/remote-test-client/src/main.rs

@@ -44,7 +44,13 @@ fn main() {
             args.next().map(|s| s.into()),
         ),
         "push" => push(Path::new(&args.next().unwrap())),
-        "run" => run(args.next().unwrap(), args.collect()),
+        "run" => run(
+            args.next().and_then(|count| count.parse().ok()).unwrap(),
+            // the last required parameter must remain the executable
+            // path so that the client works as a cargo runner
+            args.next().unwrap(),
+            args.collect(),
+        ),
         "help" | "-h" | "--help" => help(),
         cmd => {
             println!("unknown command: {}", cmd);
@@ -197,12 +203,14 @@ fn push(path: &Path) {
     println!("done pushing {:?}", path);
 }
 
-fn run(files: String, args: Vec<String>) {
+fn run(support_lib_count: usize, exe: String, all_args: Vec<String>) {
     let device_address = env::var(REMOTE_ADDR_ENV).unwrap_or(DEFAULT_ADDR.to_string());
     let client = t!(TcpStream::connect(device_address));
     let mut client = BufWriter::new(client);
     t!(client.write_all(b"run "));
 
+    let (support_libs, args) = all_args.split_at(support_lib_count);
+
     // Send over the args
     for arg in args {
         t!(client.write_all(arg.as_bytes()));
@@ -227,9 +235,7 @@ fn run(files: String, args: Vec<String>) {
     t!(client.write_all(&[0]));
 
     // Send over support libraries
-    let mut files = files.split(':');
-    let exe = files.next().unwrap();
-    for file in files.map(Path::new) {
+    for file in support_libs.iter().map(Path::new) {
         send(&file, &mut client);
     }
     t!(client.write_all(&[0]));
@@ -302,7 +308,8 @@ Usage: {0} <command> [<args>]
 Sub-commands:
     spawn-emulator <target> <server> <tmpdir> [rootfs]   See below
     push <path>                                          Copy <path> to emulator
-    run <files> [args...]                                Run program on emulator
+    run <support_lib_count> <file> [support_libs...] [args...]
+                                                         Run program on emulator
     help                                                 Display help message
 
 Spawning an emulator:
@@ -321,8 +328,8 @@ specified. The file at <path> is sent to this target.
 Executing commands on a running emulator:
 
 First the target emulator/adb session is connected to as for pushing files. Next
-the colon separated list of <files> is pushed to the target. Finally, the first
-file in <files> is executed in the emulator, preserving the current environment.
+the <file> and any specified support libs are pushed to the target. Finally, the
+<file> is executed in the emulator, preserving the current environment.
 That command's status code is returned.
 ",
         env::args().next().unwrap(),

src/tools/remote-test-server/src/main.rs

@@ -12,15 +12,19 @@
 
 #![deny(warnings)]
 
+#[cfg(not(windows))]
+use std::fs::Permissions;
+#[cfg(not(windows))]
+use std::os::unix::prelude::*;
+
 use std::cmp;
 use std::env;
-use std::fs::{self, File, Permissions};
+use std::fs::{self, File};
 use std::io::prelude::*;
 use std::io::{self, BufReader};
 use std::net::{TcpListener, TcpStream};
-use std::os::unix::prelude::*;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
+use std::process::{Command, ExitStatus, Stdio};
 use std::str;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};
@@ -72,21 +76,23 @@ fn main() {
 
     let config = Config::parse_args();
 
-    let bind_addr = if cfg!(target_os = "android") || config.remote {
+    let bind_addr = if cfg!(target_os = "android") || cfg!(windows) || config.remote {
         "0.0.0.0:12345"
     } else {
         "10.0.2.15:12345"
     };
 
-    let (listener, work) = if cfg!(target_os = "android") {
-        (t!(TcpListener::bind(bind_addr)), "/data/tmp/work")
+    let listener = t!(TcpListener::bind(bind_addr));
+    let work: PathBuf = if cfg!(target_os = "android") {
+        "/data/tmp/work".into()
     } else {
-        (t!(TcpListener::bind(bind_addr)), "/tmp/work")
+        let mut temp_dir = env::temp_dir();
+        temp_dir.push("work");
+        temp_dir
     };
     println!("listening!");
 
-    let work = Path::new(work);
-    t!(fs::create_dir_all(work));
+    t!(fs::create_dir_all(&work));
 
     let lock = Arc::new(Mutex::new(()));
 
@@ -99,10 +105,11 @@ fn main() {
         if &buf[..] == b"ping" {
             t!(socket.write_all(b"pong"));
         } else if &buf[..] == b"push" {
-            handle_push(socket, work);
+            handle_push(socket, &work);
         } else if &buf[..] == b"run " {
             let lock = lock.clone();
-            thread::spawn(move || handle_run(socket, work, &lock));
+            let work = work.clone();
+            thread::spawn(move || handle_run(socket, &work, &lock));
         } else {
             panic!("unknown command {:?}", buf);
         }
@@ -196,17 +203,28 @@ fn handle_run(socket: TcpStream, work: &Path, lock: &Mutex<()>) {
     let exe = recv(&path, &mut reader);
 
     let mut cmd = Command::new(&exe);
-    for arg in args {
-        cmd.arg(arg);
-    }
-    for (k, v) in env {
-        cmd.env(k, v);
-    }
+    cmd.args(args);
+    cmd.envs(env);
 
     // Support libraries were uploaded to `work` earlier, so make sure that's
     // in `LD_LIBRARY_PATH`. Also include our own current dir which may have
     // had some libs uploaded.
-    cmd.env("LD_LIBRARY_PATH", format!("{}:{}", work.display(), path.display()));
+    if cfg!(windows) {
+        // On windows, libraries are just searched in the executable directory,
+        // system directories, PWD, and PATH, in that order. PATH is the only one
+        // we can change for this.
+        cmd.env(
+            "PATH",
+            env::join_paths(
+                std::iter::once(work.to_owned())
+                    .chain(std::iter::once(path.clone()))
+                    .chain(env::split_paths(&env::var_os("PATH").unwrap())),
+            )
+            .unwrap(),
+        );
+    } else {
+        cmd.env("LD_LIBRARY_PATH", format!("{}:{}", work.display(), path.display()));
+    }
 
     // Spawn the child and ferry over stdout/stderr to the socket in a framed
     // fashion (poor man's style)
@@ -223,10 +241,9 @@ fn handle_run(socket: TcpStream, work: &Path, lock: &Mutex<()>) {
 
     // Finally send over the exit status.
     let status = t!(child.wait());
-    let (which, code) = match status.code() {
-        Some(n) => (0, n),
-        None => (1, status.signal().unwrap()),
-    };
+
+    let (which, code) = get_status_code(&status);
+
     t!(socket.lock().unwrap().write_all(&[
         which,
         (code >> 24) as u8,
@@ -236,6 +253,19 @@ fn handle_run(socket: TcpStream, work: &Path, lock: &Mutex<()>) {
     ]));
 }
 
+#[cfg(not(windows))]
+fn get_status_code(status: &ExitStatus) -> (u8, i32) {
+    match status.code() {
+        Some(n) => (0, n),
+        None => (1, status.signal().unwrap()),
+    }
+}
+
+#[cfg(windows)]
+fn get_status_code(status: &ExitStatus) -> (u8, i32) {
+    (0, status.code().unwrap())
+}
+
 fn recv<B: BufRead>(dir: &Path, io: &mut B) -> PathBuf {
     let mut filename = Vec::new();
     t!(io.read_until(0, &mut filename));
@@ -253,10 +283,17 @@ fn recv<B: BufRead>(dir: &Path, io: &mut B) -> PathBuf {
     let dst = dir.join(t!(str::from_utf8(&filename[..len])));
     let amt = read_u32(io) as u64;
     t!(io::copy(&mut io.take(amt), &mut t!(File::create(&dst))));
-    t!(fs::set_permissions(&dst, Permissions::from_mode(0o755)));
+    set_permissions(&dst);
     dst
 }
 
+#[cfg(not(windows))]
+fn set_permissions(path: &Path) {
+    t!(fs::set_permissions(&path, Permissions::from_mode(0o755)));
+}
+#[cfg(windows)]
+fn set_permissions(_path: &Path) {}
+
 fn my_copy(src: &mut dyn Read, which: u8, dst: &Mutex<dyn Write>) {
     let mut b = [0; 1024];
     loop {