Skip to content

Commit

Permalink
Fix #10754 - std::run functions fail after io_error
Browse files Browse the repository at this point in the history
The problem was that std::run::Process::new() was unwrap()ing the result
of std::io::process::Process::new(), which returns None in the case
where the io_error condition is raised to signal failure to start the
process.

Have std::run::Process::new() similarly return an Option<run::Process>
to reflect the fact that a subprocess might have failed to start. Update
utility functions run::process_status() and run::process_output() to
return Option<ProcessExit> and Option<ProcessOutput>, respectively.

Various parts of librustc and librustpkg needed to be updated to reflect
these API changes.

closes #10754
  • Loading branch information
cadencemarseille committed Dec 14, 2013
1 parent 00b1adf commit 5de4270
Show file tree
Hide file tree
Showing 12 changed files with 275 additions and 153 deletions.
42 changes: 26 additions & 16 deletions src/compiletest/procsrv.rs
Expand Up @@ -46,47 +46,57 @@ pub fn run(lib_path: &str,
prog: &str,
args: &[~str],
env: ~[(~str, ~str)],
input: Option<~str>) -> Result {
input: Option<~str>) -> Option<Result> {

let env = env + target_env(lib_path, prog);
let mut process = run::Process::new(prog, args, run::ProcessOptions {
let mut opt_process = run::Process::new(prog, args, run::ProcessOptions {
env: Some(env),
dir: None,
in_fd: None,
out_fd: None,
err_fd: None
});

for input in input.iter() {
process.input().write(input.as_bytes());
}
let run::ProcessOutput { status, output, error } = process.finish_with_output();
match opt_process {
Some(ref mut process) => {
for input in input.iter() {
process.input().write(input.as_bytes());
}
let run::ProcessOutput { status, output, error } = process.finish_with_output();

Result {
status: status,
out: str::from_utf8_owned(output),
err: str::from_utf8_owned(error)
Some(Result {
status: status,
out: str::from_utf8_owned(output),
err: str::from_utf8_owned(error)
})
},
None => None
}
}

pub fn run_background(lib_path: &str,
prog: &str,
args: &[~str],
env: ~[(~str, ~str)],
input: Option<~str>) -> run::Process {
input: Option<~str>) -> Option<run::Process> {

let env = env + target_env(lib_path, prog);
let mut process = run::Process::new(prog, args, run::ProcessOptions {
let opt_process = run::Process::new(prog, args, run::ProcessOptions {
env: Some(env),
dir: None,
in_fd: None,
out_fd: None,
err_fd: None
});

for input in input.iter() {
process.input().write(input.as_bytes());
}
match opt_process {
Some(mut process) => {
for input in input.iter() {
process.input().write(input.as_bytes());
}

return process;
Some(process)
},
None => None
}
}
40 changes: 26 additions & 14 deletions src/compiletest/runtest.rs
Expand Up @@ -289,20 +289,23 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
dump_output_file(config, testfile, script_str, "debugger.script");


procsrv::run("", config.adb_path.clone(),
procsrv::run("", config.adb_path,
[~"push", exe_file.as_str().unwrap().to_owned(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

procsrv::run("", config.adb_path,
[~"forward", ~"tcp:5039", ~"tcp:5039"],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

let adb_arg = format!("export LD_LIBRARY_PATH={}; gdbserver :5039 {}/{}",
config.adb_test_dir.clone(), config.adb_test_dir.clone(),
str::from_utf8(exe_file.filename().unwrap()));

let mut process = procsrv::run_background("", config.adb_path.clone(),
[~"shell",adb_arg.clone()],~[(~"",~"")], Some(~""));
let mut process = procsrv::run_background("", config.adb_path,
[~"shell",adb_arg.clone()],~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));
loop {
//waiting 1 second for gdbserver start
timer::sleep(1000);
Expand Down Expand Up @@ -334,10 +337,12 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
let debugger_opts = ~[~"-quiet", ~"-batch", ~"-nx",
"-command=" + debugger_script.as_str().unwrap().to_owned()];

let gdb_path = tool_path.append("/bin/arm-linux-androideabi-gdb");
let procsrv::Result{ out, err, status }=
procsrv::run("",
tool_path.append("/bin/arm-linux-androideabi-gdb"),
debugger_opts, ~[(~"",~"")], None);
gdb_path,
debugger_opts, ~[(~"",~"")], None)
.expect(format!("failed to exec `{}`", gdb_path));
let cmdline = {
let cmdline = make_cmdline("", "arm-linux-androideabi-gdb", debugger_opts);
logv(config, format!("executing {}", cmdline));
Expand Down Expand Up @@ -800,7 +805,8 @@ fn program_output(config: &config, testfile: &Path, lib_path: &str, prog: ~str,
cmdline
};
let procsrv::Result{ out, err, status } =
procsrv::run(lib_path, prog, args, env, input);
procsrv::run(lib_path, prog, args, env, input)
.expect(format!("failed to exec `{}`", prog));
dump_output(config, testfile, out, err);
return ProcRes {status: status,
stdout: out,
Expand Down Expand Up @@ -908,7 +914,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
// copy to target
let copy_result = procsrv::run("", config.adb_path,
[~"push", args.prog.clone(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

if config.verbose {
println!("push ({}) {} {} {}",
Expand All @@ -932,7 +939,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
for tv in args.args.iter() {
runargs.push(tv.to_owned());
}
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

// get exitcode of result
runargs = ~[];
Expand All @@ -942,7 +950,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,

let procsrv::Result{ out: exitcode_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")],
Some(~""));
Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

let mut exitcode : int = 0;
for c in exitcode_out.chars() {
Expand All @@ -960,7 +969,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
runargs.push(format!("{}/{}.stdout", config.adb_test_dir, prog_short));

let procsrv::Result{ out: stdout_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

// get stderr of result
runargs = ~[];
Expand All @@ -969,7 +979,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
runargs.push(format!("{}/{}.stderr", config.adb_test_dir, prog_short));

let procsrv::Result{ out: stderr_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

dump_output(config, testfile, stdout_out, stderr_out);

Expand Down Expand Up @@ -1004,7 +1015,8 @@ fn _arm_push_aux_shared_library(config: &config, testfile: &Path) {
// FIXME (#9639): This needs to handle non-utf8 paths
let copy_result = procsrv::run("", config.adb_path,
[~"push", file.as_str().unwrap().to_owned(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

if config.verbose {
println!("push ({}) {} {} {}",
Expand Down
26 changes: 18 additions & 8 deletions src/librustc/back/archive.rs
Expand Up @@ -40,15 +40,25 @@ fn run_ar(sess: Session, args: &str, cwd: Option<&Path>,
Some(p) => { debug!("inside {}", p.display()); }
None => {}
}
let o = Process::new(ar, args.as_slice(), opts).finish_with_output();
if !o.status.success() {
sess.err(format!("{} {} failed with: {}", ar, args.connect(" "),
o.status));
sess.note(format!("stdout ---\n{}", str::from_utf8(o.output)));
sess.note(format!("stderr ---\n{}", str::from_utf8(o.error)));
sess.abort_if_errors();
let mut opt_prog = Process::new(ar, args.as_slice(), opts);
match opt_prog {
Some(ref mut prog) => {
let o = prog.finish_with_output();
if !o.status.success() {
sess.err(format!("{} {} failed with: {}", ar, args.connect(" "),
o.status));
sess.note(format!("stdout ---\n{}", str::from_utf8(o.output)));
sess.note(format!("stderr ---\n{}", str::from_utf8(o.error)));
sess.abort_if_errors();
}
o
},
None => {
sess.err(format!("could not exec `{}`", ar));
sess.abort_if_errors();
fail!("rustc::back::archive::run_ar() should not reach this point");
}
}
o
}

impl Archive {
Expand Down
44 changes: 29 additions & 15 deletions src/librustc/back/link.rs
Expand Up @@ -310,13 +310,19 @@ pub mod write {
assembly.as_str().unwrap().to_owned()];

debug!("{} '{}'", cc, args.connect("' '"));
let prog = run::process_output(cc, args);

if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc, prog.status));
sess.note(format!("{} arguments: '{}'", cc, args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
match run::process_output(cc, args) {
Some(prog) => {
if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc, prog.status));
sess.note(format!("{} arguments: '{}'", cc, args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
}
},
None => {
sess.err(format!("could not exec `{}`", cc));
sess.abort_if_errors();
}
}
}

Expand Down Expand Up @@ -949,14 +955,22 @@ fn link_natively(sess: Session, dylib: bool, obj_filename: &Path,

// Invoke the system linker
debug!("{} {}", cc_prog, cc_args.connect(" "));
let prog = time(sess.time_passes(), "running linker", (), |()|
run::process_output(cc_prog, cc_args));

if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
sess.note(format!("{} arguments: '{}'", cc_prog, cc_args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
let opt_prog = time(sess.time_passes(), "running linker", (), |()|
run::process_output(cc_prog, cc_args));

match opt_prog {
Some(prog) => {
if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
sess.note(format!("{} arguments: '{}'", cc_prog, cc_args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
}
},
None => {
sess.err(format!("could not exec `{}`", cc_prog));
sess.abort_if_errors();
}
}


Expand Down
28 changes: 16 additions & 12 deletions src/librustpkg/api.rs
Expand Up @@ -142,14 +142,14 @@ pub fn install_pkg(cx: &BuildContext,
/// Builds an arbitrary library whose short name is `output`,
/// by invoking `tool` with arguments `args` plus "-o %s", where %s
/// is the platform-specific library name for `output`.
/// Returns that platform-specific name.
/// Returns that platform-specific name, or None if `tool` could not be started.
pub fn build_library_in_workspace(exec: &mut workcache::Exec,
context: &mut Context,
package_name: &str,
tool: &str,
flags: &[~str],
paths: &[~str],
output: &str) -> ~str {
output: &str) -> Option<~str> {
use command_failed = conditions::command_failed::cond;

let workspace = my_workspace(context, package_name);
Expand All @@ -169,16 +169,20 @@ pub fn build_library_in_workspace(exec: &mut workcache::Exec,

let all_args = flags + absolute_paths + cc_args +
~[~"-o", out_name.as_str().unwrap().to_owned()];
let exit_process = run::process_status(tool, all_args);
if exit_process.success() {
let out_name_str = out_name.as_str().unwrap().to_owned();
exec.discover_output("binary",
out_name_str,
digest_only_date(&out_name));
context.add_library_path(out_name.dir_path());
out_name_str
} else {
command_failed.raise((tool.to_owned(), all_args, exit_process))
match run::process_status(tool, all_args) {
Some(exit_process) => {
if exit_process.success() {
let out_name_str = out_name.as_str().unwrap().to_owned();
exec.discover_output("binary",
out_name_str,
digest_only_date(&out_name));
context.add_library_path(out_name.dir_path());
Some(out_name_str)
} else {
Some(command_failed.raise((tool.to_owned(), all_args, exit_process)))
}
},
None => None
}
}

Expand Down

5 comments on commit 5de4270

@bors
Copy link
Contributor

@bors bors commented on 5de4270 Dec 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at cadencemarseille@5de4270

@bors
Copy link
Contributor

@bors bors commented on 5de4270 Dec 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging cadencemarseille/rust/issue-10754-std-run-unwrap-on-None = 5de4270 into auto

@bors
Copy link
Contributor

@bors bors commented on 5de4270 Dec 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cadencemarseille/rust/issue-10754-std-run-unwrap-on-None = 5de4270 merged ok, testing candidate = aafed3e

@bors
Copy link
Contributor

@bors bors commented on 5de4270 Dec 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 5de4270 Dec 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = aafed3e

Please sign in to comment.