Skip to content

Commit fbd8f4a

Browse files
committed
auto merge of #13954 : aturon/rust/issue-11650, r=alexcrichton
## Process API The existing APIs for spawning processes took strings for the command and arguments, but the underlying system may not impose utf8 encoding, so this is overly limiting. The assumption we actually want to make is just that the command and arguments are viewable as [u8] slices with no interior NULLs, i.e., as CStrings. The ToCStr trait is a handy bound for types that meet this requirement (such as &str and Path). However, since the commands and arguments are often a mixture of strings and paths, it would be inconvenient to take a slice with a single T: ToCStr bound. So this patch revamps the process creation API to instead use a builder-style interface, called `Command`, allowing arguments to be added one at a time with differing ToCStr implementations for each. The initial cut of the builder API has some drawbacks that can be addressed once issue #13851 (libstd as a facade) is closed. These are detailed as FIXMEs. ## Dynamic library API `std::unstable::dynamic_library::open_external` currently takes a `Path`, but because `Paths` produce normalized strings, this can change the semantics of lookups in a given environment. This patch generalizes the function to take a `ToCStr`-bounded type, which includes both `Path`s and `str`s. ## ToCStr API Adds ToCStr impl for &Path and ~str. This is a stopgap until DST (#12938) lands. Until DST lands, we cannot decompose &str into & and str, so we cannot usefully take ToCStr arguments by reference (without forcing an additional & around &str). So we are instead temporarily adding an instance for &Path and ~str, so that we can take ToCStr as owned. When DST lands, the &Path instance should be removed, the string instances should be revisted, and arguments bound by ToCStr should be passed by reference. FIXMEs have been added accordingly. ## Tickets closed Closes #11650. Closes #7928.
2 parents 6a2b3d1 + e71202a commit fbd8f4a

30 files changed

+792
-736
lines changed

src/compiletest/procsrv.rs

+3-17
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
use std::os;
1212
use std::str;
13-
use std::io::process::{ProcessExit, Process, ProcessConfig, ProcessOutput};
13+
use std::io::process::{ProcessExit, Command, Process, ProcessOutput};
1414

1515
#[cfg(target_os = "win32")]
1616
fn target_env(lib_path: &str, prog: &str) -> Vec<(~str, ~str)> {
@@ -68,14 +68,7 @@ pub fn run(lib_path: &str,
6868
input: Option<~str>) -> Option<Result> {
6969

7070
let env = env.clone().append(target_env(lib_path, prog).as_slice());
71-
let opt_process = Process::configure(ProcessConfig {
72-
program: prog,
73-
args: args,
74-
env: Some(env.as_slice()),
75-
.. ProcessConfig::new()
76-
});
77-
78-
match opt_process {
71+
match Command::new(prog).args(args).env(env.as_slice()).spawn() {
7972
Ok(mut process) => {
8073
for input in input.iter() {
8174
process.stdin.get_mut_ref().write(input.as_bytes()).unwrap();
@@ -100,14 +93,7 @@ pub fn run_background(lib_path: &str,
10093
input: Option<~str>) -> Option<Process> {
10194

10295
let env = env.clone().append(target_env(lib_path, prog).as_slice());
103-
let opt_process = Process::configure(ProcessConfig {
104-
program: prog,
105-
args: args,
106-
env: Some(env.as_slice()),
107-
.. ProcessConfig::new()
108-
});
109-
110-
match opt_process {
96+
match Command::new(prog).args(args).env(env.as_slice()).spawn() {
11197
Ok(mut process) => {
11298
for input in input.iter() {
11399
process.stdin.get_mut_ref().write(input.as_bytes()).unwrap();

src/compiletest/runtest.rs

+9-21
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ fn run_debuginfo_gdb_test(config: &Config, props: &TestProps, testfile: &Path) {
420420
}
421421

422422
fn run_debuginfo_lldb_test(config: &Config, props: &TestProps, testfile: &Path) {
423-
use std::io::process::{Process, ProcessConfig, ProcessOutput};
423+
use std::io::process::{Command, ProcessOutput};
424424

425425
if config.lldb_python_dir.is_none() {
426426
fatal("Can't run LLDB test because LLDB's python path is not set.".to_owned());
@@ -483,25 +483,13 @@ fn run_debuginfo_lldb_test(config: &Config, props: &TestProps, testfile: &Path)
483483

484484
fn run_lldb(config: &Config, test_executable: &Path, debugger_script: &Path) -> ProcRes {
485485
// Prepare the lldb_batchmode which executes the debugger script
486-
let lldb_batchmode_script = "./src/etc/lldb_batchmode.py".to_owned();
487-
let test_executable_str = test_executable.as_str().unwrap().to_owned();
488-
let debugger_script_str = debugger_script.as_str().unwrap().to_owned();
489-
let commandline = format!("python {} {} {}",
490-
lldb_batchmode_script.as_slice(),
491-
test_executable_str.as_slice(),
492-
debugger_script_str.as_slice());
493-
494-
let args = &[lldb_batchmode_script, test_executable_str, debugger_script_str];
495-
let env = &[("PYTHONPATH".to_owned(), config.lldb_python_dir.clone().unwrap())];
496-
497-
let opt_process = Process::configure(ProcessConfig {
498-
program: "python",
499-
args: args,
500-
env: Some(env),
501-
.. ProcessConfig::new()
502-
});
503-
504-
let (status, out, err) = match opt_process {
486+
let mut cmd = Command::new("python");
487+
cmd.arg("./src/etc/lldb_batchmode.py")
488+
.arg(test_executable)
489+
.arg(debugger_script)
490+
.env([("PYTHONPATH", config.lldb_python_dir.clone().unwrap().as_slice())]);
491+
492+
let (status, out, err) = match cmd.spawn() {
505493
Ok(process) => {
506494
let ProcessOutput { status, output, error } =
507495
process.wait_with_output().unwrap();
@@ -520,7 +508,7 @@ fn run_debuginfo_lldb_test(config: &Config, props: &TestProps, testfile: &Path)
520508
status: status,
521509
stdout: out,
522510
stderr: err,
523-
cmdline: commandline
511+
cmdline: format!("{}", cmd)
524512
};
525513
}
526514
}

src/libnative/io/mod.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,12 @@ use std::c_str::CString;
2727
use std::io;
2828
use std::io::IoError;
2929
use std::io::net::ip::SocketAddr;
30-
use std::io::process::ProcessConfig;
3130
use std::io::signal::Signum;
3231
use std::os;
3332
use std::rt::rtio;
3433
use std::rt::rtio::{RtioTcpStream, RtioTcpListener, RtioUdpSocket};
3534
use std::rt::rtio::{RtioUnixListener, RtioPipe, RtioFileStream, RtioProcess};
36-
use std::rt::rtio::{RtioSignal, RtioTTY, CloseBehavior, RtioTimer};
35+
use std::rt::rtio::{RtioSignal, RtioTTY, CloseBehavior, RtioTimer, ProcessConfig};
3736
use ai = std::io::net::addrinfo;
3837

3938
// Local re-exports
@@ -258,10 +257,10 @@ impl rtio::IoFactory for IoFactory {
258257
fn timer_init(&mut self) -> IoResult<Box<RtioTimer:Send>> {
259258
timer::Timer::new().map(|t| box t as Box<RtioTimer:Send>)
260259
}
261-
fn spawn(&mut self, config: ProcessConfig)
260+
fn spawn(&mut self, cfg: ProcessConfig)
262261
-> IoResult<(Box<RtioProcess:Send>,
263262
Vec<Option<Box<RtioPipe:Send>>>)> {
264-
process::Process::spawn(config).map(|(p, io)| {
263+
process::Process::spawn(cfg).map(|(p, io)| {
265264
(box p as Box<RtioProcess:Send>,
266265
io.move_iter().map(|p| p.map(|p| {
267266
box p as Box<RtioPipe:Send>

0 commit comments

Comments
 (0)