Skip to content

Commit b0a2252

Browse files
borsbors
committed
Auto merge of #5186 - infinity0:stricter-need-dev-deps, r=alexcrichton
Stricter need_dev_deps behaviour The previous PR (#5012) contained an unnecessary work-around for behaviour of `--all-targets` that was misunderstood. This PR removes that work-around and adds some tests and comments to clarify the behaviour for future contributors, which may help to make easier a future fix for #5177 and #5178.
2 parents 4dde726 + 0bc1155 commit b0a2252

File tree

8 files changed

+189
-26
lines changed

8 files changed

+189
-26
lines changed

‎src/bin/commands/rustc.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
6262
}
6363
};
6464
let mut compile_opts = args.compile_options_for_single_package(config, mode)?;
65-
compile_opts.target_rustc_args = Some(values(args, "args"));
65+
let target_args = values(args, "args");
66+
compile_opts.target_rustc_args = if target_args.is_empty() {
67+
None
68+
} else {
69+
Some(target_args)
70+
};
6671
ops::compile(&ws, &compile_opts)?;
6772
Ok(())
6873
}

‎src/bin/commands/rustdoc.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,12 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
4949
let ws = args.workspace(config)?;
5050
let mut compile_opts =
5151
args.compile_options_for_single_package(config, CompileMode::Doc { deps: false })?;
52-
compile_opts.target_rustdoc_args = Some(values(args, "args"));
52+
let target_args = values(args, "args");
53+
compile_opts.target_rustdoc_args = if target_args.is_empty() {
54+
None
55+
} else {
56+
Some(target_args)
57+
};
5358
let doc_opts = DocOptions {
5459
open_result: args.is_present("open"),
5560
compile_opts,

‎src/cargo/ops/cargo_compile.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ pub fn compile_ws<'a>(
246246
let specs = spec.into_package_id_specs(ws)?;
247247
let features = Method::split_features(features);
248248
let method = Method::Required {
249-
dev_deps: ws.require_optional_deps() || filter.need_dev_deps(),
249+
dev_deps: ws.require_optional_deps() || filter.need_dev_deps(mode),
250250
features: &features,
251251
all_features,
252252
uses_default_features: !no_default_features,
@@ -442,19 +442,25 @@ impl CompileFilter {
442442
}
443443
}
444444

445-
pub fn need_dev_deps(&self) -> bool {
446-
match *self {
447-
CompileFilter::Default { .. } => true,
448-
CompileFilter::Only {
449-
ref examples,
450-
ref tests,
451-
ref benches,
452-
..
453-
} => examples.is_specific() || tests.is_specific() || benches.is_specific(),
445+
pub fn need_dev_deps(&self, mode: CompileMode) -> bool {
446+
match mode {
447+
CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true,
448+
CompileMode::Build | CompileMode::Doc { .. } | CompileMode::Check { .. } => match *self
449+
{
450+
CompileFilter::Default { .. } => false,
451+
CompileFilter::Only {
452+
ref examples,
453+
ref tests,
454+
ref benches,
455+
..
456+
} => examples.is_specific() || tests.is_specific() || benches.is_specific(),
457+
},
454458
}
455459
}
456460

457-
pub fn matches(&self, target: &Target) -> bool {
461+
// this selects targets for "cargo run". for logic to select targets for
462+
// other subcommands, see generate_targets and generate_default_targets
463+
pub fn target_run(&self, target: &Target) -> bool {
458464
match *self {
459465
CompileFilter::Default { .. } => true,
460466
CompileFilter::Only {
@@ -493,7 +499,7 @@ struct BuildProposal<'a> {
493499
required: bool,
494500
}
495501

496-
fn generate_auto_targets<'a>(
502+
fn generate_default_targets<'a>(
497503
mode: CompileMode,
498504
targets: &'a [Target],
499505
profile: &'a Profile,
@@ -715,7 +721,7 @@ fn generate_targets<'a>(
715721
} else {
716722
&profiles.test_deps
717723
};
718-
generate_auto_targets(
724+
generate_default_targets(
719725
mode,
720726
pkg.targets(),
721727
profile,

‎src/cargo/ops/cargo_run.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub fn run(
3333
!a.is_lib() && !a.is_custom_build() && if !options.filter.is_specific() {
3434
a.is_bin()
3535
} else {
36-
options.filter.matches(a)
36+
options.filter.target_run(a)
3737
}
3838
})
3939
.map(|bin| bin.name())

‎tests/testsuite/build.rs

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5381,6 +5381,70 @@ fn build_filter_infer_profile() {
53815381
);
53825382
}
53835383

5384+
#[test]
5385+
fn targets_selected_default() {
5386+
let p = project("foo")
5387+
.file(
5388+
"Cargo.toml",
5389+
r#"
5390+
[package]
5391+
name = "foo"
5392+
version = "0.1.0"
5393+
authors = []
5394+
"#,
5395+
)
5396+
.file("src/main.rs", "fn main() {}")
5397+
.build();
5398+
assert_that(
5399+
p.cargo("build").arg("-v"),
5400+
execs().with_status(0)
5401+
// bin
5402+
.with_stderr_contains("\
5403+
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
5404+
--emit=dep-info,link[..]")
5405+
// bench
5406+
.with_stderr_does_not_contain("\
5407+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
5408+
-C opt-level=3 --test [..]")
5409+
// unit test
5410+
.with_stderr_does_not_contain("\
5411+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
5412+
-C debuginfo=2 --test [..]"),
5413+
);
5414+
}
5415+
5416+
#[test]
5417+
fn targets_selected_all() {
5418+
let p = project("foo")
5419+
.file(
5420+
"Cargo.toml",
5421+
r#"
5422+
[package]
5423+
name = "foo"
5424+
version = "0.1.0"
5425+
authors = []
5426+
"#,
5427+
)
5428+
.file("src/main.rs", "fn main() {}")
5429+
.build();
5430+
assert_that(
5431+
p.cargo("build").arg("-v").arg("--all-targets"),
5432+
execs().with_status(0)
5433+
// bin
5434+
.with_stderr_contains("\
5435+
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
5436+
--emit=dep-info,link[..]")
5437+
// bench
5438+
.with_stderr_contains("\
5439+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
5440+
-C opt-level=3 --test [..]")
5441+
// unit test
5442+
.with_stderr_contains("\
5443+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
5444+
-C debuginfo=2 --test [..]"),
5445+
);
5446+
}
5447+
53845448
#[test]
53855449
fn all_targets_no_lib() {
53865450
let p = project("foo")
@@ -5471,11 +5535,9 @@ fn avoid_dev_deps() {
54715535
.file("src/main.rs", "fn main() {}")
54725536
.build();
54735537

5474-
// --bins is needed because of #5134
5475-
assert_that(p.cargo("build").arg("--bins"), execs().with_status(101));
5538+
assert_that(p.cargo("build"), execs().with_status(101));
54765539
assert_that(
54775540
p.cargo("build")
5478-
.arg("--bins")
54795541
.masquerade_as_nightly_cargo()
54805542
.arg("-Zavoid-dev-deps"),
54815543
execs().with_status(0),

‎tests/testsuite/check.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,30 @@ fn check_virtual_all_implied() {
606606
}
607607

608608
#[test]
609-
fn check_all_targets() {
609+
fn targets_selected_default() {
610+
let foo = project("foo")
611+
.file("Cargo.toml", SIMPLE_MANIFEST)
612+
.file("src/main.rs", "fn main() {}")
613+
.file("src/lib.rs", "pub fn smth() {}")
614+
.file("examples/example1.rs", "fn main() {}")
615+
.file("tests/test2.rs", "#[test] fn t() {}")
616+
.file("benches/bench3.rs", "")
617+
.build();
618+
619+
assert_that(
620+
foo.cargo("check").arg("-v"),
621+
execs()
622+
.with_status(0)
623+
.with_stderr_contains("[..] --crate-name foo src[/]lib.rs [..]")
624+
.with_stderr_contains("[..] --crate-name foo src[/]main.rs [..]")
625+
.with_stderr_does_not_contain("[..] --crate-name example1 examples[/]example1.rs [..]")
626+
.with_stderr_does_not_contain("[..] --crate-name test2 tests[/]test2.rs [..]")
627+
.with_stderr_does_not_contain("[..] --crate-name bench3 benches[/]bench3.rs [..]"),
628+
);
629+
}
630+
631+
#[test]
632+
fn targets_selected_all() {
610633
let foo = project("foo")
611634
.file("Cargo.toml", SIMPLE_MANIFEST)
612635
.file("src/main.rs", "fn main() {}")

‎tests/testsuite/install.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,9 +1222,8 @@ fn dev_dependencies_no_check() {
12221222
.file("src/main.rs", "fn main() {}")
12231223
.build();
12241224

1225-
// --bins is needed because of #5134
1226-
assert_that(p.cargo("build").arg("--bins"), execs().with_status(101));
1227-
assert_that(p.cargo("install").arg("--bins"), execs().with_status(0));
1225+
assert_that(p.cargo("build"), execs().with_status(101));
1226+
assert_that(p.cargo("install"), execs().with_status(0));
12281227
}
12291228

12301229
#[test]
@@ -1256,10 +1255,9 @@ fn dev_dependencies_lock_file_untouched() {
12561255
.file("a/src/lib.rs", "")
12571256
.build();
12581257

1259-
// --bins is needed because of #5134
1260-
assert_that(p.cargo("build").arg("--bins"), execs().with_status(0));
1258+
assert_that(p.cargo("build"), execs().with_status(0));
12611259
let lock = p.read_lockfile();
1262-
assert_that(p.cargo("install").arg("--bins"), execs().with_status(0));
1260+
assert_that(p.cargo("install"), execs().with_status(0));
12631261
let lock2 = p.read_lockfile();
12641262
assert!(lock == lock2, "different lockfiles");
12651263
}

‎tests/testsuite/rustc.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,70 @@ fn build_only_bar_dependency() {
413413
);
414414
}
415415

416+
#[test]
417+
fn targets_selected_default() {
418+
let p = project("foo")
419+
.file(
420+
"Cargo.toml",
421+
r#"
422+
[package]
423+
name = "foo"
424+
version = "0.1.0"
425+
authors = []
426+
"#,
427+
)
428+
.file("src/main.rs", "fn main() {}")
429+
.build();
430+
assert_that(
431+
p.cargo("rustc").arg("-v"),
432+
execs().with_status(0)
433+
// bin
434+
.with_stderr_contains("\
435+
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
436+
--emit=dep-info,link[..]")
437+
// bench
438+
.with_stderr_does_not_contain("\
439+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
440+
-C opt-level=3 --test [..]")
441+
// unit test
442+
.with_stderr_does_not_contain("\
443+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
444+
-C debuginfo=2 --test [..]"),
445+
);
446+
}
447+
448+
#[test]
449+
fn targets_selected_all() {
450+
let p = project("foo")
451+
.file(
452+
"Cargo.toml",
453+
r#"
454+
[package]
455+
name = "foo"
456+
version = "0.1.0"
457+
authors = []
458+
"#,
459+
)
460+
.file("src/main.rs", "fn main() {}")
461+
.build();
462+
assert_that(
463+
p.cargo("rustc").arg("-v").arg("--all-targets"),
464+
execs().with_status(0)
465+
// bin
466+
.with_stderr_contains("\
467+
[RUNNING] `rustc --crate-name foo src[/]main.rs --crate-type bin \
468+
--emit=dep-info,link[..]")
469+
// bench
470+
.with_stderr_contains("\
471+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
472+
-C opt-level=3 --test [..]")
473+
// unit test
474+
.with_stderr_contains("\
475+
[RUNNING] `rustc --crate-name foo src[/]main.rs --emit=dep-info,link \
476+
-C debuginfo=2 --test [..]"),
477+
);
478+
}
479+
416480
#[test]
417481
fn fail_with_multiple_packages() {
418482
let foo = project("foo")

0 commit comments

Comments
 (0)