Add --release and related refactoring

src/bin/cargo-build.rs

@@ -14,6 +14,7 @@ extern crate serialize;
 use std::os;
 use cargo::{execute_main_without_stdin};
 use cargo::ops;
+use cargo::ops::CompileOptions;
 use cargo::core::MultiShell;
 use cargo::util::{CliResult, CliError};
 use cargo::util::important_paths::find_project_manifest;
@@ -23,6 +24,7 @@ pub struct Options {
     manifest_path: Option<String>,
     update_remotes: bool,
     jobs: Option<uint>,
+    release: bool,
 }
 
 hammer_config!(Options "Build the current project", |c| {
@@ -50,7 +52,20 @@ fn execute(options: Options, shell: &mut MultiShell) -> CliResult<Option<()>> {
     let update = options.update_remotes;
     let jobs = options.jobs;
 
-    ops::compile(&root, update, "compile", shell, jobs).map(|_| None).map_err(|err| {
+    let env = if options.release {
+        "release"
+    } else {
+        "compile"
+    };
+
+    let opts = CompileOptions {
+        update: options.update_remotes,
+        env: env,
+        shell: shell,
+        jobs: options.jobs
+    };
+
+    ops::compile(&root, opts).map(|_| None).map_err(|err| {
         CliError::from_boxed(err, 101)
     })
 }

src/bin/cargo-test.rs

@@ -44,18 +44,29 @@ fn execute(options: Options, shell: &mut MultiShell) -> CliResult<Option<()>> {
                     }))
     };
 
-    try!(ops::compile(&root, false, "test", shell, options.jobs)
-             .map(|_| None::<()>).map_err(|err| {
+    let compile_opts = ops::CompileOptions {
+        update: false,
+        env: "test",
+        shell: shell,
+        jobs: options.jobs
+    };
+
+    try!(ops::compile(&root, compile_opts).map(|_| None::<()>).map_err(|err| {
         CliError::from_boxed(err, 101)
     }));
 
-    let test_dir = root.dir_path().join("target").join("tests");
+    let test_dir = root.dir_path().join("target").join("test");
 
     let mut walk = try!(fs::walk_dir(&test_dir).map_err(|e| {
         CliError::from_error(e, 1)
     }));
 
     for file in walk {
+        // TODO: The proper fix is to have target knows its expected
+        // output and only run expected executables.
+        if file.display().to_str().as_slice().contains("dSYM") { continue; }
+        if !file.is_file() { continue; }
+
         try!(util::process(file).exec().map_err(|e| {
             CliError::from_boxed(e.box_error(), 1)
         }));

src/cargo/core/manifest.rs

@@ -104,16 +104,48 @@ pub struct Profile {
     env: String, // compile, test, dev, bench, etc.
     opt_level: uint,
     debug: bool,
-    test: bool
+    test: bool,
+    dest: Option<String>
 }
 
 impl Profile {
-    pub fn default(env: &str) -> Profile {
+    pub fn default_dev() -> Profile {
         Profile {
-            env: env.to_str(), // run in the default environment only
+            env: "compile".to_str(), // run in the default environment only
             opt_level: 0,
             debug: true,
-            test: false // whether or not to pass --test
+            test: false, // whether or not to pass --test
+            dest: None
+        }
+    }
+
+    pub fn default_test() -> Profile {
+        Profile {
+            env: "test".to_str(), // run in the default environment only
+            opt_level: 0,
+            debug: true,
+            test: true, // whether or not to pass --test
+            dest: Some("test".to_str())
+        }
+    }
+
+    pub fn default_bench() -> Profile {
+        Profile {
+            env: "bench".to_str(), // run in the default environment only
+            opt_level: 3,
+            debug: false,
+            test: true, // whether or not to pass --test
+            dest: Some("bench".to_str())
+        }
+    }
+
+    pub fn default_release() -> Profile {
+        Profile {
+            env: "release".to_str(), // run in the default environment only
+            opt_level: 3,
+            debug: false,
+            test: false, // whether or not to pass --test
+            dest: Some("release".to_str())
         }
     }
 
@@ -125,10 +157,22 @@ impl Profile {
         self.test
     }
 
+    pub fn get_opt_level(&self) -> uint {
+        self.opt_level
+    }
+
+    pub fn get_debug(&self) -> bool {
+        self.debug
+    }
+
     pub fn get_env<'a>(&'a self) -> &'a str {
         self.env.as_slice()
     }
 
+    pub fn get_dest<'a>(&'a self) -> Option<&'a str> {
+        self.dest.as_ref().map(|d| d.as_slice())
+    }
+
     pub fn opt_level(mut self, level: uint) -> Profile {
         self.opt_level = level;
         self

src/cargo/lib.rs

@@ -80,7 +80,7 @@ pub struct NoFlags;
 
 hammer_config!(NoFlags)
 
-#[deriving(Decodable)]
+#[deriving(Show, Decodable)]
 pub struct GlobalFlags {
     verbose: bool,
     help: bool,

src/cargo/ops/cargo_compile.rs

@@ -30,10 +30,16 @@ use ops;
 use sources::{PathSource};
 use util::{CargoResult, Wrap, config, internal, human};
 
-pub fn compile(manifest_path: &Path, update: bool,
-               env: &str, shell: &mut MultiShell,
-               jobs: Option<uint>) -> CargoResult<()>
-{
+pub struct CompileOptions<'a> {
+    pub update: bool,
+    pub env: &'a str,
+    pub shell: &'a mut MultiShell,
+    pub jobs: Option<uint>
+}
+
+pub fn compile(manifest_path: &Path, options: CompileOptions) -> CargoResult<()> {
+    let CompileOptions { update, env, shell, jobs } = options;
+
     log!(4, "compile; manifest-path={}", manifest_path.display());
 
     let mut source = PathSource::for_path(&manifest_path.dir_path());

src/cargo/ops/cargo_rustc.rs

@@ -23,14 +23,36 @@ struct Context<'a, 'b> {
 
 type Job = proc():Send -> CargoResult<()>;
 
+// This is a temporary assert that ensures the consistency of the arguments
+// given the current limitations of Cargo. The long term fix is to have each
+// Target know the absolute path to the build location.
+fn uniq_target_dest<'a>(targets: &[&'a Target]) -> Option<&'a str> {
+    let mut curr: Option<Option<&str>> = None;
+
+    for t in targets.iter() {
+        let dest = t.get_profile().get_dest();
+
+        match curr {
+            Some(curr) => assert!(curr == dest),
+            None => curr = Some(dest)
+        }
+    }
+
+    curr.unwrap()
+}
+
 pub fn compile_targets<'a>(targets: &[&Target], pkg: &Package, deps: &PackageSet,
                            config: &'a mut Config<'a>) -> CargoResult<()> {
 
+    if targets.is_empty() {
+        return Ok(());
+    }
+
     debug!("compile_targets; targets={}; pkg={}; deps={}", targets, pkg, deps);
 
-    let target_dir = pkg.get_absolute_target_dir();
+    let path_fragment = uniq_target_dest(targets);
+    let target_dir = pkg.get_absolute_target_dir().join(path_fragment.unwrap_or(""));
     let deps_target_dir = target_dir.join("deps");
-    let tests_target_dir = target_dir.join("tests");
 
     let output = try!(util::process("rustc").arg("-v").exec_with_output());
     let rustc_version = str::from_utf8(output.output.as_slice()).unwrap();
@@ -46,10 +68,6 @@ pub fn compile_targets<'a>(targets: &[&Target], pkg: &Package, deps: &PackageSet
         internal(format!("Couldn't create the directory for dependencies for {} at {}",
                  pkg.get_name(), deps_target_dir.display()))));
 
-    try!(mk_target(&tests_target_dir).chain_error(||
-        internal(format!("Couldn't create the directory for tests for {} at {}",
-                 pkg.get_name(), tests_target_dir.display()))));
-
     let mut cx = Context {
         dest: &deps_target_dir,
         deps_dir: &deps_target_dir,
@@ -189,9 +207,7 @@ fn compile_custom(pkg: &Package, cmd: &str,
     proc() p.exec_with_output().map(|_| ()).map_err(|e| e.mark_human())
 }
 
-fn rustc(root: &Path, target: &Target,
-         cx: &Context) -> Job {
-
+fn rustc(root: &Path, target: &Target, cx: &mut Context) -> Job {
     let crate_types = target.rustc_crate_types();
 
     log!(5, "root={}; target={}; crate_types={}; dest={}; deps={}; verbose={}",
@@ -203,6 +219,8 @@ fn rustc(root: &Path, target: &Target,
 
     log!(5, "command={}", rustc);
 
+    cx.config.shell().verbose(|shell| shell.status("Running", rustc.to_str()));
+
     proc() {
         if primary {
             rustc.exec().map_err(|err| human(err.to_str()))
@@ -237,10 +255,19 @@ fn build_base_args(into: &mut Args, target: &Target, crate_types: Vec<&str>,
     }
 
     let mut out = cx.dest.clone();
+    let profile = target.get_profile();
+
+    if profile.get_opt_level() != 0 {
+        into.push("--opt-level".to_str());
+        into.push(profile.get_opt_level().to_str());
+    }
+
+    if profile.get_debug() {
+        into.push("-g".to_str());
+    }
 
-    if target.get_profile().is_test() {
+    if profile.is_test() {
         into.push("--test".to_str());
-        out = out.join("tests");
     }
 
     into.push("--out-dir".to_str());

src/cargo/ops/mod.rs

@@ -1,4 +1,4 @@
-pub use self::cargo_compile::compile;
+pub use self::cargo_compile::{compile, CompileOptions};
 pub use self::cargo_read_manifest::{read_manifest,read_package,read_packages};
 pub use self::cargo_rustc::compile_targets;
 

src/cargo/util/toml.rs

@@ -247,10 +247,10 @@ fn normalize(lib: Option<&[TomlLibTarget]>,
     log!(4, "normalizing toml targets; lib={}; bin={}", lib, bin);
 
     fn target_profiles(target: &TomlTarget) -> Vec<Profile> {
-        let mut ret = vec!(Profile::default("compile"));
+        let mut ret = vec!(Profile::default_dev(), Profile::default_release());
 
         match target.test {
-            Some(true) | None => ret.push(Profile::default("test").test(true)),
+            Some(true) | None => ret.push(Profile::default_test()),
             _ => {}
         };
 

tests/test_cargo_compile.rs

@@ -92,7 +92,7 @@ test!(cargo_compile_with_invalid_code {
 {filename}:1 invalid rust code!
              ^~~~~~~
 Could not execute process \
-`rustc {filename} --crate-type bin --out-dir {} -L {} -L {}` (status=101)\n",
+`rustc {filename} --crate-type bin -g --out-dir {} -L {} -L {}` (status=101)\n",
             target.display(),
             target.display(),
             target.join("deps").display(),
@@ -547,7 +547,7 @@ test!(many_crate_types {
     let mut files: Vec<String> = files.iter().filter_map(|f| {
         match f.filename_str().unwrap() {
             "deps" => None,
-            s if s.contains("fingerprint") => None,
+            s if s.contains("fingerprint") || s.contains("dSYM") => None,
             s => Some(s.to_str())
         }
     }).collect();

tests/test_cargo_compile_git_deps.rs

@@ -82,7 +82,7 @@ test!(cargo_compile_simple_git_dep {
     let root = project.root();
     let git_root = git_project.root();
 
-    assert_that(project.cargo_process("cargo-build").arg("--verbose"),
+    assert_that(project.cargo_process("cargo-build"),
         execs()
         .with_stdout(format!("{} git repository `file:{}`\n\
                               {} dep1 v0.5.0 (file:{})\n\

tests/test_cargo_test.rs

@@ -36,5 +36,5 @@ test!(cargo_test_simple {
                                     0 ignored; 0 measured\n\n",
                                     COMPILING, p.root().display())));
 
-    assert_that(&p.bin("tests/foo"), existing_file());
+    assert_that(&p.bin("test/foo"), existing_file());
 })