From a367956100e1a008906c0a9e251187f1d4203dde Mon Sep 17 00:00:00 2001 From: Dennis Duda Date: Wed, 27 May 2020 21:58:12 +0200 Subject: [PATCH 1/5] 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/tools/compiletest/src/runtest.rs | 22 ++++--- src/tools/remote-test-client/src/main.rs | 14 ++-- src/tools/remote-test-server/src/main.rs | 83 +++++++++++++++++------- 3 files changed, 83 insertions(+), 36 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 63fd052a5560d..a6995eb820a78 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1584,29 +1584,35 @@ 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", &prog]) + .arg(support_libs.len().to_string()) + .args(support_libs) + .args(args) + .envs(env.clone()); self.compose_and_run( test_client, self.config.run_lib_path.to_str().unwrap(), diff --git a/src/tools/remote-test-client/src/main.rs b/src/tools/remote-test-client/src/main.rs index 3379d82eda829..dea8bb933c9d9 100644 --- a/src/tools/remote-test-client/src/main.rs +++ b/src/tools/remote-test-client/src/main.rs @@ -44,7 +44,11 @@ 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().unwrap(), + args.next().and_then(|count| count.parse().ok()).unwrap(), + args.collect(), + ), "help" | "-h" | "--help" => help(), cmd => { println!("unknown command: {}", cmd); @@ -197,12 +201,14 @@ fn push(path: &Path) { println!("done pushing {:?}", path); } -fn run(files: String, args: Vec) { +fn run(exe: String, support_lib_count: usize, all_args: Vec) { 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 +233,7 @@ fn run(files: String, args: Vec) { 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])); diff --git a/src/tools/remote-test-server/src/main.rs b/src/tools/remote-test-server/src/main.rs index 826e3d05111ae..d9f65cf502da6 100644 --- a/src/tools/remote-test-server/src/main.rs +++ b/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!(windows) { + env::var_os("RUST_TEMP").expect("Set RUST_TEMP to your preferred temp folder").into() + } else if cfg!(target_os = "android") { + "/data/tmp/work".into() } else { - (t!(TcpListener::bind(bind_addr)), "/tmp/work") + "/tmp/work".into() }; 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(dir: &Path, io: &mut B) -> PathBuf { let mut filename = Vec::new(); t!(io.read_until(0, &mut filename)); @@ -253,10 +283,17 @@ fn recv(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(&dst, Permissions::from_mode(0o755))); +} +#[cfg(windows)] +fn set_permissions(_path: &Path) {} + fn my_copy(src: &mut dyn Read, which: u8, dst: &Mutex) { let mut b = [0; 1024]; loop { From 41d540fcb72ccef0e1c20f145288993a41260e25 Mon Sep 17 00:00:00 2001 From: Dennis Duda Date: Thu, 28 May 2020 10:04:43 +0200 Subject: [PATCH 2/5] Unify temp path generation for non-android --- src/tools/remote-test-server/src/main.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/remote-test-server/src/main.rs b/src/tools/remote-test-server/src/main.rs index d9f65cf502da6..9112985e140a7 100644 --- a/src/tools/remote-test-server/src/main.rs +++ b/src/tools/remote-test-server/src/main.rs @@ -83,12 +83,12 @@ fn main() { }; let listener = t!(TcpListener::bind(bind_addr)); - let work: PathBuf = if cfg!(windows) { - env::var_os("RUST_TEMP").expect("Set RUST_TEMP to your preferred temp folder").into() - } else if cfg!(target_os = "android") { + let work: PathBuf = if cfg!(target_os = "android") { "/data/tmp/work".into() } else { - "/tmp/work".into() + let mut temp_dir = env::temp_dir(); + temp_dir.push("work"); + temp_dir }; println!("listening!"); From 577ac2f836f860646ca4f47a5effcfe6f024ec39 Mon Sep 17 00:00:00 2001 From: Dennis Duda Date: Fri, 29 May 2020 11:57:23 +0200 Subject: [PATCH 3/5] Fix `set_permissions` call for non-windows --- src/tools/remote-test-server/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/remote-test-server/src/main.rs b/src/tools/remote-test-server/src/main.rs index 9112985e140a7..e7eff35e55725 100644 --- a/src/tools/remote-test-server/src/main.rs +++ b/src/tools/remote-test-server/src/main.rs @@ -289,7 +289,7 @@ fn recv(dir: &Path, io: &mut B) -> PathBuf { #[cfg(not(windows))] fn set_permissions(path: &Path) { - t!(fs::set_permissions(&dst, Permissions::from_mode(0o755))); + t!(fs::set_permissions(&path, Permissions::from_mode(0o755))); } #[cfg(windows)] fn set_permissions(_path: &Path) {} From 0199fdc0f72ec6942d93d0bf23c7fa1fb9fbf54f Mon Sep 17 00:00:00 2001 From: Dennis Duda Date: Sun, 31 May 2020 15:20:52 +0200 Subject: [PATCH 4/5] Update help text of remote-test-client to reflect changed command --- src/tools/remote-test-client/src/main.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tools/remote-test-client/src/main.rs b/src/tools/remote-test-client/src/main.rs index dea8bb933c9d9..88eaaac287b81 100644 --- a/src/tools/remote-test-client/src/main.rs +++ b/src/tools/remote-test-client/src/main.rs @@ -306,7 +306,8 @@ Usage: {0} [] Sub-commands: spawn-emulator [rootfs] See below push Copy to emulator - run [args...] Run program on emulator + run [support_libs...] [args...] + Run program on emulator help Display help message Spawning an emulator: @@ -325,8 +326,8 @@ specified. The file at 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 is pushed to the target. Finally, the first -file in is executed in the emulator, preserving the current environment. +the and any specified support libs are pushed to the target. Finally, the + is executed in the emulator, preserving the current environment. That command's status code is returned. ", env::args().next().unwrap(), From 036da3a6dcc084db90dbe6ea2831eb7332a1c535 Mon Sep 17 00:00:00 2001 From: Dennis Duda Date: Sun, 31 May 2020 17:36:17 +0200 Subject: [PATCH 5/5] Make `remote-test-client` work as cargo runner again Since cargo appends executable/args, the support_lib count parameter has to come first. --- src/bootstrap/test.rs | 2 +- src/tools/compiletest/src/runtest.rs | 3 +-- src/tools/remote-test-client/src/main.rs | 8 +++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index f1305e2540b4c..3ae6c34d22841 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1762,7 +1762,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()), ); } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a6995eb820a78..4f8cf92b86938 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1608,8 +1608,7 @@ impl<'test> TestCx<'test> { let mut test_client = Command::new(self.config.remote_test_client.as_ref().unwrap()); test_client - .args(&["run", &prog]) - .arg(support_libs.len().to_string()) + .args(&["run", &support_libs.len().to_string(), &prog]) .args(support_libs) .args(args) .envs(env.clone()); diff --git a/src/tools/remote-test-client/src/main.rs b/src/tools/remote-test-client/src/main.rs index 88eaaac287b81..259477e9a1c36 100644 --- a/src/tools/remote-test-client/src/main.rs +++ b/src/tools/remote-test-client/src/main.rs @@ -45,8 +45,10 @@ fn main() { ), "push" => push(Path::new(&args.next().unwrap())), "run" => run( - args.next().unwrap(), 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(), @@ -201,7 +203,7 @@ fn push(path: &Path) { println!("done pushing {:?}", path); } -fn run(exe: String, support_lib_count: usize, all_args: Vec) { +fn run(support_lib_count: usize, exe: String, all_args: Vec) { 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); @@ -306,7 +308,7 @@ Usage: {0} [] Sub-commands: spawn-emulator [rootfs] See below push Copy to emulator - run [support_libs...] [args...] + run [support_libs...] [args...] Run program on emulator help Display help message