From 6601ffca07f9a18a07351dcb54bb85a451aacb74 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 13:30:41 -0400 Subject: [PATCH 1/9] replace procsrv functions with `Command` --- src/tools/compiletest/src/procsrv.rs | 92 +-------------- src/tools/compiletest/src/runtest.rs | 160 ++++++++++++--------------- 2 files changed, 70 insertions(+), 182 deletions(-) diff --git a/src/tools/compiletest/src/procsrv.rs b/src/tools/compiletest/src/procsrv.rs index ffcc60e785292..ab9eb5b9a5dab 100644 --- a/src/tools/compiletest/src/procsrv.rs +++ b/src/tools/compiletest/src/procsrv.rs @@ -10,10 +10,8 @@ use std::env; use std::ffi::OsString; -use std::io::prelude::*; -use std::io; use std::path::PathBuf; -use std::process::{Child, Command, ExitStatus, Output, Stdio}; +use std::process::Command; /// Get the name of the environment variable that holds dynamic library /// locations @@ -31,7 +29,7 @@ pub fn dylib_env_var() -> &'static str { /// Add `lib_path` and `aux_path` (if it is `Some`) to the dynamic library /// env var -fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) { +pub fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) { // Need to be sure to put both the lib_path and the aux path in the dylib // search path for the child. let var = dylib_env_var(); @@ -46,89 +44,3 @@ fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) { let newpath = env::join_paths(&path).unwrap(); cmd.env(var, newpath); } - -/// Represents exit status, stdout and stderr of a completed process -pub struct Result { - pub status: ExitStatus, - pub out: String, - pub err: String, -} - -/// Runs a test program -/// -/// # Params -/// - `lib_path` Path to search for required library -/// - `prog` command to run -/// - `aux_path` Optional extra path to search for required -/// auxiliary libraries -/// - `args` List of arguments to pass to `prog` -/// - `env` List of environment variables to set, `.0` is variable name, -/// `.1` is value -/// - `input` String to be fed as stdin -/// - `current_dir` Optional working dir to run command in -/// -pub fn run(lib_path: &str, - prog: &str, - aux_path: Option<&str>, - args: &[String], - env: Vec<(String, String)>, - input: Option, - current_dir: Option) - -> io::Result { - - let mut cmd = Command::new(prog); - cmd.args(args) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .stdin(Stdio::piped()); - - add_target_env(&mut cmd, lib_path, aux_path); - for (key, val) in env { - cmd.env(&key, &val); - } - if let Some(cwd) = current_dir { - cmd.current_dir(cwd); - } - - let mut process = cmd.spawn()?; - if let Some(input) = input { - process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap(); - } - let Output { status, stdout, stderr } = process.wait_with_output().unwrap(); - - Ok(Result { - status, - out: String::from_utf8(stdout).unwrap(), - err: String::from_utf8(stderr).unwrap(), - }) -} - -/// Same as `run`, but return process rather than waiting on completion -pub fn run_background(lib_path: &str, - prog: &str, - aux_path: Option<&str>, - args: &[String], - env: Vec<(String, String)>, - input: Option, - current_dir: Option) - -> io::Result { - - let mut cmd = Command::new(prog); - cmd.args(args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()); - add_target_env(&mut cmd, lib_path, aux_path); - for (key, val) in env { - cmd.env(&key, &val); - } - if let Some(cwd) = current_dir { - cmd.current_dir(cwd); - } - - let mut process = cmd.spawn()?; - if let Some(input) = input { - process.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap(); - } - - Ok(process) -} diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 02511ac6d98bc..7614c865ce2cb 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -27,7 +27,7 @@ use std::fs::{self, File, create_dir_all}; use std::io::prelude::*; use std::io::{self, BufReader}; use std::path::{Path, PathBuf}; -use std::process::{Command, Output, ExitStatus}; +use std::process::{Command, Output, ExitStatus, Stdio}; use std::str; use std::collections::HashMap; @@ -500,32 +500,19 @@ actual:\n\ debug!("script_str = {}", script_str); self.dump_output_file(&script_str, "debugger.script"); + let adb_path = &self.config.adb_path; - procsrv::run("", - &self.config.adb_path, - None, - &[ - "push".to_owned(), - exe_file.to_str().unwrap().to_owned(), - self.config.adb_test_dir.clone() - ], - Vec::new(), - None, - None) - .expect(&format!("failed to exec `{:?}`", self.config.adb_path)); - - procsrv::run("", - &self.config.adb_path, - None, - &[ - "forward".to_owned(), - "tcp:5039".to_owned(), - "tcp:5039".to_owned() - ], - Vec::new(), - None, - None) - .expect(&format!("failed to exec `{:?}`", self.config.adb_path)); + Command::new(adb_path) + .arg("push") + .arg(&exe_file) + .arg(&self.config.adb_test_dir) + .status() + .expect(&format!("failed to exec `{:?}`", adb_path)); + + Command::new(adb_path) + .args(&["forward", "tcp:5039", "tcp:5039"]) + .status() + .expect(&format!("failed to exec `{:?}`", adb_path)); let adb_arg = format!("export LD_LIBRARY_PATH={}; \ gdbserver{} :5039 {}/{}", @@ -537,23 +524,15 @@ actual:\n\ .unwrap()); debug!("adb arg: {}", adb_arg); - let mut process = procsrv::run_background("", - &self.config.adb_path - , - None, - &[ - "shell".to_owned(), - adb_arg.clone() - ], - Vec::new(), - None, - None) - .expect(&format!("failed to exec `{:?}`", self.config.adb_path)); + let mut adb = Command::new(adb_path) + .args(&["shell", &adb_arg]) + .spawn() + .expect(&format!("failed to exec `{:?}`", adb_path)); // Wait for the gdbserver to print out "Listening on port ..." // at which point we know that it's started and then we can // execute the debugger below. - let mut stdout = BufReader::new(process.stdout.take().unwrap()); + let mut stdout = BufReader::new(adb.stdout.take().unwrap()); let mut line = String::new(); loop { line.truncate(0); @@ -574,17 +553,13 @@ actual:\n\ let mut gdb_path = tool_path; gdb_path.push_str("/bin/gdb"); - let procsrv::Result { - out, - err, - status - } = procsrv::run("", - &gdb_path, - None, - &debugger_opts, - Vec::new(), - None, - None) + let Output { + status, + stdout, + stderr + } = Command::new(&gdb_path) + .args(&debugger_opts) + .output() .expect(&format!("failed to exec `{:?}`", gdb_path)); let cmdline = { let cmdline = self.make_cmdline("", @@ -596,11 +571,11 @@ actual:\n\ debugger_run_result = ProcRes { status, - stdout: out, - stderr: err, + stdout: String::from_utf8(stdout).unwrap(), + stderr: String::from_utf8(stderr).unwrap(), cmdline, }; - if process.kill().is_err() { + if adb.kill().is_err() { println!("Adb process is already finished."); } } @@ -1367,7 +1342,46 @@ actual:\n\ aux_path: Option<&str>, input: Option, working_dir: Option) -> ProcRes { - self.program_output(lib_path, prog, aux_path, args, procenv, input, working_dir) + let cmdline = + { + let cmdline = self.make_cmdline(lib_path, + &prog, + &args); + logv(self.config, format!("executing {}", cmdline)); + cmdline + }; + + let mut process = Command::new(&prog); + process + .args(&args) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .stdin(Stdio::piped()); + + procsrv::add_target_env(&mut process, lib_path, aux_path); + for (key, val) in procenv { + process.env(&key, &val); + } + if let Some(cwd) = working_dir { + process.current_dir(cwd); + } + + let mut child = process.spawn().expect(&format!("failed to exec `{}`", prog)); + if let Some(input) = input { + child.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap(); + } + let Output { status, stdout, stderr } = child.wait_with_output().unwrap(); + + let result = ProcRes { + status, + stdout: String::from_utf8(stdout).unwrap(), + stderr: String::from_utf8(stderr).unwrap(), + cmdline, + }; + + self.dump_output(&result.stdout, &result.stderr); + + result } fn make_compile_args(&self, @@ -1554,44 +1568,6 @@ actual:\n\ } } - fn program_output(&self, - lib_path: &str, - prog: String, - aux_path: Option<&str>, - args: Vec, - env: Vec<(String, String)>, - input: Option, - working_dir: Option) - -> ProcRes { - let cmdline = - { - let cmdline = self.make_cmdline(lib_path, - &prog, - &args); - logv(self.config, format!("executing {}", cmdline)); - cmdline - }; - - let procsrv::Result { - out, - err, - status - } = procsrv::run(lib_path, - &prog, - aux_path, - &args, - env, - input, - working_dir).expect(&format!("failed to exec `{}`", prog)); - self.dump_output(&out, &err); - ProcRes { - status, - stdout: out, - stderr: err, - cmdline, - } - } - fn make_cmdline(&self, libpath: &str, prog: &str, args: &[String]) -> String { use util; From e385bc5bcceaeca1c3e0a0b8c0ae38b9ff462a3a Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 14:37:20 -0400 Subject: [PATCH 2/9] let `compose_and_run` take a `Command` --- src/tools/compiletest/src/runtest.rs | 123 ++++++++++++--------------- 1 file changed, 55 insertions(+), 68 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 7614c865ce2cb..a17036ead6381 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -325,12 +325,19 @@ impl<'test> TestCx<'test> { } } - fn print_source(&self, - src: String, - pretty_type: &str) - -> ProcRes { + fn print_source(&self, src: String, pretty_type: &str) -> ProcRes { let aux_dir = self.aux_output_dir_name(); - self.compose_and_run(self.make_pp_args(pretty_type.to_owned()), + + let mut rustc = Command::new(&self.config.rustc_path); + rustc.arg("-") + .arg("-Zunstable-options") + .args(&["--unpretty", &pretty_type]) + .args(&["--target", &self.config.target]) + .arg("-L").arg(&aux_dir) + .args(self.split_maybe_args(&self.config.target_rustcflags)) + .args(&self.props.compile_flags); + + self.compose_and_run(rustc, self.props.exec_env.clone(), self.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -338,26 +345,6 @@ impl<'test> TestCx<'test> { None) } - fn make_pp_args(&self, - pretty_type: String) - -> ProcArgs { - let aux_dir = self.aux_output_dir_name(); - // FIXME (#9639): This needs to handle non-utf8 paths - let mut args = vec!["-".to_owned(), - "-Zunstable-options".to_owned(), - "--unpretty".to_owned(), - pretty_type, - format!("--target={}", self.config.target), - "-L".to_owned(), - aux_dir.to_str().unwrap().to_owned()]; - args.extend(self.split_maybe_args(&self.config.target_rustcflags)); - args.extend(self.props.compile_flags.iter().cloned()); - ProcArgs { - prog: self.config.rustc_path.to_str().unwrap().to_owned(), - args, - } - } - fn compare_source(&self, expected: &str, actual: &str) { @@ -562,9 +549,9 @@ actual:\n\ .output() .expect(&format!("failed to exec `{:?}`", gdb_path)); let cmdline = { - let cmdline = self.make_cmdline("", - &format!("{}-gdb", self.config.target), - &debugger_opts); + let mut gdb = Command::new(&format!("{}-gdb", self.config.target)); + gdb.args(&debugger_opts); + let cmdline = self.make_cmdline(&gdb, ""); logv(self.config, format!("executing {}", cmdline)); cmdline }; @@ -654,15 +641,13 @@ actual:\n\ "-nx".to_owned(), format!("-command={}", debugger_script.to_str().unwrap())]; - let proc_args = ProcArgs { - prog: self.config.gdb.as_ref().unwrap().to_owned(), - args: debugger_opts, - }; + let mut gdb = Command::new(self.config.gdb.as_ref().unwrap()); + gdb.args(&debugger_opts); let environment = vec![("PYTHONPATH".to_owned(), rust_pp_module_abs_path)]; debugger_run_result = - self.compose_and_run(proc_args, + self.compose_and_run(gdb, environment, self.config.run_lib_path.to_str().unwrap(), None, @@ -1205,23 +1190,22 @@ actual:\n\ // 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 mut args = self.make_run_args(); - let mut program = args.prog.clone(); + let ProcArgs { mut prog, args } = self.make_run_args(); if let Ok(entries) = aux_dir.read_dir() { for entry in entries { let entry = entry.unwrap(); if !entry.path().is_file() { continue } - program.push_str(":"); - program.push_str(entry.path().to_str().unwrap()); + prog.push_str(":"); + prog.push_str(entry.path().to_str().unwrap()); } } - args.args.insert(0, program); - args.args.insert(0, "run".to_string()); - args.prog = self.config.remote_test_client.clone().unwrap() - .into_os_string().into_string().unwrap(); - self.compose_and_run(args, + let mut test_client = Command::new( + self.config.remote_test_client.as_ref().unwrap()); + test_client.args(&["run", &prog]); + test_client.args(args); + self.compose_and_run(test_client, env, self.config.run_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1234,7 +1218,10 @@ actual:\n\ Some(self.output_base_name() .parent().unwrap() .to_str().unwrap().to_owned()); - self.compose_and_run(self.make_run_args(), + let ProcArgs { prog, args } = self.make_run_args(); + let mut program = Command::new(&prog); + program.args(args); + self.compose_and_run(program, env, self.config.run_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1312,8 +1299,11 @@ actual:\n\ testpaths: &aux_testpaths, revision: self.revision }; - let aux_args = aux_cx.make_compile_args(crate_type, &aux_testpaths.file, aux_output); - let auxres = aux_cx.compose_and_run(aux_args, + let ProcArgs { prog, args } = + aux_cx.make_compile_args(crate_type, &aux_testpaths.file, aux_output); + let mut rustc = Command::new(prog); + rustc.args(&args); + let auxres = aux_cx.compose_and_run(rustc, Vec::new(), aux_cx.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1327,7 +1317,11 @@ actual:\n\ } } - self.compose_and_run(args, + let ProcArgs { prog, args } = args; + let mut rustc = Command::new(prog); + rustc.args(args); + + self.compose_and_run(rustc, self.props.rustc_env.clone(), self.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1336,7 +1330,7 @@ actual:\n\ } fn compose_and_run(&self, - ProcArgs{ args, prog }: ProcArgs, + mut command: Command, procenv: Vec<(String, String)> , lib_path: &str, aux_path: Option<&str>, @@ -1344,29 +1338,25 @@ actual:\n\ working_dir: Option) -> ProcRes { let cmdline = { - let cmdline = self.make_cmdline(lib_path, - &prog, - &args); + let cmdline = self.make_cmdline(&command, lib_path); logv(self.config, format!("executing {}", cmdline)); cmdline }; - let mut process = Command::new(&prog); - process - .args(&args) + command .stdout(Stdio::piped()) .stderr(Stdio::piped()) .stdin(Stdio::piped()); - procsrv::add_target_env(&mut process, lib_path, aux_path); + procsrv::add_target_env(&mut command, lib_path, aux_path); for (key, val) in procenv { - process.env(&key, &val); + command.env(&key, &val); } if let Some(cwd) = working_dir { - process.current_dir(cwd); + command.current_dir(cwd); } - let mut child = process.spawn().expect(&format!("failed to exec `{}`", prog)); + let mut child = command.spawn().expect(&format!("failed to exec `{:?}`", &command)); if let Some(input) = input { child.stdin.as_mut().unwrap().write_all(input.as_bytes()).unwrap(); } @@ -1568,12 +1558,12 @@ actual:\n\ } } - fn make_cmdline(&self, libpath: &str, prog: &str, args: &[String]) -> String { + fn make_cmdline(&self, command: &Command, libpath: &str) -> String { use util; // Linux and mac don't require adjusting the library search path if cfg!(unix) { - format!("{} {}", prog, args.join(" ")) + format!("{:?}", command) } else { // Build the LD_LIBRARY_PATH variable as it would be seen on the command line // for diagnostic purposes @@ -1581,7 +1571,7 @@ actual:\n\ format!("{}=\"{}\"", util::lib_path_env_var(), util::make_new_path(path)) } - format!("{} {} {}", lib_path_cmd_prefix(libpath), prog, args.join(" ")) + format!("{} {:?}", lib_path_cmd_prefix(libpath), command) } } @@ -1715,14 +1705,11 @@ actual:\n\ fn check_ir_with_filecheck(&self) -> ProcRes { let irfile = self.output_base_name().with_extension("ll"); - let prog = self.config.llvm_filecheck.as_ref().unwrap(); - let proc_args = ProcArgs { - // FIXME (#9639): This needs to handle non-utf8 paths - prog: prog.to_str().unwrap().to_owned(), - args: vec![format!("-input-file={}", irfile.to_str().unwrap()), - self.testpaths.file.to_str().unwrap().to_owned()] - }; - self.compose_and_run(proc_args, Vec::new(), "", None, None, None) + let mut filecheck = Command::new(self.config.llvm_filecheck.as_ref().unwrap()); + // FIXME (#9639): This needs to handle non-utf8 paths + filecheck.arg(&format!("-input-file={}", irfile.to_str().unwrap())); + filecheck.arg(&self.testpaths.file); + self.compose_and_run(filecheck, Vec::new(), "", None, None, None) } fn run_codegen_test(&self) { From 0a666f87caa3590d2dad54b30ac752e01c6b372c Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 14:44:08 -0400 Subject: [PATCH 3/9] remove `working_dir` argument --- src/tools/compiletest/src/runtest.rs | 60 ++++++++++------------------ 1 file changed, 20 insertions(+), 40 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a17036ead6381..300861b22a168 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -335,14 +335,13 @@ impl<'test> TestCx<'test> { .args(&["--target", &self.config.target]) .arg("-L").arg(&aux_dir) .args(self.split_maybe_args(&self.config.target_rustcflags)) - .args(&self.props.compile_flags); + .args(&self.props.compile_flags) + .envs(self.props.exec_env.clone()); self.compose_and_run(rustc, - self.props.exec_env.clone(), self.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), - Some(src), - None) + Some(src)) } fn compare_source(&self, @@ -642,16 +641,13 @@ actual:\n\ format!("-command={}", debugger_script.to_str().unwrap())]; let mut gdb = Command::new(self.config.gdb.as_ref().unwrap()); - gdb.args(&debugger_opts); - - let environment = vec![("PYTHONPATH".to_owned(), rust_pp_module_abs_path)]; + gdb.args(&debugger_opts) + .env("PYTHONPATH", rust_pp_module_abs_path); debugger_run_result = self.compose_and_run(gdb, - environment, self.config.run_lib_path.to_str().unwrap(), None, - None, None); } } @@ -1203,30 +1199,26 @@ actual:\n\ } let mut test_client = Command::new( self.config.remote_test_client.as_ref().unwrap()); - test_client.args(&["run", &prog]); - test_client.args(args); + test_client + .args(&["run", &prog]) + .args(args) + .envs(env.clone()); self.compose_and_run(test_client, - env, self.config.run_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), - None, None) } _ => { let aux_dir = self.aux_output_dir_name(); - let working_dir = - Some(self.output_base_name() - .parent().unwrap() - .to_str().unwrap().to_owned()); let ProcArgs { prog, args } = self.make_run_args(); let mut program = Command::new(&prog); - program.args(args); + program.args(args) + .current_dir(&self.output_base_name().parent().unwrap()) + .envs(env.clone()); self.compose_and_run(program, - env, self.config.run_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), - None, - working_dir) + None) } } } @@ -1304,10 +1296,8 @@ actual:\n\ let mut rustc = Command::new(prog); rustc.args(&args); let auxres = aux_cx.compose_and_run(rustc, - Vec::new(), aux_cx.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), - None, None); if !auxres.status.success() { self.fatal_proc_rec( @@ -1319,23 +1309,20 @@ actual:\n\ let ProcArgs { prog, args } = args; let mut rustc = Command::new(prog); - rustc.args(args); + rustc.args(args) + .envs(self.props.rustc_env.clone()); self.compose_and_run(rustc, - self.props.rustc_env.clone(), self.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), - input, - None) + input) } fn compose_and_run(&self, mut command: Command, - procenv: Vec<(String, String)> , lib_path: &str, aux_path: Option<&str>, - input: Option, - working_dir: Option) -> ProcRes { + input: Option) -> ProcRes { let cmdline = { let cmdline = self.make_cmdline(&command, lib_path); @@ -1349,12 +1336,6 @@ actual:\n\ .stdin(Stdio::piped()); procsrv::add_target_env(&mut command, lib_path, aux_path); - for (key, val) in procenv { - command.env(&key, &val); - } - if let Some(cwd) = working_dir { - command.current_dir(cwd); - } let mut child = command.spawn().expect(&format!("failed to exec `{:?}`", &command)); if let Some(input) = input { @@ -1706,10 +1687,9 @@ actual:\n\ fn check_ir_with_filecheck(&self) -> ProcRes { let irfile = self.output_base_name().with_extension("ll"); let mut filecheck = Command::new(self.config.llvm_filecheck.as_ref().unwrap()); - // FIXME (#9639): This needs to handle non-utf8 paths - filecheck.arg(&format!("-input-file={}", irfile.to_str().unwrap())); - filecheck.arg(&self.testpaths.file); - self.compose_and_run(filecheck, Vec::new(), "", None, None, None) + filecheck.arg("--input-file").arg(irfile) + .arg(&self.testpaths.file); + self.compose_and_run(filecheck, "", None, None) } fn run_codegen_test(&self) { From 90a87be9cb075ba03322de085ca0077c7f20dc47 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 15:18:00 -0400 Subject: [PATCH 4/9] make `compose_and_run_compiler` take `Command` --- src/tools/compiletest/src/runtest.rs | 92 ++++++++++++---------------- 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 300861b22a168..4d54d5d0d0c08 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -365,45 +365,35 @@ actual:\n\ } fn typecheck_source(&self, src: String) -> ProcRes { - let args = self.make_typecheck_args(); - self.compose_and_run_compiler(args, Some(src)) - } + let mut rustc = Command::new(&self.config.rustc_path); + + let out_dir = self.output_base_name().with_extension("pretty-out"); + let _ = fs::remove_dir_all(&out_dir); + create_dir_all(&out_dir).unwrap(); - fn make_typecheck_args(&self) -> ProcArgs { - let aux_dir = self.aux_output_dir_name(); let target = if self.props.force_host { &*self.config.host } else { &*self.config.target }; - let out_dir = self.output_base_name().with_extension("pretty-out"); - let _ = fs::remove_dir_all(&out_dir); - create_dir_all(&out_dir).unwrap(); + let aux_dir = self.aux_output_dir_name(); + + rustc.arg("-") + .arg("-Zno-trans") + .arg("--out-dir").arg(&out_dir) + .arg(&format!("--target={}", target)) + .arg("-L").arg(&self.config.build_base) + .arg("-L").arg(aux_dir); - // FIXME (#9639): This needs to handle non-utf8 paths - let mut args = vec!["-".to_owned(), - "-Zno-trans".to_owned(), - "--out-dir".to_owned(), - out_dir.to_str().unwrap().to_owned(), - format!("--target={}", target), - "-L".to_owned(), - self.config.build_base.to_str().unwrap().to_owned(), - "-L".to_owned(), - aux_dir.to_str().unwrap().to_owned()]; if let Some(revision) = self.revision { - args.extend(vec![ - "--cfg".to_string(), - revision.to_string(), - ]); - } - args.extend(self.split_maybe_args(&self.config.target_rustcflags)); - args.extend(self.props.compile_flags.iter().cloned()); - // FIXME (#9639): This needs to handle non-utf8 paths - ProcArgs { - prog: self.config.rustc_path.to_str().unwrap().to_owned(), - args, + rustc.args(&["--cfg", revision]); } + + rustc.args(self.split_maybe_args(&self.config.target_rustcflags)); + rustc.args(&self.props.compile_flags); + + self.compose_and_run_compiler(rustc, Some(src)) } fn run_debuginfo_gdb_test(&self) { @@ -1126,10 +1116,11 @@ actual:\n\ } _ => {} } - let args = self.make_compile_args(extra_args, - &self.testpaths.file, - TargetLocation::ThisFile(self.make_exe_name())); - self.compose_and_run_compiler(args, None) + let ProcArgs { prog, args } = self.make_compile_args( + extra_args, &self.testpaths.file, TargetLocation::ThisFile(self.make_exe_name())); + let mut rustc = Command::new(prog); + rustc.args(args); + self.compose_and_run_compiler(rustc, None) } fn document(&self, out_dir: &Path) -> ProcRes { @@ -1153,18 +1144,16 @@ actual:\n\ } let aux_dir = self.aux_output_dir_name(); - let mut args = vec!["-L".to_owned(), - aux_dir.to_str().unwrap().to_owned(), - "-o".to_owned(), - out_dir.to_str().unwrap().to_owned(), - self.testpaths.file.to_str().unwrap().to_owned()]; - args.extend(self.props.compile_flags.iter().cloned()); - let args = ProcArgs { - prog: self.config.rustdoc_path - .as_ref().expect("--rustdoc-path passed").to_str().unwrap().to_owned(), - args, - }; - self.compose_and_run_compiler(args, None) + + let rustdoc_path = self.config.rustdoc_path.as_ref().expect("--rustdoc-path passed"); + let mut rustdoc = Command::new(rustdoc_path); + + rustdoc.arg("-L").arg(aux_dir) + .arg("-o").arg(out_dir) + .arg(&self.testpaths.file) + .args(&self.props.compile_flags); + + self.compose_and_run_compiler(rustdoc, None) } fn exec_compiled_test(&self) -> ProcRes { @@ -1247,7 +1236,7 @@ actual:\n\ } } - fn compose_and_run_compiler(&self, args: ProcArgs, input: Option) -> ProcRes { + fn compose_and_run_compiler(&self, mut rustc: Command, input: Option) -> ProcRes { if !self.props.aux_builds.is_empty() { create_dir_all(&self.aux_output_dir_name()).unwrap(); } @@ -1307,11 +1296,7 @@ actual:\n\ } } - let ProcArgs { prog, args } = args; - let mut rustc = Command::new(prog); - rustc.args(args) - .envs(self.props.rustc_env.clone()); - + rustc.envs(self.props.rustc_env.clone()); self.compose_and_run(rustc, self.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1681,7 +1666,10 @@ actual:\n\ self.output_base_name().parent() .unwrap() .to_path_buf())); - self.compose_and_run_compiler(args, None) + let ProcArgs { prog, args } = args; + let mut rustc = Command::new(prog); + rustc.args(args); + self.compose_and_run_compiler(rustc, None) } fn check_ir_with_filecheck(&self) -> ProcRes { From 04dee085016741016c77d958323fe4e19cea3037 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 15:30:29 -0400 Subject: [PATCH 5/9] return `Command` from make_compile_args --- src/tools/compiletest/src/runtest.rs | 113 +++++++++++---------------- 1 file changed, 45 insertions(+), 68 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4d54d5d0d0c08..67e5e0321360a 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1116,10 +1116,9 @@ actual:\n\ } _ => {} } - let ProcArgs { prog, args } = self.make_compile_args( + + let rustc = self.make_compile_args( extra_args, &self.testpaths.file, TargetLocation::ThisFile(self.make_exe_name())); - let mut rustc = Command::new(prog); - rustc.args(args); self.compose_and_run_compiler(rustc, None) } @@ -1280,11 +1279,9 @@ actual:\n\ testpaths: &aux_testpaths, revision: self.revision }; - let ProcArgs { prog, args } = + let aux_rustc = aux_cx.make_compile_args(crate_type, &aux_testpaths.file, aux_output); - let mut rustc = Command::new(prog); - rustc.args(&args); - let auxres = aux_cx.compose_and_run(rustc, + let auxres = aux_cx.compose_and_run(aux_rustc, aux_cx.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), None); @@ -1341,21 +1338,14 @@ actual:\n\ } fn make_compile_args(&self, - extras: Vec , + extra_args: Vec, input_file: &Path, output_file: TargetLocation) - -> ProcArgs + -> Command { - let target = if self.props.force_host { - &*self.config.host - } else { - &*self.config.target - }; - - // FIXME (#9639): This needs to handle non-utf8 paths - let mut args = vec![input_file.to_str().unwrap().to_owned(), - "-L".to_owned(), - self.config.build_base.to_str().unwrap().to_owned()]; + let mut rustc = Command::new(&self.config.rustc_path); + rustc.arg(input_file) + .arg("-L").arg(&self.config.build_base); // Optionally prevent default --target if specified in test compile-flags. let custom_target = self.props.compile_flags @@ -1363,26 +1353,23 @@ actual:\n\ .fold(false, |acc, x| acc || x.starts_with("--target")); if !custom_target { - args.extend(vec![ - format!("--target={}", target), - ]); + let target = if self.props.force_host { + &*self.config.host + } else { + &*self.config.target + }; + + rustc.arg(&format!("--target={}", target)); } if let Some(revision) = self.revision { - args.extend(vec![ - "--cfg".to_string(), - revision.to_string(), - ]); + rustc.args(&["--cfg", revision]); } if let Some(ref incremental_dir) = self.props.incremental_dir { - args.extend(vec![ - "-Z".to_string(), - format!("incremental={}", incremental_dir.display()), - ]); + rustc.args(&["-Z", &format!("incremental={}", incremental_dir.display())]); } - match self.config.mode { CompileFail | ParseFail | @@ -1391,19 +1378,14 @@ actual:\n\ // fashion, then you want JSON mode. Old-skool error // patterns still match the raw compiler output. if self.props.error_patterns.is_empty() { - args.extend(["--error-format", - "json"] - .iter() - .map(|s| s.to_string())); + rustc.args(&["--error-format", "json"]); } } MirOpt => { - args.extend(["-Zdump-mir=all", - "-Zmir-opt-level=3", - "-Zdump-mir-exclude-pass-number"] - .iter() - .map(|s| s.to_string())); - + rustc.args(&[ + "-Zdump-mir=all", + "-Zmir-opt-level=3", + "-Zdump-mir-exclude-pass-number"]); let mir_dump_dir = self.get_mir_dump_dir(); create_dir_all(mir_dump_dir.as_path()).unwrap(); @@ -1411,7 +1393,7 @@ actual:\n\ dir_opt.push_str(mir_dump_dir.to_str().unwrap()); debug!("dir_opt: {:?}", dir_opt); - args.push(dir_opt); + rustc.arg(dir_opt); } RunPass | RunFail | @@ -1428,32 +1410,30 @@ actual:\n\ } } - args.extend_from_slice(&extras); + rustc.args(&extra_args); + if !self.props.no_prefer_dynamic { - args.push("-C".to_owned()); - args.push("prefer-dynamic".to_owned()); + rustc.args(&["-C", "prefer-dynamic"]); } - let path = match output_file { + + match output_file { TargetLocation::ThisFile(path) => { - args.push("-o".to_owned()); - path + rustc.arg("-o").arg(path); } TargetLocation::ThisDirectory(path) => { - args.push("--out-dir".to_owned()); - path + rustc.arg("--out-dir").arg(path); } - }; - args.push(path.to_str().unwrap().to_owned()); + } + if self.props.force_host { - args.extend(self.split_maybe_args(&self.config.host_rustcflags)); + rustc.args(self.split_maybe_args(&self.config.host_rustcflags)); } else { - args.extend(self.split_maybe_args(&self.config.target_rustcflags)); - } - args.extend(self.props.compile_flags.iter().cloned()); - ProcArgs { - prog: self.config.rustc_path.to_str().unwrap().to_owned(), - args, + rustc.args(self.split_maybe_args(&self.config.target_rustcflags)); } + + rustc.args(&self.props.compile_flags); + + rustc } fn make_lib_name(&self, auxfile: &Path) -> PathBuf { @@ -1660,15 +1640,12 @@ actual:\n\ aux_dir.to_str().unwrap().to_owned()]; let llvm_args = vec!["--emit=llvm-ir".to_owned(),]; link_args.extend(llvm_args); - let args = self.make_compile_args(link_args, - &self.testpaths.file, - TargetLocation::ThisDirectory( - self.output_base_name().parent() - .unwrap() - .to_path_buf())); - let ProcArgs { prog, args } = args; - let mut rustc = Command::new(prog); - rustc.args(args); + let rustc = self.make_compile_args(link_args, + &self.testpaths.file, + TargetLocation::ThisDirectory( + self.output_base_name().parent() + .unwrap() + .to_path_buf())); self.compose_and_run_compiler(rustc, None) } From 6f21239fa5d7f586bbed1dfaf413ca7a031ab0d4 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 16:31:11 -0400 Subject: [PATCH 6/9] remove `extra_args` argument --- src/tools/compiletest/src/runtest.rs | 86 ++++++++++++---------------- 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 67e5e0321360a..e129c39936dd7 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1100,10 +1100,11 @@ actual:\n\ } fn compile_test(&self) -> ProcRes { - let aux_dir = self.aux_output_dir_name(); - // FIXME (#9639): This needs to handle non-utf8 paths - let mut extra_args = vec!["-L".to_owned(), - aux_dir.to_str().unwrap().to_owned()]; + let mut rustc = self.make_compile_args( + &self.testpaths.file, TargetLocation::ThisFile(self.make_exe_name())); + + rustc.arg("-L").arg(&self.aux_output_dir_name()); + match self.config.mode { CompileFail | Ui => { // compile-fail and ui tests tend to have tons of unused code as @@ -1111,14 +1112,11 @@ actual:\n\ // want to actually assert warnings about all this code. Instead // let's just ignore unused code warnings by defaults and tests // can turn it back on if needed. - extra_args.push("-A".to_owned()); - extra_args.push("unused".to_owned()); + rustc.args(&["-A", "unused"]); } _ => {} } - let rustc = self.make_compile_args( - extra_args, &self.testpaths.file, TargetLocation::ThisFile(self.make_exe_name())); self.compose_and_run_compiler(rustc, None) } @@ -1241,17 +1239,27 @@ actual:\n\ } let aux_dir = self.aux_output_dir_name(); - // FIXME (#9639): This needs to handle non-utf8 paths - let extra_link_args = vec!["-L".to_owned(), - aux_dir.to_str().unwrap().to_owned()]; for rel_ab in &self.props.aux_builds { let aux_testpaths = self.compute_aux_test_paths(rel_ab); let aux_props = self.props.from_aux_file(&aux_testpaths.file, self.revision, self.config); - let mut crate_type = if aux_props.no_prefer_dynamic { - Vec::new() + let aux_output = { + let f = self.make_lib_name(&self.testpaths.file); + let parent = f.parent().unwrap(); + TargetLocation::ThisDirectory(parent.to_path_buf()) + }; + let aux_cx = TestCx { + config: self.config, + props: &aux_props, + testpaths: &aux_testpaths, + revision: self.revision + }; + let mut aux_rustc = aux_cx.make_compile_args(&aux_testpaths.file, aux_output); + + let crate_type = if aux_props.no_prefer_dynamic { + None } else if (self.config.target.contains("musl") && !aux_props.force_host) || self.config.target.contains("emscripten") { // We primarily compile all auxiliary libraries as dynamic libraries @@ -1263,24 +1271,17 @@ actual:\n\ // dynamic libraries so we just go back to building a normal library. Note, // however, that for MUSL if the library is built with `force_host` then // it's ok to be a dylib as the host should always support dylibs. - vec!["--crate-type=lib".to_owned()] + Some("lib") } else { - vec!["--crate-type=dylib".to_owned()] - }; - crate_type.extend(extra_link_args.clone()); - let aux_output = { - let f = self.make_lib_name(&self.testpaths.file); - let parent = f.parent().unwrap(); - TargetLocation::ThisDirectory(parent.to_path_buf()) - }; - let aux_cx = TestCx { - config: self.config, - props: &aux_props, - testpaths: &aux_testpaths, - revision: self.revision + Some("dylib") }; - let aux_rustc = - aux_cx.make_compile_args(crate_type, &aux_testpaths.file, aux_output); + + if let Some(crate_type) = crate_type { + aux_rustc.args(&["--crate-type", crate_type]); + } + + aux_rustc.arg("-L").arg(&aux_dir); + let auxres = aux_cx.compose_and_run(aux_rustc, aux_cx.config.compile_lib_path.to_str().unwrap(), Some(aux_dir.to_str().unwrap()), @@ -1337,12 +1338,7 @@ actual:\n\ result } - fn make_compile_args(&self, - extra_args: Vec, - input_file: &Path, - output_file: TargetLocation) - -> Command - { + fn make_compile_args(&self, input_file: &Path, output_file: TargetLocation) -> Command { let mut rustc = Command::new(&self.config.rustc_path); rustc.arg(input_file) .arg("-L").arg(&self.config.build_base); @@ -1410,8 +1406,6 @@ actual:\n\ } } - rustc.args(&extra_args); - if !self.props.no_prefer_dynamic { rustc.args(&["-C", "prefer-dynamic"]); } @@ -1635,17 +1629,13 @@ actual:\n\ fn compile_test_and_save_ir(&self) -> ProcRes { let aux_dir = self.aux_output_dir_name(); - // FIXME (#9639): This needs to handle non-utf8 paths - let mut link_args = vec!["-L".to_owned(), - aux_dir.to_str().unwrap().to_owned()]; - let llvm_args = vec!["--emit=llvm-ir".to_owned(),]; - link_args.extend(llvm_args); - let rustc = self.make_compile_args(link_args, - &self.testpaths.file, - TargetLocation::ThisDirectory( - self.output_base_name().parent() - .unwrap() - .to_path_buf())); + + let output_file = TargetLocation::ThisDirectory( + self.output_base_name().parent().unwrap().to_path_buf()); + let mut rustc = self.make_compile_args(&self.testpaths.file, output_file); + rustc.arg("-L").arg(aux_dir) + .arg("--emit=llvm-ir"); + self.compose_and_run_compiler(rustc, None) } From 88bac1f74521b913f044212936932de5d924e631 Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Thu, 10 Aug 2017 19:28:25 -0400 Subject: [PATCH 7/9] remove procsrv module --- src/tools/compiletest/src/main.rs | 1 - src/tools/compiletest/src/procsrv.rs | 46 ---------------------------- src/tools/compiletest/src/runtest.rs | 32 ++++++++++++++++--- 3 files changed, 28 insertions(+), 51 deletions(-) delete mode 100644 src/tools/compiletest/src/procsrv.rs diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 384ae3f45f60c..20239e974788b 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -39,7 +39,6 @@ use util::logv; use self::header::EarlyProps; -pub mod procsrv; pub mod util; mod json; pub mod header; diff --git a/src/tools/compiletest/src/procsrv.rs b/src/tools/compiletest/src/procsrv.rs deleted file mode 100644 index ab9eb5b9a5dab..0000000000000 --- a/src/tools/compiletest/src/procsrv.rs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2012 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use std::env; -use std::ffi::OsString; -use std::path::PathBuf; -use std::process::Command; - -/// Get the name of the environment variable that holds dynamic library -/// locations -pub fn dylib_env_var() -> &'static str { - if cfg!(windows) { - "PATH" - } else if cfg!(target_os = "macos") { - "DYLD_LIBRARY_PATH" - } else if cfg!(target_os = "haiku") { - "LIBRARY_PATH" - } else { - "LD_LIBRARY_PATH" - } -} - -/// Add `lib_path` and `aux_path` (if it is `Some`) to the dynamic library -/// env var -pub fn add_target_env(cmd: &mut Command, lib_path: &str, aux_path: Option<&str>) { - // Need to be sure to put both the lib_path and the aux path in the dylib - // search path for the child. - let var = dylib_env_var(); - let mut path = env::split_paths(&env::var_os(var).unwrap_or(OsString::new())) - .collect::>(); - if let Some(p) = aux_path { - path.insert(0, PathBuf::from(p)) - } - path.insert(0, PathBuf::from(lib_path)); - - // Add the new dylib search path var - let newpath = env::join_paths(&path).unwrap(); - cmd.env(var, newpath); -} diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index e129c39936dd7..af6b4c279318a 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -17,22 +17,35 @@ use errors::{self, ErrorKind, Error}; use filetime::FileTime; use json; use header::TestProps; -use procsrv; use test::TestPaths; use util::logv; +use std::collections::HashMap; use std::collections::HashSet; use std::env; +use std::ffi::OsString; use std::fs::{self, File, create_dir_all}; use std::io::prelude::*; use std::io::{self, BufReader}; use std::path::{Path, PathBuf}; use std::process::{Command, Output, ExitStatus, Stdio}; use std::str; -use std::collections::HashMap; use extract_gdb_version; +/// The name of the environment variable that holds dynamic library locations. +pub fn dylib_env_var() -> &'static str { + if cfg!(windows) { + "PATH" + } else if cfg!(target_os = "macos") { + "DYLD_LIBRARY_PATH" + } else if cfg!(target_os = "haiku") { + "LIBRARY_PATH" + } else { + "LD_LIBRARY_PATH" + } +} + pub fn run(config: Config, testpaths: &TestPaths) { match &*config.target { @@ -1318,7 +1331,18 @@ actual:\n\ .stderr(Stdio::piped()) .stdin(Stdio::piped()); - procsrv::add_target_env(&mut command, lib_path, aux_path); + // Need to be sure to put both the lib_path and the aux path in the dylib + // search path for the child. + let mut path = env::split_paths(&env::var_os(dylib_env_var()).unwrap_or(OsString::new())) + .collect::>(); + if let Some(p) = aux_path { + path.insert(0, PathBuf::from(p)) + } + path.insert(0, PathBuf::from(lib_path)); + + // Add the new dylib search path var + let newpath = env::join_paths(&path).unwrap(); + command.env(dylib_env_var(), newpath); let mut child = command.spawn().expect(&format!("failed to exec `{:?}`", &command)); if let Some(input) = input { @@ -2077,7 +2101,7 @@ actual:\n\ .env("RUSTDOC", cwd.join(&self.config.rustdoc_path.as_ref().expect("--rustdoc-path passed"))) .env("TMPDIR", &tmpdir) - .env("LD_LIB_PATH_ENVVAR", procsrv::dylib_env_var()) + .env("LD_LIB_PATH_ENVVAR", dylib_env_var()) .env("HOST_RPATH_DIR", cwd.join(&self.config.compile_lib_path)) .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path)) .env("LLVM_COMPONENTS", &self.config.llvm_components) From 02e95e508df195becd939ae7f6ad70094a1117aa Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Tue, 22 Aug 2017 10:09:54 -0500 Subject: [PATCH 8/9] remove needless clone --- src/tools/compiletest/src/runtest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index af6b4c279318a..a68491b5bc73f 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1167,7 +1167,7 @@ actual:\n\ } fn exec_compiled_test(&self) -> ProcRes { - let env = self.props.exec_env.clone(); + let env = &self.props.exec_env; match &*self.config.target { // This is pretty similar to below, we're transforming: From 91bfe3f55bddf508624a39eec9da859f911f966c Mon Sep 17 00:00:00 2001 From: Andy Russell Date: Tue, 22 Aug 2017 10:14:43 -0500 Subject: [PATCH 9/9] capture `adb shell` stdout --- src/tools/compiletest/src/runtest.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a68491b5bc73f..d2a0c776b33e6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -515,6 +515,8 @@ actual:\n\ debug!("adb arg: {}", adb_arg); let mut adb = Command::new(adb_path) .args(&["shell", &adb_arg]) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) .spawn() .expect(&format!("failed to exec `{:?}`", adb_path));