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

Rebased and fixed 4025: Apply --all if workspace is virtual #4335

Merged
merged 6 commits into from
Jul 28, 2017
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ src/registry/Cargo.lock
rustc
__pycache__
.idea/
*.iml
*.iml
*.swp
19 changes: 13 additions & 6 deletions src/bin/bench.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::env;

use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, CliError, Config, CargoErrorKind};
Expand Down Expand Up @@ -88,17 +90,23 @@ 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 = Packages::from_flags(options.flag_all,
&options.flag_exclude,
&options.flag_package)?;
debug!("executing; cmd=cargo-bench; args={:?}",
env::args().collect::<Vec<_>>());

config.configure(options.flag_verbose,
options.flag_quiet,
&options.flag_color,
options.flag_frozen,
options.flag_locked)?;

let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
let ws = Workspace::new(&root, config)?;

let spec = Packages::from_flags(ws.is_virtual(),
options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

let ops = ops::TestOptions {
no_run: options.flag_no_run,
no_fail_fast: options.flag_no_fail_fast,
Expand All @@ -124,7 +132,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
},
};

let ws = Workspace::new(&root, config)?;
let err = ops::run_benches(&ws, &ops, &options.arg_args)?;
match err {
None => Ok(()),
Expand Down
5 changes: 3 additions & 2 deletions src/bin/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,10 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
options.flag_locked)?;

let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
let ws = Workspace::new(&root, config)?;

let spec = Packages::from_flags(options.flag_all,
let spec = Packages::from_flags(ws.is_virtual(),
options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

Expand All @@ -117,7 +119,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
target_rustc_args: None,
};

let ws = Workspace::new(&root, config)?;
ops::compile(&ws, &opts)?;
Ok(())
}
3 changes: 2 additions & 1 deletion src/bin/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ 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 = Packages::from_flags(options.flag_all,
let spec = Packages::from_flags(ws.is_virtual(),
options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

Expand Down
9 changes: 7 additions & 2 deletions src/bin/doc.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::env;

use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, Config};
Expand Down Expand Up @@ -69,15 +71,19 @@ the `cargo help pkgid` command.
";

pub fn execute(options: Options, config: &Config) -> CliResult {
debug!("executing; cmd=cargo-check; args={:?}",
env::args().collect::<Vec<_>>());

config.configure(options.flag_verbose,
options.flag_quiet,
&options.flag_color,
options.flag_frozen,
options.flag_locked)?;

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 {
let spec = if options.flag_all || (ws.is_virtual() && options.flag_package.is_empty()) {
Packages::All
} else {
Packages::Packages(&options.flag_package)
Expand Down Expand Up @@ -109,7 +115,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
},
};

let ws = Workspace::new(&root, config)?;
ops::doc(&ws, &doc_opts)?;
Ok(())
}
10 changes: 8 additions & 2 deletions src/bin/test.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::env;

use cargo::core::Workspace;
use cargo::ops::{self, MessageFormat, Packages};
use cargo::util::{CliResult, CliError, Config, CargoErrorKind};
Expand Down Expand Up @@ -109,13 +111,17 @@ To get the list of all options available for the test binaries use this:
";

pub fn execute(options: Options, config: &Config) -> CliResult {
debug!("executing; cmd=cargo-test; args={:?}",
env::args().collect::<Vec<_>>());

config.configure(options.flag_verbose,
options.flag_quiet,
&options.flag_color,
options.flag_frozen,
options.flag_locked)?;

let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
let ws = Workspace::new(&root, config)?;

let empty = Vec::new();
let (mode, filter);
Expand All @@ -132,7 +138,8 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
&options.flag_bench, options.flag_benches);
}

let spec = Packages::from_flags(options.flag_all,
let spec = Packages::from_flags(ws.is_virtual(),
options.flag_all,
&options.flag_exclude,
&options.flag_package)?;

Expand All @@ -157,7 +164,6 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
},
};

let ws = Workspace::new(&root, config)?;
let err = ops::run_tests(&ws, &ops, &options.arg_args)?;
match err {
None => Ok(()),
Expand Down
7 changes: 7 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ impl<'cfg> Workspace<'cfg> {
}
}

pub fn is_virtual(&self) -> bool {
match *self.packages.get(&self.current_manifest) {
MaybePackage::Package(..) => false,
MaybePackage::Virtual(..) => true
}
}

/// Returns the `Config` this workspace is associated with.
pub fn config(&self) -> &'cfg Config {
self.config
Expand Down
4 changes: 3 additions & 1 deletion src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ pub enum Packages<'a> {
}

impl<'a> Packages<'a> {
pub fn from_flags(all: bool, exclude: &'a Vec<String>, package: &'a Vec<String>)
pub fn from_flags(virtual_ws: bool, all: bool, exclude: &'a Vec<String>, package: &'a Vec<String>)
-> CargoResult<Self>
{
let all = all || (virtual_ws && package.is_empty());

let packages = match (all, &exclude) {
(true, exclude) if exclude.is_empty() => Packages::All,
(true, exclude) => Packages::OptOut(exclude),
Expand Down
52 changes: 52 additions & 0 deletions tests/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1189,3 +1189,55 @@ fn legacy_bench_name() {
[WARNING] path `[..]src[/]bench.rs` was erroneously implicitly accepted for benchmark `bench`,
please set bench.path in Cargo.toml"));
}

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

let p = project("workspace")
.file("Cargo.toml", r#"
[workspace]
members = ["foo", "bar"]
"#)
.file("foo/Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"
"#)
.file("foo/src/lib.rs", r#"
pub fn foo() {}
"#)
.file("foo/benches/foo.rs", r#"
#![feature(test)]
extern crate test;
use test::Bencher;
#[bench]
fn bench_foo(_: &mut Bencher) -> () { () }
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#)
.file("bar/benches/bar.rs", r#"
#![feature(test)]
extern crate test;
use test::Bencher;
#[bench]
fn bench_bar(_: &mut Bencher) -> () { () }
"#);

// The order in which foo and bar are built is not guaranteed

assert_that(p.cargo_process("bench"),
execs().with_status(0)
.with_stderr_contains("\
[RUNNING] target[/]release[/]deps[/]bar-[..][EXE]")
.with_stdout_contains("test bench_bar ... bench: [..]")
.with_stderr_contains("\
[RUNNING] target[/]release[/]deps[/]foo-[..][EXE]")
.with_stdout_contains("test bench_foo ... bench: [..]"));
}
68 changes: 68 additions & 0 deletions tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3038,6 +3038,74 @@ fn build_all_virtual_manifest() {
[..] Finished dev [unoptimized + debuginfo] target(s) in [..]\n"));
}

#[test]
fn build_virtual_manifest_all_implied() {
let p = project("workspace")
.file("Cargo.toml", r#"
[workspace]
members = ["foo", "bar"]
"#)
.file("foo/Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"
"#)
.file("foo/src/lib.rs", r#"
pub fn foo() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#);

// The order in which foo and bar are built is not guaranteed
assert_that(p.cargo_process("build"),
execs().with_status(0)
.with_stderr_contains("[..] Compiling bar v0.1.0 ([..])")
.with_stderr_contains("[..] Compiling foo v0.1.0 ([..])")
.with_stderr("[..] Compiling [..] v0.1.0 ([..])\n\
[..] Compiling [..] v0.1.0 ([..])\n\
[..] Finished dev [unoptimized + debuginfo] target(s) in [..]\n"));
}

#[test]
fn build_virtual_manifest_one_project() {
let p = project("workspace")
.file("Cargo.toml", r#"
[workspace]
members = ["foo", "bar"]
"#)
.file("foo/Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"
"#)
.file("foo/src/lib.rs", r#"
pub fn foo() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#);

// The order in which foo and bar are built is not guaranteed
Copy link
Member

Choose a reason for hiding this comment

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

The comment is a bit misleading, because we don' compile bar at all :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

assert_that(p.cargo_process("build")
.arg("-p").arg("foo"),
execs().with_status(0)
.with_stderr_does_not_contain("[..] Compiling bar v0.1.0 ([..])")
Copy link
Member

Choose a reason for hiding this comment

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

Let's change this to .with_stderr_does_not_contain("bar") just to make this more robust in case we change the precise output format in the future.

.with_stderr_contains("[..] Compiling foo v0.1.0 ([..])")
.with_stderr("[..] Compiling [..] v0.1.0 ([..])\n\
[..] Finished dev [unoptimized + debuginfo] target(s) in [..]\n"));
}

#[test]
fn build_all_virtual_manifest_implicit_examples() {
let p = project("foo")
Expand Down
31 changes: 31 additions & 0 deletions tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,3 +393,34 @@ fn check_all() {
.with_stderr_contains("[..] --crate-name b b[/]src[/]main.rs [..]")
);
}

#[test]
fn check_virtual_all_implied() {
let p = project("workspace")
.file("Cargo.toml", r#"
[workspace]
members = ["foo", "bar"]
"#)
.file("foo/Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"
"#)
.file("foo/src/lib.rs", r#"
pub fn foo() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#);

assert_that(p.cargo_process("check").arg("-v"),
execs().with_status(0)
.with_stderr_contains("[..] --crate-name foo foo[/]src[/]lib.rs [..]")
.with_stderr_contains("[..] --crate-name bar bar[/]src[/]lib.rs [..]")
);
}
31 changes: 31 additions & 0 deletions tests/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,37 @@ fn doc_all_virtual_manifest() {
.with_stderr_contains("[..] Documenting foo v0.1.0 ([..])"));
}

#[test]
fn doc_virtual_manifest_all_implied() {
let p = project("workspace")
.file("Cargo.toml", r#"
[workspace]
members = ["foo", "bar"]
"#)
.file("foo/Cargo.toml", r#"
[project]
name = "foo"
version = "0.1.0"
"#)
.file("foo/src/lib.rs", r#"
pub fn foo() {}
"#)
.file("bar/Cargo.toml", r#"
[project]
name = "bar"
version = "0.1.0"
"#)
.file("bar/src/lib.rs", r#"
pub fn bar() {}
"#);

// The order in which foo and bar are documented is not guaranteed
assert_that(p.cargo_process("doc"),
execs().with_status(0)
.with_stderr_contains("[..] Documenting bar v0.1.0 ([..])")
.with_stderr_contains("[..] Documenting foo v0.1.0 ([..])"));
}

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