Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --exclude flag #4031

Merged
merged 5 commits into from
May 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/bin/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub struct Options {
flag_locked: bool,
arg_args: Vec<String>,
flag_all: bool,
flag_exclude: Vec<String>,
}

pub const USAGE: &'static str = "
Expand All @@ -52,6 +53,7 @@ Options:
--no-run Compile, but don't run benchmarks
-p SPEC, --package SPEC ... Package to run benchmarks for
--all Benchmark all packages in the workspace
--exclude SPEC ... Exclude packages from the benchmark
-j N, --jobs N Number of parallel jobs, defaults to # of CPUs
--features FEATURES Space-separated list of features to also build
--all-features Build all available features
Expand Down Expand Up @@ -86,11 +88,9 @@ Compilation can be customized with the `bench` profile in the manifest.
pub fn execute(options: Options, config: &Config) -> CliResult {
let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;

let spec = if options.flag_all {
Packages::All
} else {
Packages::Packages(&options.flag_package)
};
let spec = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

config.configure(options.flag_verbose,
options.flag_quiet,
Expand Down
11 changes: 6 additions & 5 deletions src/bin/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub struct Options {
flag_locked: bool,
flag_frozen: bool,
flag_all: bool,
flag_exclude: Vec<String>,
}

pub const USAGE: &'static str = "
Expand All @@ -43,6 +44,7 @@ Options:
-h, --help Print this message
-p SPEC, --package SPEC ... Package to build
--all Build all packages in the workspace
--exclude SPEC ... Exclude packages from the build
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems pretty verbose to manually handle conflicts / requirements of flags. Is there a way to let docopt do this? There is currently sort of a bug when executing cargo build -p whatever --all: The -p is silently ignored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to let docopt do this?

Basically we need to switch to clap, and hopefully get rid of a lot of duplication along the way. But this is a hard task :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically we need to switch to clap

Oh it is desired to switch to clap? It's good to hear that! Is there someone working on it already? If not, I'd love to do it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there someone working on it already?

I've tried to do that, but haven't arrived at something very satisfying :)

It would be super if you would be able to pull this off!

-j N, --jobs N Number of parallel jobs, defaults to # of CPUs
--lib Build only this package's library
--bin NAME Build only the specified binary
Expand Down Expand Up @@ -73,6 +75,7 @@ current package is built. For more information on SPEC and its format, see the

All packages in the workspace are built if the `--all` flag is supplied. The
`--all` flag may be supplied in the presence of a virtual manifest.
Note that `--exclude` has to be specified in conjunction with the `--all` flag.

Compilation can be configured via the use of profiles which are configured in
the manifest. The default profile for this command is `dev`, but passing
Expand All @@ -90,11 +93,9 @@ pub fn execute(options: Options, config: &Config) -> CliResult {

let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;

let spec = if options.flag_all {
Packages::All
} else {
Packages::Packages(&options.flag_package)
};
let spec = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

let opts = CompileOptions {
config: config,
Expand Down
10 changes: 5 additions & 5 deletions src/bin/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Options:
-h, --help Print this message
-p SPEC, --package SPEC ... Package(s) to check
--all Check all packages in the workspace
--exclude SPEC ... Exclude packages from the check
-j N, --jobs N Number of parallel jobs, defaults to # of CPUs
--lib Check only this package's library
--bin NAME Check only the specified binary
Expand Down Expand Up @@ -74,6 +75,7 @@ pub struct Options {
flag_locked: bool,
flag_frozen: bool,
flag_all: bool,
flag_exclude: Vec<String>,
}

pub fn execute(options: Options, config: &Config) -> CliResult {
Expand All @@ -89,11 +91,9 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
let ws = Workspace::new(&root, config)?;

let spec = if options.flag_all {
Packages::All
} else {
Packages::Packages(&options.flag_package)
};
let spec = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

let opts = CompileOptions {
config: config,
Expand Down
10 changes: 5 additions & 5 deletions src/bin/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct Options {
flag_frozen: bool,
flag_locked: bool,
flag_all: bool,
flag_exclude: Vec<String>,
}

pub const USAGE: &'static str = "
Expand All @@ -56,6 +57,7 @@ Options:
--no-run Compile, but don't run tests
-p SPEC, --package SPEC ... Package to run tests for
--all Test all packages in the workspace
--exclude SPEC ... Exclude packages from the test
-j N, --jobs N Number of parallel builds, see below for details
--release Build artifacts in release mode, with optimizations
--features FEATURES Space-separated list of features to also build
Expand Down Expand Up @@ -130,11 +132,9 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
&options.flag_bench, options.flag_benches);
}

let spec = if options.flag_all {
Packages::All
} else {
Packages::Packages(&options.flag_package)
};
let spec = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

let ops = ops::TestOptions {
no_run: options.flag_no_run,
Expand Down
22 changes: 22 additions & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,25 @@ pub enum MessageFormat {
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum Packages<'a> {
All,
OptOut(&'a [String]),
Packages(&'a [String]),
}

impl<'a> Packages<'a> {
pub fn from_flags(all: bool, exclude: &'a Vec<String>, package: &'a Vec<String>)
-> CargoResult<Self>
{
let packages = match (all, &exclude) {
(true, exclude) if exclude.is_empty() => Packages::All,
(true, exclude) => Packages::OptOut(exclude),
(false, exclude) if !exclude.is_empty() => bail!("--exclude can only be used together \
with --all"),
_ => Packages::Packages(package),
};

Ok(packages)
}

pub fn into_package_id_specs(self, ws: &Workspace) -> CargoResult<Vec<PackageIdSpec>> {
let specs = match self {
Packages::All => {
Expand All @@ -117,6 +132,13 @@ impl<'a> Packages<'a> {
.map(PackageIdSpec::from_package_id)
.collect()
}
Packages::OptOut(opt_out) => {
ws.members()
.map(Package::package_id)
.map(PackageIdSpec::from_package_id)
.filter(|p| opt_out.iter().position(|x| *x == p.name()).is_none())
.collect()
}
Packages::Packages(packages) => {
packages.iter().map(|p| PackageIdSpec::parse(&p)).collect::<CargoResult<Vec<_>>>()?
}
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub fn run(ws: &Workspace,

let pkg = match options.spec {
Packages::All => unreachable!("cargo run supports single package only"),
Packages::OptOut(_) => unreachable!("cargo run supports single package only"),
Packages::Packages(xs) => match xs.len() {
0 => ws.current()?,
1 => ws.members()
Expand Down
53 changes: 53 additions & 0 deletions tests/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,59 @@ fn bench_all_workspace() {
.with_stdout_contains("test bench_foo ... bench: [..]"));
}

#[test]
fn bench_all_exclude() {
if !is_nightly() { return }

let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"

[workspace]
members = ["bar", "baz"]
"#)
.file("src/main.rs", r#"
fn main() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
#![feature(test)]

extern crate test;

#[bench]
pub fn bar(b: &mut test::Bencher) {
b.iter(|| {});
}
"#)
.file("baz/Cargo.toml", r#"
[project]
name = "baz"
version = "0.1.0"
"#)
.file("baz/src/lib.rs", r#"
#[test]
pub fn baz() {
break_the_build();
}
"#);

assert_that(p.cargo_process("bench")
.arg("--all")
.arg("--exclude")
.arg("baz"),
execs().with_status(0)
.with_stdout_contains("\
running 1 test
test bar ... bench: 0 ns/iter (+/- 0)"));
}

#[test]
fn bench_all_virtual_manifest() {
if !is_nightly() { return }
Expand Down
43 changes: 43 additions & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2803,6 +2803,49 @@ fn build_all_workspace() {
[..] Finished dev [unoptimized + debuginfo] target(s) in [..]\n"));
}

#[test]
fn build_all_exclude() {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"

[workspace]
members = ["bar", "baz"]
"#)
.file("src/main.rs", r#"
fn main() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#)
.file("baz/Cargo.toml", r#"
[project]
name = "baz"
version = "0.1.0"
"#)
.file("baz/src/lib.rs", r#"
pub fn baz() {
break_the_build();
}
"#);

assert_that(p.cargo_process("build")
.arg("--all")
.arg("--exclude")
.arg("baz"),
execs().with_status(0)
.with_stderr_contains("[..]Compiling foo v0.1.0 [..]")
.with_stderr_contains("[..]Compiling bar v0.1.0 [..]")
.with_stderr_does_not_contain("[..]Compiling baz v0.1.0 [..]"));
}

#[test]
fn build_all_workspace_implicit_examples() {
let p = project("foo")
Expand Down
44 changes: 44 additions & 0 deletions tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2377,6 +2377,50 @@ fn test_all_workspace() {
.with_stdout_contains("test bar_test ... ok"));
}

#[test]
fn test_all_exclude() {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"

[workspace]
members = ["bar", "baz"]
"#)
.file("src/main.rs", r#"
fn main() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
#[test]
pub fn bar() {}
"#)
.file("baz/Cargo.toml", r#"
[project]
name = "baz"
version = "0.1.0"
"#)
.file("baz/src/lib.rs", r#"
#[test]
pub fn baz() {
assert!(false);
}
"#);

assert_that(p.cargo_process("test")
.arg("--all")
.arg("--exclude")
.arg("baz"),
execs().with_status(0)
.with_stdout_contains("running 1 test
test bar ... ok"));
}

#[test]
fn test_all_virtual_manifest() {
let p = project("workspace")
Expand Down