Wrote more integration tests for cargo compile

At the moment, the rustc exec for the root project inherits the stdout
and stderr FDs (so that warnings and errors flow through), but
output from dependencies is only emitted if the compilation fails to
avoid warning noise from dependencies.

src/bin/cargo-compile.rs

@@ -32,5 +32,5 @@ fn execute(options: Options) -> CLIResult<Option<()>> {
                         CLIError::new("Could not find Cargo.toml in this directory or any parent directory", Some(err.to_str()), 102)))
     };
 
-    compile(root.as_str().unwrap().as_slice()).map(|v| Some(v)).to_cli(101)
+    compile(root.as_str().unwrap().as_slice()).map(|_| None).to_cli(101)
 }

src/cargo/core/package.rs

@@ -105,9 +105,18 @@ pub struct PackageSet {
 
 impl PackageSet {
     pub fn new(packages: &[Package]) -> PackageSet {
+        assert!(packages.len() > 0, "PackageSet must be created with at least one package")
         PackageSet { packages: Vec::from_slice(packages) }
     }
 
+    pub fn len(&self) -> uint {
+        self.packages.len()
+    }
+
+    pub fn pop(&mut self) -> Package {
+        self.packages.pop().unwrap()
+    }
+
     /**
      * Get a package by name out of the set
      */

src/cargo/mod.rs

@@ -60,7 +60,7 @@ pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable<json::Enc
         exec(flags)
     }
 
-    process_executed(call(exec))
+    process_executed(call(exec));
 }
 
 pub fn process_executed<'a, T: Encodable<json::Encoder<'a>, io::IoError>>(result: CLIResult<Option<T>>) {
@@ -76,8 +76,11 @@ pub fn process_executed<'a, T: Encodable<json::Encoder<'a>, io::IoError>>(result
 }
 
 pub fn handle_error(err: CLIError) {
-    let _ = write!(&mut std::io::stderr(), "{}", err.msg);
-    std::os::set_exit_status(err.exit_code as int);
+    let CLIError { msg, exit_code, .. } = err;
+    let _ = write!(&mut std::io::stderr(), "{}", msg);
+    //detail.map(|d| write!(&mut std::io::stderr(), ":\n{}", d));
+
+    std::os::set_exit_status(exit_code as int);
 }
 
 fn flags_from_args<T: RepresentsFlags>() -> CLIResult<T> {

src/cargo/ops/cargo_compile.rs

@@ -44,7 +44,7 @@ pub fn compile(manifest_path: &str) -> CargoResult<()> {
 
     try!(source.download(resolved.as_slice()).wrap("unable to download packages"));
 
-    let packages = try!(source.get(resolved.as_slice()).wrap("unable ot get packages from source"));
+    let packages = try!(source.get(resolved.as_slice()).wrap("unable to get packages from source"));
 
     let package_set = PackageSet::new(packages.as_slice());
 

src/cargo/ops/cargo_rustc.rs

@@ -1,27 +1,34 @@
 use std::os::args;
 use std::io;
 use std::path::Path;
+use std::str;
 use core;
 use util;
-use util::{other_error,CargoResult,CargoError};
+use util::{other_error,human_error,CargoResult,CargoError,ProcessBuilder};
+use util::result::ProcessError;
 
 type Args = Vec<~str>;
 
 pub fn compile(pkgs: &core::PackageSet) -> CargoResult<()> {
-    let sorted = match pkgs.sort() {
+    let mut sorted = match pkgs.sort() {
         Some(pkgs) => pkgs,
         None => return Err(other_error("circular dependency detected"))
     };
 
+    let root = sorted.pop();
+
     for pkg in sorted.iter() {
         println!("Compiling {}", pkg);
-        try!(compile_pkg(pkg, pkgs));
+        try!(compile_pkg(pkg, pkgs, |rustc| rustc.exec_with_output()));
     }
 
+    println!("Compiling {}", root);
+    try!(compile_pkg(&root, pkgs, |rustc| rustc.exec()));
+
     Ok(())
 }
 
-fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()> {
+fn compile_pkg<T>(pkg: &core::Package, pkgs: &core::PackageSet, exec: |&ProcessBuilder| -> CargoResult<T>) -> CargoResult<()> {
     // Build up the destination
     // let src = pkg.get_root().join(Path::new(pkg.get_source().path.as_slice()));
     let target_dir = pkg.get_absolute_target_dir();
@@ -31,7 +38,7 @@ fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()>
 
     // compile
     for target in pkg.get_targets().iter() {
-        try!(rustc(pkg.get_root(), target, &target_dir, pkgs.get_packages()))
+        try!(rustc(pkg.get_root(), target, &target_dir, pkgs.get_packages(), |rustc| exec(rustc)))
     }
 
     Ok(())
@@ -41,19 +48,24 @@ fn mk_target(target: &Path) -> io::IoResult<()> {
     io::fs::mkdir_recursive(target, io::UserRWX)
 }
 
-fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> CargoResult<()> {
+fn rustc<T>(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package], exec: |&ProcessBuilder| -> CargoResult<T>) -> CargoResult<()> {
+    let rustc = prepare_rustc(root, target, dest, deps);
+
+    try!(exec(&rustc)
+        .map_err(|err| rustc_to_cargo_err(rustc.get_args(), root, err)));
+
+    Ok(())
+}
+
+fn prepare_rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> ProcessBuilder {
     let mut args = Vec::new();
 
     build_base_args(&mut args, target, dest);
     build_deps_args(&mut args, deps);
 
-    try!(util::process("rustc")
+    util::process("rustc")
         .cwd(root.clone())
         .args(args.as_slice())
-        .exec()
-        .map_err(|err| rustc_to_cargo_err(&args, root, err)));
-
-    Ok(())
 }
 
 fn build_base_args(into: &mut Args, target: &core::Target, dest: &Path) {
@@ -73,7 +85,17 @@ fn build_deps_args(dst: &mut Args, deps: &[core::Package]) {
     }
 }
 
-fn rustc_to_cargo_err(args: &Vec<~str>, cwd: &Path, err: io::IoError) -> CargoError {
-    other_error("failed to exec rustc")
-        .with_detail(format!("args={}; root={}; cause={}", args.connect(" "), cwd.display(), err.to_str()))
+fn rustc_to_cargo_err(args: &[~str], cwd: &Path, err: CargoError) -> CargoError {
+    let msg = {
+        let output = match err {
+            CargoError { kind: ProcessError(_, ref output), .. } => output,
+            _ => fail!("Bug! exec() returned an error other than a ProcessError")
+        };
+
+        let mut msg = StrBuf::from_str(format!("failed to execute: `rustc {}`", args.connect(" ")));
+        output.as_ref().map(|o| msg.push_str(format!("; Error:\n{}", str::from_utf8_lossy(o.error.as_slice()))));
+        msg
+    };
+
+    human_error(msg.to_owned(), format!("root={}", cwd.display()), err)
 }

src/cargo/util/mod.rs

@@ -1,5 +1,5 @@
 pub use self::process_builder::{process,ProcessBuilder};
-pub use self::result::{CargoError,CargoResult,Wrap,Require,ToCLI,other_error,human_error,toml_error};
+pub use self::result::{CargoError,CargoResult,Wrap,Require,ToCLI,other_error,human_error,toml_error,io_error,process_error};
 
 pub mod graph;
 pub mod process_builder;

src/cargo/util/process_builder.rs

@@ -2,8 +2,8 @@ use std::fmt;
 use std::fmt::{Show,Formatter};
 use std::os;
 use std::path::Path;
-use std::io;
 use std::io::process::{Process,ProcessConfig,ProcessOutput,InheritFd};
+use util::{CargoResult,io_error,process_error};
 use collections::HashMap;
 
 #[deriving(Clone,Eq)]
@@ -36,6 +36,10 @@ impl ProcessBuilder {
         self
     }
 
+    pub fn get_args<'a>(&'a self) -> &'a [~str] {
+        self.args.as_slice()
+    }
+
     pub fn extra_path(mut self, path: Path) -> ProcessBuilder {
         // For now, just convert to a string, but we should do something better
         self.path.push(format!("{}", path.display()));
@@ -48,7 +52,7 @@ impl ProcessBuilder {
     }
 
     // TODO: should InheritFd be hardcoded?
-    pub fn exec(&self) -> io::IoResult<()> {
+    pub fn exec(&self) -> CargoResult<()> {
         let mut config = try!(self.build_config());
         let env = self.build_env();
 
@@ -57,31 +61,34 @@ impl ProcessBuilder {
         config.stdout = InheritFd(1);
         config.stderr = InheritFd(2);
 
-        let mut process = try!(Process::configure(config));
+        let mut process = try!(Process::configure(config).map_err(io_error));
         let exit = process.wait();
 
         if exit.success() {
             Ok(())
-        }
-        else {
-            Err(io::IoError {
-                kind: io::OtherIoError,
-                desc: "process did not exit successfully",
-                detail: None
-            })
+        } else {
+            let msg = format!("Could not execute process `{}`", self.debug_string());
+            Err(process_error(msg, exit, None))
         }
     }
 
-    pub fn exec_with_output(&self) -> io::IoResult<ProcessOutput> {
+    pub fn exec_with_output(&self) -> CargoResult<ProcessOutput> {
         let mut config = try!(self.build_config());
         let env = self.build_env();
 
         config.env = Some(env.as_slice());
 
-        Process::configure(config).map(|mut ok| ok.wait_with_output())
+        let output = try!(Process::configure(config).map(|mut ok| ok.wait_with_output()).map_err(io_error));
+
+        if output.status.success() {
+            Ok(output)
+        } else {
+            let msg = format!("Could not execute process `{}`", self.debug_string());
+            Err(process_error(msg, output.status.clone(), Some(output)))
+        }
     }
 
-    fn build_config<'a>(&'a self) -> io::IoResult<ProcessConfig<'a>> {
+    fn build_config<'a>(&'a self) -> CargoResult<ProcessConfig<'a>> {
         let mut config = ProcessConfig::new();
 
         config.program = self.program.as_slice();
@@ -91,6 +98,10 @@ impl ProcessBuilder {
         Ok(config)
     }
 
+    fn debug_string(&self) -> ~str {
+        format!("{} {}", self.program, self.args.connect(" "))
+    }
+
     fn build_env(&self) -> ~[(~str, ~str)] {
         let mut ret = Vec::new();
 

src/cargo/util/result.rs

@@ -1,4 +1,8 @@
+use std::fmt;
+use std::fmt::{Show,Formatter};
 use std::io;
+use std::io::IoError;
+use std::io::process::{ProcessOutput,ProcessExit};
 use core::errors::{CLIError,CLIResult};
 use toml;
 
@@ -18,6 +22,26 @@ pub fn other_error(desc: &'static str) -> CargoError {
     }
 }
 
+pub fn io_error(err: IoError) -> CargoError {
+    let desc = err.desc;
+
+    CargoError {
+        kind: IoError(err),
+        desc: StaticDescription(desc),
+        detail: None,
+        cause: None
+    }
+}
+
+pub fn process_error(detail: ~str, exit: ProcessExit, output: Option<ProcessOutput>) -> CargoError {
+    CargoError {
+        kind: ProcessError(exit, output),
+        desc: BoxedDescription(detail),
+        detail: None,
+        cause: None
+    }
+}
+
 pub fn human_error(desc: ~str, detail: ~str, cause: CargoError) -> CargoError {
     CargoError {
         kind: HumanReadableError,
@@ -38,7 +62,7 @@ pub fn toml_error(desc: &'static str, error: toml::Error) -> CargoError {
 
 #[deriving(Show,Clone)]
 pub struct CargoError {
-    kind: CargoErrorKind,
+    pub kind: CargoErrorKind,
     desc: CargoErrorDescription,
     detail: Option<~str>,
     cause: Option<Box<CargoError>>
@@ -85,15 +109,45 @@ impl CargoError {
     }
 }
 
-#[deriving(Show,Clone)]
 pub enum CargoErrorKind {
     HumanReadableError,
     InternalError,
+    ProcessError(ProcessExit, Option<ProcessOutput>),
     IoError(io::IoError),
     TomlError(toml::Error),
     OtherCargoError
 }
 
+impl Show for CargoErrorKind {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        match self {
+            &ProcessError(ref exit, _) => write!(f.buf, "ProcessError({})", exit),
+            &HumanReadableError => write!(f.buf, "HumanReadableError"),
+            &InternalError => write!(f.buf, "InternalError"),
+            &IoError(ref err) => write!(f.buf, "IoError({})", err),
+            &TomlError(ref err) => write!(f.buf, "TomlError({})", err),
+            &OtherCargoError => write!(f.buf, "OtherCargoError")
+        }
+    }
+}
+
+impl Clone for CargoErrorKind {
+    fn clone(&self) -> CargoErrorKind {
+        match self {
+            &ProcessError(ref exit, ref output) => {
+                ProcessError(exit.clone(), output.as_ref().map(|output| ProcessOutput {
+                    status: output.status.clone(), output: output.output.clone(), error: output.error.clone()
+                }))
+            },
+            &HumanReadableError => HumanReadableError,
+            &InternalError => InternalError,
+            &IoError(ref err) => IoError(err.clone()),
+            &TomlError(ref err) => TomlError(err.clone()),
+            &OtherCargoError => OtherCargoError
+        }
+    }
+}
+
 type CargoCliResult<T> = Result<T, CargoCliError>;
 
 #[deriving(Show,Clone)]

tests/support.rs

@@ -8,7 +8,8 @@ use std::str;
 use std::vec::Vec;
 use std::fmt::Show;
 use ham = hamcrest;
-use cargo::util::{process,ProcessBuilder};
+use cargo::util::{process,ProcessBuilder,CargoError};
+use cargo::util::result::ProcessError;
 
 static CARGO_INTEGRATION_TEST_DIR : &'static str = "cargo-integration-tests";
 
@@ -230,6 +231,7 @@ impl ham::Matcher<ProcessBuilder> for Execs {
 
     match res {
       Ok(out) => self.match_output(&out),
+      Err(CargoError { kind: ProcessError(_, ref out), .. }) => self.match_output(out.get_ref()),
       Err(_) => Err(format!("could not exec process {}", process))
     }
   }

tests/test_cargo_compile.rs

@@ -6,19 +6,23 @@ use cargo::util::process;
 fn setup() {
 }
 
-test!(cargo_compile {
-    let p = project("foo")
-        .file("Cargo.toml", r#"
+fn basic_bin_manifest(name: &str) -> ~str {
+    format!(r#"
         [project]
 
-            name = "foo"
+        name = "{}"
         version = "0.5.0"
         authors = ["wycats@example.com"]
 
         [[bin]]
 
-            name = "foo"
-        "#)
+        name = "{}"
+    "#, name, name)
+}
+
+test!(cargo_compile {
+    let p = project("foo")
+        .file("Cargo.toml", basic_bin_manifest("foo"))
         .file("src/foo.rs", main_file(r#""i am foo""#, []));
 
     assert_that(p.cargo_process("cargo-compile"), execs());
@@ -50,6 +54,84 @@ test!(cargo_compile_without_manifest {
         .with_stderr("Could not find Cargo.toml in this directory or any parent directory"));
 })
 
+test!(cargo_compile_with_invalid_code {
+    let p = project("foo")
+        .file("Cargo.toml", basic_bin_manifest("foo"))
+        .file("src/foo.rs", "invalid rust code!");
+
+    let target = p.root().join("target");
+
+    assert_that(p.cargo_process("cargo-compile"),
+        execs()
+        .with_status(101)
+        .with_stderr(format!("src/foo.rs:1:1: 1:8 error: expected item but found `invalid`\nsrc/foo.rs:1 invalid rust code!\n             ^~~~~~~\nfailed to execute: `rustc src/foo.rs --crate-type bin --out-dir {} -L {}`", target.display(), target.display())));
+})
+
+test!(cargo_compile_with_warnings_in_the_root_package {
+    let p = project("foo")
+        .file("Cargo.toml", basic_bin_manifest("foo"))
+        .file("src/foo.rs", "fn main() {} fn dead() {}");
+
+    assert_that(p.cargo_process("cargo-compile"),
+        execs()
+        .with_stderr("src/foo.rs:1:14: 1:26 warning: code is never used: `dead`, #[warn(dead_code)] on by default\nsrc/foo.rs:1 fn main() {} fn dead() {}\n                          ^~~~~~~~~~~~\n"));
+})
+
+test!(cargo_compile_with_warnings_in_a_dep_package {
+    let mut p = project("foo");
+    let bar = p.root().join("bar");
+
+    p = p
+        .file(".cargo/config", format!(r#"
+            paths = ["{}"]
+        "#, bar.display()))
+        .file("Cargo.toml", r#"
+            [project]
+
+            name = "foo"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [dependencies]
+
+            bar = "0.5.0"
+
+            [[bin]]
+
+            name = "foo"
+        "#)
+        .file("src/foo.rs", main_file(r#""{}", bar::gimme()"#, ["bar"]))
+        .file("bar/Cargo.toml", r#"
+            [project]
+
+            name = "bar"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [[lib]]
+
+            name = "bar"
+        "#)
+        .file("bar/src/bar.rs", r#"
+            pub fn gimme() -> ~str {
+                "test passed".to_owned()
+            }
+
+            fn dead() {}
+        "#);
+
+    assert_that(p.cargo_process("cargo-compile"),
+        execs()
+        .with_stdout("Compiling bar v0.5.0\nCompiling foo v0.5.0\n")
+        .with_stderr(""));
+
+    assert_that(&p.root().join("target/foo"), existing_file());
+
+    assert_that(
+      cargo::util::process("foo").extra_path(p.root().join("target")),
+      execs().with_stdout("test passed\n"));
+})
+
 test!(cargo_compile_with_nested_deps {
     let mut p = project("foo");
     let bar = p.root().join("bar");