auto merge of #10936 : cadencemarseille/rust/issue-10754-std-run-unwrap-on-None, r=alexcrichton

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

Author: bors

src/compiletest/procsrv.rs

@@ -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
+    }
 }

src/compiletest/runtest.rs

@@ -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);
@@ -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));
@@ -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,
@@ -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 ({}) {} {} {}",
@@ -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 = ~[];
@@ -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() {
@@ -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 = ~[];
@@ -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);
 
@@ -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 ({}) {} {} {}",

src/librustc/back/archive.rs

@@ -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 {

src/librustc/back/link.rs

@@ -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();
+            }
         }
     }
 
@@ -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();
+        }
     }
 
 

src/librustpkg/api.rs

@@ -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);
@@ -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
     }
 }