auto merge of #6433 : Dretch/rust/run-refactor, r=thestinger

...mentioned in #2625.

This change makes the module more oriented around
Process values instead of having to deal with process ids
directly.

Apart from issues mentioned in #2625, other changes include:
- Changing the naming to be more consistent - Process/process
  is now used instead of a mixture of Program/program and
  Process/process.
- More docs/tests.

Some io/scheduler related issues remain (mentioned in #2625). I am not sure how best to address these.

src/compiletest/procsrv.rs

@@ -10,8 +10,6 @@
 
 use core::prelude::*;
 
-use core::libc::c_int;
-use core::run::spawn_process;
 use core::run;
 
 #[cfg(target_os = "win32")]
@@ -38,86 +36,35 @@ fn target_env(lib_path: &str, prog: &str) -> ~[(~str,~str)] {
 #[cfg(target_os = "macos")]
 #[cfg(target_os = "freebsd")]
 fn target_env(_lib_path: &str, _prog: &str) -> ~[(~str,~str)] {
-    ~[]
+    os::env()
 }
 
 pub struct Result {status: int, out: ~str, err: ~str}
 
-// FIXME (#2659): This code is duplicated in core::run::program_output
 pub fn run(lib_path: &str,
            prog: &str,
            args: &[~str],
            env: ~[(~str, ~str)],
            input: Option<~str>) -> Result {
-    let pipe_in = os::pipe();
-    let pipe_out = os::pipe();
-    let pipe_err = os::pipe();
-    let pid = spawn_process(prog, args,
-                            &Some(env + target_env(lib_path, prog)),
-                            &None, pipe_in.in, pipe_out.out, pipe_err.out);
-
-    os::close(pipe_in.in);
-    os::close(pipe_out.out);
-    os::close(pipe_err.out);
-    if pid == -1i32 {
-        os::close(pipe_in.out);
-        os::close(pipe_out.in);
-        os::close(pipe_err.in);
-        fail!();
-    }
 
+    let env = env + target_env(lib_path, prog);
+    let mut proc = run::Process::new(prog, args, run::ProcessOptions {
+        env: Some(env.slice(0, env.len())),
+        dir: None,
+        in_fd: None,
+        out_fd: None,
+        err_fd: None
+    });
 
-    writeclose(pipe_in.out, input);
-    let p = comm::PortSet::new();
-    let ch = p.chan();
-    do task::spawn_sched(task::SingleThreaded) || {
-        let errput = readclose(pipe_err.in);
-        ch.send((2, errput));
+    for input.each |input| {
+        proc.input().write_str(*input);
     }
-    let ch = p.chan();
-    do task::spawn_sched(task::SingleThreaded) || {
-        let output = readclose(pipe_out.in);
-        ch.send((1, output));
-    }
-    let status = run::waitpid(pid);
-    let mut errs = ~"";
-    let mut outs = ~"";
-    let mut count = 2;
-    while count > 0 {
-        match p.recv() {
-          (1, s) => {
-            outs = s;
-          }
-          (2, s) => {
-            errs = s;
-          }
-          _ => { fail!() }
-        };
-        count -= 1;
-    };
-    return Result {status: status, out: outs, err: errs};
-}
+    let output = proc.finish_with_output();
 
-fn writeclose(fd: c_int, s: Option<~str>) {
-    if s.is_some() {
-        let writer = io::fd_writer(fd, false);
-        writer.write_str(s.get());
+    Result {
+        status: output.status,
+        out: str::from_bytes(output.output),
+        err: str::from_bytes(output.error)
     }
-
-    os::close(fd);
 }
 
-fn readclose(fd: c_int) -> ~str {
-    unsafe {
-        // Copied from run::program_output
-        let file = os::fdopen(fd);
-        let reader = io::FILE_reader(file, false);
-        let mut buf = ~"";
-        while !reader.eof() {
-            let bytes = reader.read_bytes(4096u);
-            str::push_str(&mut buf, str::from_bytes(bytes));
-        }
-        os::fclose(file);
-        return buf;
-    }
-}

src/libextra/workcache.rs

@@ -415,7 +415,7 @@ fn test() {
         prep.declare_input("file", pth.to_str(), digest_file(&pth));
         do prep.exec |_exe| {
             let out = Path("foo.o");
-            run::run_program("gcc", [~"foo.c", ~"-o", out.to_str()]);
+            run::process_status("gcc", [~"foo.c", ~"-o", out.to_str()]);
             out.to_str()
         }
     };

src/libfuzzer/fuzzer.rc

@@ -412,20 +412,20 @@ pub fn check_whole_compiler(code: &str,
 pub fn removeIfExists(filename: &Path) {
     // So sketchy!
     assert!(!contains(filename.to_str(), " "));
-    run::program_output("bash", [~"-c", ~"rm " + filename.to_str()]);
+    run::process_status("bash", [~"-c", ~"rm " + filename.to_str()]);
 }
 
 pub fn removeDirIfExists(filename: &Path) {
     // So sketchy!
     assert!(!contains(filename.to_str(), " "));
-    run::program_output("bash", [~"-c", ~"rm -r " + filename.to_str()]);
+    run::process_status("bash", [~"-c", ~"rm -r " + filename.to_str()]);
 }
 
 pub fn check_running(exe_filename: &Path) -> happiness {
-    let p = run::program_output(
+    let p = run::process_output(
         "/Users/jruderman/scripts/timed_run_rust_program.py",
         [exe_filename.to_str()]);
-    let comb = p.out + ~"\n" + p.err;
+    let comb = str::from_bytes(p.output) + ~"\n" + str::from_bytes(p.error);
     if str::len(comb) > 1u {
         error!("comb comb comb: %?", comb);
     }
@@ -461,33 +461,35 @@ pub fn check_running(exe_filename: &Path) -> happiness {
 }
 
 pub fn check_compiling(filename: &Path) -> happiness {
-    let p = run::program_output(
-        "/Users/jruderman/code/rust/build/x86_64-apple-darwin/\
-         stage1/bin/rustc",
+    let p = run::process_output(
+        "/Users/jruderman/code/rust/build/x86_64-apple-darwin/stage1/bin/rustc",
         [filename.to_str()]);
 
+    let out = str::from_bytes(p.output);
+    let err = str::from_bytes(p.error);
+
     //error!("Status: %d", p.status);
     if p.status == 0 {
         passed
-    } else if p.err != ~"" {
-        if contains(p.err, "error:") {
+    } else if !err.is_empty() {
+        if err.contains("error:") {
             cleanly_rejected(~"rejected with span_error")
         } else {
-            error!("Stderr: %?", p.err);
+            error!("Stderr: %?", err);
             failed(~"Unfamiliar error message")
         }
-    } else if contains(p.out, "Assertion") && contains(p.out, "failed") {
-        error!("Stdout: %?", p.out);
+    } else if out.contains("Assertion") && out.contains("failed") {
+        error!("Stdout: %?", out);
         failed(~"Looks like an llvm assertion failure")
-    } else if contains(p.out, "internal compiler error unimplemented") {
+    } else if out.contains("internal compiler error unimplemented") {
         known_bug(~"Something unimplemented")
-    } else if contains(p.out, "internal compiler error") {
-        error!("Stdout: %?", p.out);
+    } else if out.contains("internal compiler error") {
+        error!("Stdout: %?", out);
         failed(~"internal compiler error")
 
     } else {
         error!("%?", p.status);
-        error!("!Stdout: %?", p.out);
+        error!("!Stdout: %?", out);
         failed(~"What happened?")
     }
 }
@@ -608,9 +610,7 @@ pub fn check_roundtrip_convergence(code: @~str, maxIters: uint) {
         error!("Did not converge after %u iterations!", i);
         write_file(&Path("round-trip-a.rs"), *oldv);
         write_file(&Path("round-trip-b.rs"), *newv);
-        run::run_program("diff",
-                         [~"-w", ~"-u", ~"round-trip-a.rs",
-                          ~"round-trip-b.rs"]);
+        run::process_status("diff", [~"-w", ~"-u", ~"round-trip-a.rs", ~"round-trip-b.rs"]);
         fail!("Mismatch");
     }
 }

src/librust/rust.rc

@@ -163,7 +163,7 @@ fn cmd_test(args: &[~str]) -> ValidUsage {
             let test_exec = Path(filename).filestem().unwrap() + "test~";
             invoke("rustc", &[~"--test", filename.to_owned(),
                               ~"-o", test_exec.to_owned()], rustc::main);
-            let exit_code = run::run_program(~"./" + test_exec, []);
+            let exit_code = run::process_status(~"./" + test_exec, []);
             Valid(exit_code)
         }
         _ => Invalid
@@ -176,7 +176,7 @@ fn cmd_run(args: &[~str]) -> ValidUsage {
             let exec = Path(filename).filestem().unwrap() + "~";
             invoke("rustc", &[filename.to_owned(), ~"-o", exec.to_owned()],
                    rustc::main);
-            let exit_code = run::run_program(~"./"+exec, prog_args);
+            let exit_code = run::process_status(~"./"+exec, prog_args);
             Valid(exit_code)
         }
         _ => Invalid

src/librustc/back/link.rs

@@ -403,14 +403,14 @@ pub fn run_ndk(sess: Session, assembly: &Path, object: &Path) {
         cc_args.push(object.to_str());
         cc_args.push(assembly.to_str());
 
-        let prog = run::program_output(cc_prog, cc_args);
+        let prog = run::process_output(cc_prog, cc_args);
 
         if prog.status != 0 {
             sess.err(fmt!("building with `%s` failed with code %d",
                         cc_prog, prog.status));
             sess.note(fmt!("%s arguments: %s",
                         cc_prog, str::connect(cc_args, " ")));
-            sess.note(prog.err + prog.out);
+            sess.note(str::from_bytes(prog.error + prog.output));
             sess.abort_if_errors();
         }
     }
@@ -817,19 +817,19 @@ pub fn link_binary(sess: Session,
     let cc_args = link_args(sess, obj_filename, out_filename, lm);
     debug!("%s link args: %s", cc_prog, str::connect(cc_args, " "));
     // We run 'cc' here
-    let prog = run::program_output(cc_prog, cc_args);
+    let prog = run::process_output(cc_prog, cc_args);
     if 0 != prog.status {
         sess.err(fmt!("linking with `%s` failed with code %d",
                       cc_prog, prog.status));
         sess.note(fmt!("%s arguments: %s",
                        cc_prog, str::connect(cc_args, " ")));
-        sess.note(prog.err + prog.out);
+        sess.note(str::from_bytes(prog.error + prog.output));
         sess.abort_if_errors();
     }
 
     // Clean up on Darwin
     if sess.targ_cfg.os == session::os_macos {
-        run::run_program("dsymutil", [output.to_str()]);
+        run::process_status("dsymutil", [output.to_str()]);
     }
 
     // Remove the temporary object file if we aren't saving temps

src/librustdoc/config.rs

@@ -12,7 +12,7 @@
 
 use core::cell::Cell;
 use core::run;
-use core::run::ProgramOutput;
+use core::run::ProcessOutput;
 use core::result::Result;
 use extra::getopts;
 
@@ -89,27 +89,27 @@ pub fn default_config(input_crate: &Path) -> Config {
     }
 }
 
-type Process = ~fn((&str), (&[~str])) -> ProgramOutput;
+type Process = ~fn((&str), (&[~str])) -> ProcessOutput;
 
-pub fn mock_program_output(_prog: &str, _args: &[~str]) -> ProgramOutput {
-    ProgramOutput {
+pub fn mock_process_output(_prog: &str, _args: &[~str]) -> ProcessOutput {
+    ProcessOutput {
         status: 0,
-        out: ~"",
-        err: ~""
+        output: ~[],
+        error: ~[]
     }
 }
 
-pub fn program_output(prog: &str, args: &[~str]) -> ProgramOutput {
-    run::program_output(prog, args)
+pub fn process_output(prog: &str, args: &[~str]) -> ProcessOutput {
+    run::process_output(prog, args)
 }
 
 pub fn parse_config(args: &[~str]) -> Result<Config, ~str> {
-    parse_config_(args, program_output)
+    parse_config_(args, process_output)
 }
 
 pub fn parse_config_(
     args: &[~str],
-    program_output: Process
+    process_output: Process
 ) -> Result<Config, ~str> {
     let args = args.tail();
     let opts = vec::unzip(opts()).first();
@@ -117,7 +117,7 @@ pub fn parse_config_(
         Ok(matches) => {
             if matches.free.len() == 1 {
                 let input_crate = Path(*matches.free.head());
-                config_from_opts(&input_crate, &matches, program_output)
+                config_from_opts(&input_crate, &matches, process_output)
             } else if matches.free.is_empty() {
                 Err(~"no crates specified")
             } else {
@@ -133,7 +133,7 @@ pub fn parse_config_(
 fn config_from_opts(
     input_crate: &Path,
     matches: &getopts::Matches,
-    program_output: Process
+    process_output: Process
 ) -> Result<Config, ~str> {
 
     let config = default_config(input_crate);
@@ -175,11 +175,11 @@ fn config_from_opts(
             }
         }
     };
-    let program_output = Cell(program_output);
+    let process_output = Cell(process_output);
     let result = do result::chain(result) |config| {
         let pandoc_cmd = getopts::opt_maybe_str(matches, opt_pandoc_cmd());
         let pandoc_cmd = maybe_find_pandoc(
-            &config, pandoc_cmd, program_output.take());
+            &config, pandoc_cmd, process_output.take());
         do result::chain(pandoc_cmd) |pandoc_cmd| {
             result::Ok(Config {
                 pandoc_cmd: pandoc_cmd,
@@ -209,7 +209,7 @@ fn parse_output_style(output_style: &str) -> Result<OutputStyle, ~str> {
 pub fn maybe_find_pandoc(
     config: &Config,
     maybe_pandoc_cmd: Option<~str>,
-    program_output: Process
+    process_output: Process
 ) -> Result<Option<~str>, ~str> {
     if config.output_format != PandocHtml {
         return result::Ok(maybe_pandoc_cmd);
@@ -228,7 +228,7 @@ pub fn maybe_find_pandoc(
     };
 
     let pandoc = do vec::find(possible_pandocs) |pandoc| {
-        let output = program_output(*pandoc, [~"--version"]);
+        let output = process_output(*pandoc, [~"--version"]);
         debug!("testing pandoc cmd %s: %?", *pandoc, output);
         output.status == 0
     };
@@ -244,10 +244,10 @@ pub fn maybe_find_pandoc(
 mod test {
     use core::prelude::*;
     use config::*;
-    use core::run::ProgramOutput;
+    use core::run::ProcessOutput;
 
     fn parse_config(args: &[~str]) -> Result<Config, ~str> {
-        parse_config_(args, mock_program_output)
+        parse_config_(args, mock_process_output)
     }
 
     #[test]
@@ -256,10 +256,10 @@ fn should_find_pandoc() {
             output_format: PandocHtml,
             .. default_config(&Path("test"))
         };
-        let mock_program_output: ~fn(&str, &[~str]) -> ProgramOutput = |_, _| {
-            ProgramOutput { status: 0, out: ~"pandoc 1.8.2.1", err: ~"" }
+        let mock_process_output: ~fn(&str, &[~str]) -> ProcessOutput = |_, _| {
+            ProcessOutput { status: 0, output: "pandoc 1.8.2.1".to_bytes(), error: ~[] }
         };
-        let result = maybe_find_pandoc(&config, None, mock_program_output);
+        let result = maybe_find_pandoc(&config, None, mock_process_output);
         assert!(result == result::Ok(Some(~"pandoc")));
     }
 
@@ -269,10 +269,10 @@ fn should_error_with_no_pandoc() {
             output_format: PandocHtml,
             .. default_config(&Path("test"))
         };
-        let mock_program_output: ~fn(&str, &[~str]) -> ProgramOutput = |_, _| {
-            ProgramOutput { status: 1, out: ~"", err: ~"" }
+        let mock_process_output: ~fn(&str, &[~str]) -> ProcessOutput = |_, _| {
+            ProcessOutput { status: 1, output: ~[], error: ~[] }
         };
-        let result = maybe_find_pandoc(&config, None, mock_program_output);
+        let result = maybe_find_pandoc(&config, None, mock_process_output);
         assert!(result == result::Err(~"couldn't find pandoc"));
     }
 

src/librustdoc/markdown_writer.rs

@@ -14,7 +14,6 @@
 use doc::ItemUtils;
 use doc;
 
-use core::libc;
 use core::run;
 use core::comm::*;
 use extra::future;
@@ -105,60 +104,20 @@ fn pandoc_writer(
         debug!("pandoc cmd: %s", pandoc_cmd);
         debug!("pandoc args: %s", str::connect(pandoc_args, " "));
 
-        let pipe_in = os::pipe();
-        let pipe_out = os::pipe();
-        let pipe_err = os::pipe();
-        let pid = run::spawn_process(
-            pandoc_cmd, pandoc_args, &None, &None,
-            pipe_in.in, pipe_out.out, pipe_err.out);
+        let mut proc = run::Process::new(pandoc_cmd, pandoc_args, run::ProcessOptions::new());
 
-        let writer = io::fd_writer(pipe_in.out, false);
-        writer.write_str(markdown);
+        proc.input().write_str(markdown);
+        let output = proc.finish_with_output();
 
-        os::close(pipe_in.in);
-        os::close(pipe_out.out);
-        os::close(pipe_err.out);
-        os::close(pipe_in.out);
-
-        let (stdout_po, stdout_ch) = comm::stream();
-        do task::spawn_sched(task::SingleThreaded) || {
-            stdout_ch.send(readclose(pipe_out.in));
-        }
-
-        let (stderr_po, stderr_ch) = comm::stream();
-        do task::spawn_sched(task::SingleThreaded) || {
-            stderr_ch.send(readclose(pipe_err.in));
-        }
-        let stdout = stdout_po.recv();
-        let stderr = stderr_po.recv();
-
-        let status = run::waitpid(pid);
-        debug!("pandoc result: %i", status);
-        if status != 0 {
-            error!("pandoc-out: %s", stdout);
-            error!("pandoc-err: %s", stderr);
+        debug!("pandoc result: %i", output.status);
+        if output.status != 0 {
+            error!("pandoc-out: %s", str::from_bytes(output.output));
+            error!("pandoc-err: %s", str::from_bytes(output.error));
             fail!("pandoc failed");
         }
     }
 }
 
-fn readclose(fd: libc::c_int) -> ~str {
-    // Copied from run::program_output
-    unsafe {
-        let file = os::fdopen(fd);
-        let reader = io::FILE_reader(file, false);
-        let buf = io::with_bytes_writer(|writer| {
-            let mut bytes = [0, ..4096];
-            while !reader.eof() {
-                let nread = reader.read(bytes, bytes.len());
-                writer.write(bytes.slice(0, nread).to_owned());
-            }
-        });
-        os::fclose(file);
-        str::from_bytes(buf)
-    }
-}
-
 fn generic_writer(process: ~fn(markdown: ~str)) -> Writer {
     let (po, ch) = stream::<WriteInstr>();
     do task::spawn || {

src/librustpkg/rustpkg.rc

@@ -131,17 +131,17 @@ impl<'self> PkgScript<'self> {
                                                &exe, @copy os::args()[0],
                                                driver::cu_everything);
                 debug!("Running program: %s %s %s", exe.to_str(), root.to_str(), what);
-                let status = run::run_program(exe.to_str(), [root.to_str(), what]);
+                let status = run::process_status(exe.to_str(), [root.to_str(), what]);
                 if status != 0 {
                     return (~[], status);
                 }
                 else {
                     debug!("Running program (configs): %s %s %s",
                            exe.to_str(), root.to_str(), "configs");
-                    let output = run::program_output(exe.to_str(), [root.to_str(), ~"configs"]);
+                    let output = run::process_output(exe.to_str(), [root.to_str(), ~"configs"]);
                     // Run the configs() function to get the configs
                     let mut cfgs = ~[];
-                    for str::each_word(output.out) |w| {
+                    for str::each_word(str::from_bytes(output.output)) |w| {
                         cfgs.push(w.to_owned());
                     }
                     (cfgs, output.status)
@@ -551,7 +551,7 @@ impl PkgSrc {
         let url = fmt!("https://%s", self.id.remote_path.to_str());
         util::note(fmt!("git clone %s %s", url, local.to_str()));
 
-        if run::program_output("git", [~"clone", copy url, local.to_str()]).status != 0 {
+        if run::process_output("git", [~"clone", copy url, local.to_str()]).status != 0 {
             util::note(fmt!("fetching %s failed: can't clone repository", url));
             return false;
         }

src/librustpkg/testsuite/pass/src/fancy-lib/pkg.rs

@@ -23,6 +23,6 @@ pub fn main() {
     file.write_str("pub fn wheeeee() { for [1, 2, 3].each() |_| { assert!(true); } }");
 
     // now compile the crate itself
-    run::run_program("rustc", ~[~"src/fancy-lib/fancy-lib.rs", ~"--lib",
-                                ~"-o", out_path.push(~"fancy_lib").to_str()]);
+    run::process_status("rustc", [~"src/fancy-lib/fancy-lib.rs", ~"--lib", ~"-o",
+                        out_path.push(~"fancy_lib").to_str()]);
 }
\ No newline at end of file

src/libstd/os.rs

@@ -1671,7 +1671,7 @@ fn copy_file_ok() {
             fail!("%s doesn't exist", in.to_str());
           }
           assert!((rs));
-          let rslt = run::run_program("diff", [in.to_str(), out.to_str()]);
+          let rslt = run::process_status("diff", [in.to_str(), out.to_str()]);
           assert_eq!(rslt, 0);
           assert_eq!(out.get_mode(), in_mode);
           assert!((remove_file(&in)));

src/test/run-pass/core-run-destroy.rs

@@ -20,13 +20,13 @@
 
 #[test]
 fn test_destroy_once() {
-    let mut p = run::start_program("echo", []);
+    let mut p = run::Process::new("echo", [], run::ProcessOptions::new());
     p.destroy(); // this shouldn't crash (and nor should the destructor)
 }
 
 #[test]
 fn test_destroy_twice() {
-    let mut p = run::start_program("echo", []);
+    let mut p = run::Process::new("echo", [], run::ProcessOptions::new());
     p.destroy(); // this shouldnt crash...
     p.destroy(); // ...and nor should this (and nor should the destructor)
 }
@@ -41,7 +41,8 @@ fn test_destroy_actually_kills(force: bool) {
 
     #[cfg(unix)]
     fn process_exists(pid: libc::pid_t) -> bool {
-        run::program_output("ps", [~"-p", pid.to_str()]).out.contains(pid.to_str())
+        let run::ProcessOutput {output, _} = run::process_output("ps", [~"-p", pid.to_str()]);
+        str::from_bytes(output).contains(pid.to_str())
     }
 
     #[cfg(windows)]
@@ -64,8 +65,8 @@ fn process_exists(pid: libc::pid_t) -> bool {
         }
     }
 
-    // this program will stay alive indefinitely trying to read from stdin
-    let mut p = run::start_program(BLOCK_COMMAND, []);
+    // this process will stay alive indefinitely trying to read from stdin
+    let mut p = run::Process::new(BLOCK_COMMAND, [], run::ProcessOptions::new());
 
     assert!(process_exists(p.get_id()));