From f97cef0c3088c0d577fd28b6e06ef92e4f47f31f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 23 Sep 2014 18:10:27 -0700 Subject: [PATCH] Add cargo {test,bench} -p This functionality allows running tests and benchmarks on any upstream dependencies in the dependency graph. This is most useful for path sources all developed in tandem (see Servo for instance). In terms of built artifacts, this will actually preserve as many artifacts as possible. That means that if you test a low-level dependency with the high-level artifacts already built, the high-level artifacts will not get removed. This means that it's possible to accidentally have a low-level dependency to depend on a higher level one just because it's lib is picked up via -L, but this is generally a necessary evil to get testing to not rebuild packages too often. Closes #483 --- src/bin/bench.rs | 27 +++++--- src/bin/build.rs | 34 +++++----- src/bin/clean.rs | 27 ++++---- src/bin/test.rs | 27 +++++--- src/cargo/ops/cargo_compile.rs | 5 ++ src/cargo/ops/cargo_rustc/job.rs | 11 ++++ src/cargo/ops/cargo_rustc/job_queue.rs | 11 +++- src/cargo/ops/cargo_rustc/mod.rs | 24 ++++--- tests/test_cargo_compile_git_deps.rs | 3 +- tests/test_cargo_compile_path_deps.rs | 6 +- tests/test_cargo_test.rs | 87 ++++++++++++++++++++++++++ 11 files changed, 197 insertions(+), 65 deletions(-) diff --git a/src/bin/bench.rs b/src/bin/bench.rs index 9f9396cdb21..27aede98b1e 100644 --- a/src/bin/bench.rs +++ b/src/bin/bench.rs @@ -13,20 +13,27 @@ Usage: cargo bench [options] [--] [...] Options: - -h, --help Print this message - --no-run Compile, but don't run benchmarks - -j N, --jobs N The number of jobs to run in parallel - --features FEATURES Space-separated list of features to also build - --no-default-features Do not build the `default` feature - --target TRIPLE Build for the target triple - --manifest-path PATH Path to the manifest to build benchmarks for - -v, --verbose Use verbose output + -h, --help Print this message + --no-run Compile, but don't run benchmarks + -p SPEC, --package SPEC Package to run benchmarks for + -j N, --jobs N The number of jobs to run in parallel + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature + --target TRIPLE Build for the target triple + --manifest-path PATH Path to the manifest to build benchmarks for + -v, --verbose Use verbose output All of the trailing arguments are passed to the benchmark binaries generated for filtering benchmarks and generally providing options configuring how they run. + +If the --package argument is given, then SPEC is a package id specification +which indicates which package should be benchmarked. If it is not given, then +the current package is benchmarked. For more information on SPEC and its format, +see the `cargo help pkgid` command. ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option, flag_features: Vec) + flag_manifest_path: Option, flag_features: Vec, + flag_package: Option) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path)); @@ -42,7 +49,7 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult dev_deps: true, features: options.flag_features.as_slice(), no_default_features: options.flag_no_default_features, - spec: None, + spec: options.flag_package.as_ref().map(|s| s.as_slice()), }, }; diff --git a/src/bin/build.rs b/src/bin/build.rs index 77047359ca6..fd05a1fa9b6 100644 --- a/src/bin/build.rs +++ b/src/bin/build.rs @@ -11,26 +11,26 @@ docopt!(Options, " Compile a local package and all of its dependencies Usage: - cargo build [options] [] + cargo build [options] Options: - -h, --help Print this message - -j N, --jobs N The number of jobs to run in parallel - --release Build artifacts in release mode, with optimizations - --features FEATURES Space-separated list of features to also build - --no-default-features Do not build the `default` feature - --target TRIPLE Build for the target triple - --manifest-path PATH Path to the manifest to compile - -v, --verbose Use verbose output - -If is given, then only the package specified will be build (along with -all its dependencies). If is not given, then the current package will be -built. - -For more information about the format of , see `cargo help pkgid`. + -h, --help Print this message + -p SPEC, --package SPEC Package to run benchmarks for + -j N, --jobs N The number of jobs to run in parallel + --release Build artifacts in release mode, with optimizations + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature + --target TRIPLE Build for the target triple + --manifest-path PATH Path to the manifest to compile + -v, --verbose Use verbose output + +If the --package argument is given, then SPEC is a package id specification +which indicates which package should be built. If it is not given, then the +current package built tested. For more information on SPEC and its format, see the +`cargo help pkgid` command. ", flag_jobs: Option, flag_target: Option, flag_manifest_path: Option, flag_features: Vec, - arg_spec: Option) + flag_package: Option) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { debug!("executing; cmd=cargo-build; args={}", os::args()); @@ -52,7 +52,7 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult dev_deps: false, features: options.flag_features.as_slice(), no_default_features: options.flag_no_default_features, - spec: options.arg_spec.as_ref().map(|s| s.as_slice()), + spec: options.flag_package.as_ref().map(|s| s.as_slice()), }; ops::compile(&root, &mut opts).map(|_| None).map_err(|err| { diff --git a/src/bin/clean.rs b/src/bin/clean.rs index fac0c9c3daf..fcdf810d52d 100644 --- a/src/bin/clean.rs +++ b/src/bin/clean.rs @@ -10,21 +10,20 @@ docopt!(Options, " Remove artifacts that cargo has generated in the past Usage: - cargo clean [options] [] + cargo clean [options] Options: - -h, --help Print this message - --manifest-path PATH Path to the manifest to the package to clean - --target TRIPLE Target triple to clean output for (default all) - -v, --verbose Use verbose output - -If is provided, then it is interpreted as a package id specification and -only the output for the package specified will be removed. If is not -provided, then all output from cargo will be cleaned out. Note that a lockfile -must exist for to be given. - -For more information about , see `cargo help pkgid`. -", flag_manifest_path: Option, arg_spec: Option, + -h, --help Print this message + -p SPEC, --package SPEC Package to run benchmarks for + --manifest-path PATH Path to the manifest to the package to clean + --target TRIPLE Target triple to clean output for (default all) + -v, --verbose Use verbose output + +If the --package argument is given, then SPEC is a package id specification +which indicates which package's artifacts should be cleaned out. If it is not +given, then all packages' artifacts are removed. For more information on SPEC +and its format, see the `cargo help pkgid` command. +", flag_manifest_path: Option, flag_package: Option, flag_target: Option) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { @@ -34,7 +33,7 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path)); let mut opts = ops::CleanOptions { shell: shell, - spec: options.arg_spec.as_ref().map(|s| s.as_slice()), + spec: options.flag_package.as_ref().map(|s| s.as_slice()), target: options.flag_target.as_ref().map(|s| s.as_slice()), }; ops::clean(&root, &mut opts).map(|_| None).map_err(|err| { diff --git a/src/bin/test.rs b/src/bin/test.rs index cfecce9cdfc..590026a59c8 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -13,19 +13,26 @@ Usage: cargo test [options] [--] [...] Options: - -h, --help Print this message - --no-run Compile, but don't run tests - -j N, --jobs N The number of jobs to run in parallel - --features FEATURES Space-separated list of features to also build - --no-default-features Do not build the `default` feature - --target TRIPLE Build for the target triple - --manifest-path PATH Path to the manifest to build tests for - -v, --verbose Use verbose output + -h, --help Print this message + --no-run Compile, but don't run tests + -p SPEC, --package SPEC Package to run tests for + -j N, --jobs N The number of jobs to run in parallel + --features FEATURES Space-separated list of features to also build + --no-default-features Do not build the `default` feature + --target TRIPLE Build for the target triple + --manifest-path PATH Path to the manifest to build tests for + -v, --verbose Use verbose output All of the trailing arguments are passed to the test binaries generated for filtering tests and generally providing options configuring how they run. + +If the --package argument is given, then SPEC is a package id specification +which indicates which package should be tested. If it is not given, then the +current package is tested. For more information on SPEC and its format, see the +`cargo help pkgid` command. ", flag_jobs: Option, flag_target: Option, - flag_manifest_path: Option, flag_features: Vec) + flag_manifest_path: Option, flag_features: Vec, + flag_package: Option) pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult> { let root = try!(find_root_manifest_for_cwd(options.flag_manifest_path)); @@ -41,7 +48,7 @@ pub fn execute(options: Options, shell: &mut MultiShell) -> CliResult dev_deps: true, features: options.flag_features.as_slice(), no_default_features: options.flag_no_default_features, - spec: None, + spec: options.flag_package.as_ref().map(|s| s.as_slice()), }, }; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index cfd82c0284a..01f10f6a760 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -56,6 +56,11 @@ pub fn compile(manifest_path: &Path, log!(4, "compile; manifest-path={}", manifest_path.display()); + if spec.is_some() && (no_default_features || features.len() > 0) { + return Err(human("features cannot be modified when the main package \ + is not being built")) + } + let mut source = try!(PathSource::for_path(&manifest_path.dir_path())); try!(source.update()); diff --git a/src/cargo/ops/cargo_rustc/job.rs b/src/cargo/ops/cargo_rustc/job.rs index 6493772f152..34853a26e54 100644 --- a/src/cargo/ops/cargo_rustc/job.rs +++ b/src/cargo/ops/cargo_rustc/job.rs @@ -15,6 +15,17 @@ impl Job { Job { dirty: dirty, fresh: fresh, desc: desc } } + /// Create a new job which will run `fresh` if the job is fresh and + /// otherwise not run `dirty`. + /// + /// Retains the same signature as `new` for compatibility. This job does not + /// describe itself to the console. + pub fn noop(_dirty: proc():Send -> CargoResult<()>, + fresh: proc():Send -> CargoResult<()>, + _desc: String) -> Job { + Job { dirty: proc() Ok(()), fresh: fresh, desc: String::new() } + } + /// Consumes this job by running it, returning the result of the /// computation. pub fn run(self, fresh: Freshness) -> CargoResult<()> { diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index 6c05c7df7ce..ad6aea045d8 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -1,5 +1,4 @@ -use std::collections::HashMap; -use std::collections::hashmap::{Occupied, Vacant}; +use std::collections::hashmap::{HashMap, HashSet, Occupied, Vacant}; use term::color::YELLOW; use core::{Package, PackageId, Resolve}; @@ -23,6 +22,7 @@ pub struct JobQueue<'a, 'b> { active: uint, pending: HashMap<(&'a PackageId, TargetStage), PendingBuild>, state: HashMap<&'a PackageId, Freshness>, + ignored: HashSet<&'a PackageId>, } /// A helper structure for metadata about the state of a building package. @@ -66,6 +66,7 @@ impl<'a, 'b> JobQueue<'a, 'b> { active: 0, pending: HashMap::new(), state: HashMap::new(), + ignored: HashSet::new(), } } @@ -85,6 +86,10 @@ impl<'a, 'b> JobQueue<'a, 'b> { (pkg, jobs)); } + pub fn ignore(&mut self, pkg: &'a Package) { + self.ignored.insert(pkg.get_package_id()); + } + /// Execute all jobs necessary to build the dependency graph. /// /// This function will spawn off `config.jobs()` workers to build all of the @@ -149,7 +154,7 @@ impl<'a, 'b> JobQueue<'a, 'b> { let amt = if njobs == 0 {1} else {njobs}; let id = pkg.get_package_id().clone(); - if stage == StageStart { + if stage == StageStart && !self.ignored.contains(&pkg.get_package_id()) { match fresh.combine(self.state[pkg.get_package_id()]) { Fresh => try!(config.shell().verbose(|c| { c.status("Fresh", pkg) diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index dd00134cfd3..0dc884f4f3f 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -77,30 +77,30 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, // particular package. No actual work is executed as part of this, that's // all done later as part of the `execute` function which will run // everything in order with proper parallelism. - let mut dep_pkgids = HashSet::new(); + let mut compiled = HashSet::new(); each_dep(pkg, &cx, |dep| { - dep_pkgids.insert(dep.get_package_id().clone()); + compiled.insert(dep.get_package_id().clone()); }); for dep in deps.iter() { if dep == pkg { continue } - if !dep_pkgids.contains(dep.get_package_id()) { continue } // Only compile lib targets for dependencies let targets = dep.get_targets().iter().filter(|target| { cx.is_relevant_target(*target) }).collect::>(); - if targets.len() == 0 { + if targets.len() == 0 && dep.get_package_id() != resolve.root() { return Err(human(format!("Package `{}` has no library targets", dep))) } - try!(compile(targets.as_slice(), dep, &mut cx, &mut queue)); + let compiled = compiled.contains(dep.get_package_id()); + try!(compile(targets.as_slice(), dep, compiled, &mut cx, &mut queue)); } if pkg.get_package_id() == resolve.root() { cx.primary(); } - try!(compile(targets, pkg, &mut cx, &mut queue)); + try!(compile(targets, pkg, true, &mut cx, &mut queue)); // Now that we've figured out everything that we're going to do, do it! try!(queue.execute(cx.config)); @@ -109,11 +109,19 @@ pub fn compile_targets<'a>(env: &str, targets: &[&'a Target], pkg: &'a Package, } fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, + compiled: bool, cx: &mut Context<'a, 'b>, jobs: &mut JobQueue<'a, 'b>) -> CargoResult<()> { debug!("compile_pkg; pkg={}; targets={}", pkg, targets); let _p = profile::start(format!("preparing: {}", pkg)); + // Packages/targets which are actually getting compiled are constructed into + // a real job. Packages which are *not* compiled still have their jobs + // executed, but only if the work is fresh. This is to preserve their + // artifacts if any exist. + let job = if compiled {Job::new} else {Job::noop}; + if !compiled { jobs.ignore(pkg); } + if targets.is_empty() { return Ok(()) } @@ -145,7 +153,7 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, for cmd in build_cmds.into_iter() { try!(cmd()) } dirty() }; - jobs.enqueue(pkg, StageCustomBuild, vec![(Job::new(dirty, fresh, desc), + jobs.enqueue(pkg, StageCustomBuild, vec![(job(dirty, fresh, desc), freshness)]); // After the custom command has run, execute rustc for all targets of our @@ -169,7 +177,7 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package, try!(fingerprint::prepare_target(cx, pkg, target, kind)); let dirty = proc() { try!(work()); dirty() }; - dst.push((Job::new(dirty, fresh, desc), freshness)); + dst.push((job(dirty, fresh, desc), freshness)); } } jobs.enqueue(pkg, StageLibraries, libs); diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index 8794889000f..8bab2b417c8 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -607,7 +607,8 @@ test!(recompilation { COMPILING, p.url()))); // Make sure clean only cleans one dep - assert_that(p.process(cargo_dir().join("cargo")).arg("clean").arg("foo"), + assert_that(p.process(cargo_dir().join("cargo")).arg("clean") + .arg("-p").arg("foo"), execs().with_stdout("")); assert_that(p.process(cargo_dir().join("cargo")).arg("build"), execs().with_stdout(format!("{} foo v0.5.0 ({})\n", diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs index 1edd24c69f5..1a4b611f8a7 100644 --- a/tests/test_cargo_compile_path_deps.rs +++ b/tests/test_cargo_compile_path_deps.rs @@ -88,11 +88,13 @@ test!(cargo_compile_with_nested_deps_shorthand { assert_that(p.process(cargo_dir().join("cargo")).arg("clean"), execs().with_stdout("")); println!("building baz"); - assert_that(p.process(cargo_dir().join("cargo")).arg("build").arg("baz"), + assert_that(p.process(cargo_dir().join("cargo")).arg("build") + .arg("-p").arg("baz"), execs().with_stdout(format!("{} baz v0.5.0 ({})\n", COMPILING, p.url()))); println!("building foo"); - assert_that(p.process(cargo_dir().join("cargo")).arg("build").arg("foo"), + assert_that(p.process(cargo_dir().join("cargo")).arg("build") + .arg("-p").arg("foo"), execs().with_stdout(format!("{} bar v0.5.0 ({})\n\ {} foo v0.5.0 ({})\n", COMPILING, p.url(), diff --git a/tests/test_cargo_test.rs b/tests/test_cargo_test.rs index 39be85a6b64..0d164c8b817 100644 --- a/tests/test_cargo_test.rs +++ b/tests/test_cargo_test.rs @@ -896,3 +896,90 @@ test!(test_no_harness { compiling = COMPILING, running = RUNNING, dir = p.url()).as_slice())); }) + +test!(selective_testing { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.d1] + path = "d1" + [dependencies.d2] + path = "d2" + + [lib] + name = "foo" + doctest = false + "#) + .file("src/lib.rs", "") + .file("d1/Cargo.toml", r#" + [package] + name = "d1" + version = "0.0.1" + authors = [] + + [lib] + name = "d1" + doctest = false + "#) + .file("d1/src/lib.rs", "") + .file("d2/Cargo.toml", r#" + [package] + name = "d2" + version = "0.0.1" + authors = [] + + [lib] + name = "d2" + doctest = false + "#) + .file("d2/src/lib.rs", ""); + p.build(); + + println!("d1"); + assert_that(p.process(cargo_dir().join("cargo")).arg("test") + .arg("-p").arg("d1"), + execs().with_status(0) + .with_stderr("") + .with_stdout(format!("\ +{compiling} d1 v0.0.1 ({dir}) +{running} target[..]d1-[..] + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured\n +", compiling = COMPILING, running = RUNNING, + dir = p.url()).as_slice())); + + println!("d2"); + assert_that(p.process(cargo_dir().join("cargo")).arg("test") + .arg("-p").arg("d2"), + execs().with_status(0) + .with_stderr("") + .with_stdout(format!("\ +{compiling} d2 v0.0.1 ({dir}) +{running} target[..]d2-[..] + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured\n +", compiling = COMPILING, running = RUNNING, + dir = p.url()).as_slice())); + + println!("whole"); + assert_that(p.process(cargo_dir().join("cargo")).arg("test"), + execs().with_status(0) + .with_stderr("") + .with_stdout(format!("\ +{compiling} foo v0.0.1 ({dir}) +{running} target[..]foo-[..] + +running 0 tests + +test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured\n +", compiling = COMPILING, running = RUNNING, + dir = p.url()).as_slice())); +})