Skip to content

Commit

Permalink
auto merge of #13954 : aturon/rust/issue-11650, r=alexcrichton
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
bors committed May 15, 2014
2 parents 6a2b3d1 + e71202a commit fbd8f4a
Show file tree
Hide file tree
Showing 30 changed files with 792 additions and 736 deletions.
20 changes: 3 additions & 17 deletions src/compiletest/procsrv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use std::os;
use std::str;
use std::io::process::{ProcessExit, Process, ProcessConfig, ProcessOutput};
use std::io::process::{ProcessExit, Command, Process, ProcessOutput};

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

let env = env.clone().append(target_env(lib_path, prog).as_slice());
let opt_process = Process::configure(ProcessConfig {
program: prog,
args: args,
env: Some(env.as_slice()),
.. ProcessConfig::new()
});

match opt_process {
match Command::new(prog).args(args).env(env.as_slice()).spawn() {
Ok(mut process) => {
for input in input.iter() {
process.stdin.get_mut_ref().write(input.as_bytes()).unwrap();
Expand All @@ -100,14 +93,7 @@ pub fn run_background(lib_path: &str,
input: Option<~str>) -> Option<Process> {

let env = env.clone().append(target_env(lib_path, prog).as_slice());
let opt_process = Process::configure(ProcessConfig {
program: prog,
args: args,
env: Some(env.as_slice()),
.. ProcessConfig::new()
});

match opt_process {
match Command::new(prog).args(args).env(env.as_slice()).spawn() {
Ok(mut process) => {
for input in input.iter() {
process.stdin.get_mut_ref().write(input.as_bytes()).unwrap();
Expand Down
30 changes: 9 additions & 21 deletions src/compiletest/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ fn run_debuginfo_gdb_test(config: &Config, props: &TestProps, testfile: &Path) {
}

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

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

fn run_lldb(config: &Config, test_executable: &Path, debugger_script: &Path) -> ProcRes {
// Prepare the lldb_batchmode which executes the debugger script
let lldb_batchmode_script = "./src/etc/lldb_batchmode.py".to_owned();
let test_executable_str = test_executable.as_str().unwrap().to_owned();
let debugger_script_str = debugger_script.as_str().unwrap().to_owned();
let commandline = format!("python {} {} {}",
lldb_batchmode_script.as_slice(),
test_executable_str.as_slice(),
debugger_script_str.as_slice());

let args = &[lldb_batchmode_script, test_executable_str, debugger_script_str];
let env = &[("PYTHONPATH".to_owned(), config.lldb_python_dir.clone().unwrap())];

let opt_process = Process::configure(ProcessConfig {
program: "python",
args: args,
env: Some(env),
.. ProcessConfig::new()
});

let (status, out, err) = match opt_process {
let mut cmd = Command::new("python");
cmd.arg("./src/etc/lldb_batchmode.py")
.arg(test_executable)
.arg(debugger_script)
.env([("PYTHONPATH", config.lldb_python_dir.clone().unwrap().as_slice())]);

let (status, out, err) = match cmd.spawn() {
Ok(process) => {
let ProcessOutput { status, output, error } =
process.wait_with_output().unwrap();
Expand All @@ -520,7 +508,7 @@ fn run_debuginfo_lldb_test(config: &Config, props: &TestProps, testfile: &Path)
status: status,
stdout: out,
stderr: err,
cmdline: commandline
cmdline: format!("{}", cmd)
};
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/libnative/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ use std::c_str::CString;
use std::io;
use std::io::IoError;
use std::io::net::ip::SocketAddr;
use std::io::process::ProcessConfig;
use std::io::signal::Signum;
use std::os;
use std::rt::rtio;
use std::rt::rtio::{RtioTcpStream, RtioTcpListener, RtioUdpSocket};
use std::rt::rtio::{RtioUnixListener, RtioPipe, RtioFileStream, RtioProcess};
use std::rt::rtio::{RtioSignal, RtioTTY, CloseBehavior, RtioTimer};
use std::rt::rtio::{RtioSignal, RtioTTY, CloseBehavior, RtioTimer, ProcessConfig};
use ai = std::io::net::addrinfo;

// Local re-exports
Expand Down Expand Up @@ -258,10 +257,10 @@ impl rtio::IoFactory for IoFactory {
fn timer_init(&mut self) -> IoResult<Box<RtioTimer:Send>> {
timer::Timer::new().map(|t| box t as Box<RtioTimer:Send>)
}
fn spawn(&mut self, config: ProcessConfig)
fn spawn(&mut self, cfg: ProcessConfig)
-> IoResult<(Box<RtioProcess:Send>,
Vec<Option<Box<RtioPipe:Send>>>)> {
process::Process::spawn(config).map(|(p, io)| {
process::Process::spawn(cfg).map(|(p, io)| {
(box p as Box<RtioProcess:Send>,
io.move_iter().map(|p| p.map(|p| {
box p as Box<RtioPipe:Send>
Expand Down
Loading

0 comments on commit fbd8f4a

Please sign in to comment.