From a1980dc78b5e008febc4f61987668f79744ae66c Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Mon, 7 Jul 2014 15:17:34 -0700 Subject: [PATCH] Add --release and related refactoring --- src/bin/cargo-build.rs | 17 ++++++++- src/bin/cargo-test.rs | 17 +++++++-- src/cargo/core/manifest.rs | 52 +++++++++++++++++++++++++--- src/cargo/lib.rs | 2 +- src/cargo/ops/cargo_compile.rs | 14 +++++--- src/cargo/ops/cargo_rustc.rs | 49 ++++++++++++++++++++------ src/cargo/ops/mod.rs | 2 +- src/cargo/util/toml.rs | 4 +-- tests/test_cargo_compile.rs | 4 +-- tests/test_cargo_compile_git_deps.rs | 2 +- tests/test_cargo_test.rs | 2 +- 11 files changed, 134 insertions(+), 31 deletions(-) diff --git a/src/bin/cargo-build.rs b/src/bin/cargo-build.rs index 533ce9ece00..04e2ec790cf 100755 --- a/src/bin/cargo-build.rs +++ b/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, update_remotes: bool, jobs: Option, + release: bool, } hammer_config!(Options "Build the current project", |c| { @@ -50,7 +52,20 @@ fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { 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) }) } diff --git a/src/bin/cargo-test.rs b/src/bin/cargo-test.rs index be01b17b7f2..79624e47cf2 100755 --- a/src/bin/cargo-test.rs +++ b/src/bin/cargo-test.rs @@ -44,18 +44,29 @@ fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { })) }; - 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) })); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index ffd1e6eee98..8a6223061bf 100644 --- a/src/cargo/core/manifest.rs +++ b/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 } 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 diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index ba00b39a56e..32a07fd9c13 100644 --- a/src/cargo/lib.rs +++ b/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, diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 789f65e3e26..cadc97a9c44 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/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) -> CargoResult<()> -{ +pub struct CompileOptions<'a> { + pub update: bool, + pub env: &'a str, + pub shell: &'a mut MultiShell, + pub jobs: Option +} + +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()); diff --git a/src/cargo/ops/cargo_rustc.rs b/src/cargo/ops/cargo_rustc.rs index 6c6218cf15d..6083932a3b3 100644 --- a/src/cargo/ops/cargo_rustc.rs +++ b/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> = 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()); diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index 785f77fad93..45ec38101f6 100644 --- a/src/cargo/ops/mod.rs +++ b/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; diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 15a4e11d561..8de333e6ae2 100644 --- a/src/cargo/util/toml.rs +++ b/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 { - 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()), _ => {} }; diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 9fa6a8aa8b1..9b6e3b816ce 100644 --- a/tests/test_cargo_compile.rs +++ b/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 = 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(); diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index 62370c11dad..c44f1c860a4 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/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\ diff --git a/tests/test_cargo_test.rs b/tests/test_cargo_test.rs index 455b0ced638..426f0f2e4fb 100644 --- a/tests/test_cargo_test.rs +++ b/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()); })