diff --git a/src/bin/cargo-compile.rs b/src/bin/cargo-compile.rs index 39bc267d1..269ec346b 100644 --- a/src/bin/cargo-compile.rs +++ b/src/bin/cargo-compile.rs @@ -32,5 +32,5 @@ fn execute(options: Options) -> CLIResult> { 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) } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 513106601..a208517f6 100644 --- a/src/cargo/core/package.rs +++ b/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 */ diff --git a/src/cargo/mod.rs b/src/cargo/mod.rs index b0ad6c23e..aaaa7f60e 100644 --- a/src/cargo/mod.rs +++ b/src/cargo/mod.rs @@ -60,7 +60,7 @@ pub fn execute_main_without_stdin<'a, T: RepresentsFlags, V: Encodable, io::IoError>>(result: CLIResult>) { @@ -76,8 +76,11 @@ pub fn process_executed<'a, T: Encodable, 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() -> CLIResult { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index e1522caaa..919a6b90b 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/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()); diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index e93129f92..ab9e7acab 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/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(pkg: &core::Package, pkgs: &core::PackageSet, exec: |&ProcessBuilder| -> CargoResult) -> 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(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package], exec: |&ProcessBuilder| -> CargoResult) -> 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) } diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 22c84efc3..fd80bfb6d 100644 --- a/src/cargo/util/mod.rs +++ b/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; diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index 1740c9a4a..e8aea0e0e 100644 --- a/src/cargo/util/process_builder.rs +++ b/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 { + pub fn exec_with_output(&self) -> CargoResult { 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> { + fn build_config<'a>(&'a self) -> CargoResult> { 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(); diff --git a/src/cargo/util/result.rs b/src/cargo/util/result.rs index 2645b0775..1692e1ab8 100644 --- a/src/cargo/util/result.rs +++ b/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) -> 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> @@ -85,15 +109,45 @@ impl CargoError { } } -#[deriving(Show,Clone)] pub enum CargoErrorKind { HumanReadableError, InternalError, + ProcessError(ProcessExit, Option), 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 = Result; #[deriving(Show,Clone)] diff --git a/tests/support.rs b/tests/support.rs index 49dc81032..338564da5 100644 --- a/tests/support.rs +++ b/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 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)) } } diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index a909f802f..15b29c284 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -6,19 +6,23 @@ use cargo::util::process; fn setup() { } +fn basic_bin_manifest(name: &str) -> ~str { + format!(r#" + [project] + + name = "{}" + version = "0.5.0" + authors = ["wycats@example.com"] + + [[bin]] + + name = "{}" + "#, name, name) +} + test!(cargo_compile { let p = project("foo") - .file("Cargo.toml", r#" - [project] - - name = "foo" - version = "0.5.0" - authors = ["wycats@example.com"] - - [[bin]] - - name = "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");