Auto merge of #5012 - infinity0:pr4988, r=alexcrichton

Don't require dev-dependencies when not needed in certain cases

Specifically, when running `cargo install` and add a flag for this behaviour in `cargo build`.

This closes #4988.

Author: bors

src/bin/build.rs

@@ -98,7 +98,10 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
                      &options.flag_z)?;
 
     let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
-    let ws = Workspace::new(&root, config)?;
+    let mut ws = Workspace::new(&root, config)?;
+    if config.cli_unstable().avoid_dev_deps {
+        ws.set_require_optional_deps(false);
+    }
 
     let spec = Packages::from_flags(options.flag_all,
                                     &options.flag_exclude,

src/cargo/core/features.rs

@@ -278,6 +278,7 @@ pub struct CliUnstable {
     pub unstable_options: bool,
     pub offline: bool,
     pub no_index_update: bool,
+    pub avoid_dev_deps: bool,
 }
 
 impl CliUnstable {
@@ -310,6 +311,7 @@ impl CliUnstable {
             "unstable-options" => self.unstable_options = true,
             "offline" => self.offline = true,
             "no-index-update" => self.no_index_update = true,
+            "avoid-dev-deps" => self.avoid_dev_deps = true,
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/core/resolver/mod.rs

@@ -98,14 +98,26 @@ pub struct DepsNotReplaced<'a> {
 
 #[derive(Clone, Copy)]
 pub enum Method<'a> {
-    Everything,
+    Everything, // equivalent to Required { dev_deps: true, all_features: true, .. }
     Required {
         dev_deps: bool,
         features: &'a [String],
+        all_features: bool,
         uses_default_features: bool,
     },
 }
 
+impl<'r> Method<'r> {
+    pub fn split_features(features: &[String]) -> Vec<String> {
+        features.iter()
+            .flat_map(|s| s.split_whitespace())
+            .flat_map(|s| s.split(','))
+            .filter(|s| !s.is_empty())
+            .map(|s| s.to_string())
+            .collect::<Vec<String>>()
+    }
+}
+
 // Information about the dependencies for a crate, a tuple of:
 //
 // (dependency info, candidates, features activated)
@@ -910,6 +922,7 @@ fn activate_deps_loop(
             let method = Method::Required {
                 dev_deps: false,
                 features: &features,
+                all_features: false,
                 uses_default_features: dep.uses_default_features(),
             };
             trace!("{}[{}]>{} trying {}", parent.name(), cur, dep.name(), candidate.summary.version());
@@ -1255,7 +1268,8 @@ fn build_requirements<'a, 'b: 'a>(s: &'a Summary, method: &'b Method)
                                   -> CargoResult<Requirements<'a>> {
     let mut reqs = Requirements::new(s);
     match *method {
-        Method::Everything => {
+        Method::Everything |
+        Method::Required { all_features: true, .. } => {
             for key in s.features().keys() {
                 reqs.require_feature(key)?;
             }
@@ -1308,10 +1322,11 @@ impl Context {
         }
         debug!("checking if {} is already activated", summary.package_id());
         let (features, use_default) = match *method {
+            Method::Everything |
+            Method::Required { all_features: true, .. } => return Ok(false),
             Method::Required { features, uses_default_features, .. } => {
                 (features, uses_default_features)
             }
-            Method::Everything => return Ok(false),
         };
 
         let has_default_feature = summary.features().contains_key("default");

src/cargo/core/workspace.rs

@@ -64,7 +64,8 @@ pub struct Workspace<'cfg> {
 
     // True if this workspace should enforce optional dependencies even when
     // not needed; false if this workspace should only enforce dependencies
-    // needed by the current configuration (such as in cargo install).
+    // needed by the current configuration (such as in cargo install). In some
+    // cases `false` also results in the non-enforcement of dev-dependencies.
     require_optional_deps: bool,
 }
 
@@ -296,6 +297,11 @@ impl<'cfg> Workspace<'cfg> {
         self.require_optional_deps
     }
 
+    pub fn set_require_optional_deps<'a>(&'a mut self, require_optional_deps: bool) -> &mut Workspace<'cfg> {
+        self.require_optional_deps = require_optional_deps;
+        self
+    }
+
     /// Finds the root of a workspace for the crate whose manifest is located
     /// at `manifest_path`.
     ///

src/cargo/ops/cargo_compile.rs

@@ -29,7 +29,7 @@ use std::sync::Arc;
 
 use core::{Source, Package, Target};
 use core::{Profile, TargetKind, Profiles, Workspace, PackageId, PackageIdSpec};
-use core::resolver::Resolve;
+use core::resolver::{Resolve, Method};
 use ops::{self, BuildOutput, Executor, DefaultExecutor};
 use util::config::Config;
 use util::{CargoResult, profile};
@@ -233,12 +233,18 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
     let profiles = ws.profiles();
 
     let specs = spec.into_package_id_specs(ws)?;
-    let resolve = ops::resolve_ws_precisely(ws,
-                                            source,
-                                            features,
-                                            all_features,
-                                            no_default_features,
-                                            &specs)?;
+    let features = Method::split_features(features);
+    let method = Method::Required {
+        dev_deps: ws.require_optional_deps() || filter.need_dev_deps(),
+        features: &features,
+        all_features,
+        uses_default_features: !no_default_features,
+    };
+    let resolve = ops::resolve_ws_with_method(ws,
+                                              source,
+                                              method,
+                                              &specs,
+                                              )?;
     let (packages, resolve_with_overrides) = resolve;
 
     let to_builds = specs.iter().map(|p| {
@@ -414,6 +420,14 @@ impl<'a> CompileFilter<'a> {
         }
     }
 
+    pub fn need_dev_deps(&self) -> bool {
+        match *self {
+            CompileFilter::Default { .. } => true,
+            CompileFilter::Only { examples, tests, benches, .. } =>
+                examples.is_specific() || tests.is_specific() || benches.is_specific()
+        }
+    }
+
     pub fn matches(&self, target: &Target) -> bool {
         match *self {
             CompileFilter::Default { .. } => true,

src/cargo/ops/cargo_install.rs

@@ -176,7 +176,11 @@ fn install_one(root: &Filesystem,
 
     let ws = match overidden_target_dir {
         Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?,
-        None => Workspace::new(pkg.manifest_path(), config)?,
+        None => {
+            let mut ws = Workspace::new(pkg.manifest_path(), config)?;
+            ws.set_require_optional_deps(false);
+            ws
+        }
     };
     let pkg = ws.current()?;
 

src/cargo/ops/mod.rs

@@ -22,7 +22,7 @@ pub use self::registry::{modify_owners, yank, OwnersOptions, PublishOpts};
 pub use self::registry::configure_http_handle;
 pub use self::cargo_fetch::fetch;
 pub use self::cargo_pkgid::pkgid;
-pub use self::resolve::{resolve_ws, resolve_ws_precisely, resolve_with_previous};
+pub use self::resolve::{resolve_ws, resolve_ws_precisely, resolve_ws_with_method, resolve_with_previous};
 pub use self::cargo_output_metadata::{output_metadata, OutputMetadataOptions, ExportInfo};
 
 mod cargo_clean;

src/cargo/ops/resolve.rs

@@ -29,13 +29,25 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
                                 no_default_features: bool,
                                 specs: &[PackageIdSpec])
                                 -> CargoResult<(PackageSet<'a>, Resolve)> {
-    let features = features.iter()
-        .flat_map(|s| s.split_whitespace())
-        .flat_map(|s| s.split(','))
-        .filter(|s| !s.is_empty())
-        .map(|s| s.to_string())
-        .collect::<Vec<String>>();
+    let features = Method::split_features(features);
+    let method = if all_features {
+        Method::Everything
+    } else {
+        Method::Required {
+            dev_deps: true,
+            features: &features,
+            all_features: false,
+            uses_default_features: !no_default_features,
+        }
+    };
+    resolve_ws_with_method(ws, source, method, specs)
+}
 
+pub fn resolve_ws_with_method<'a>(ws: &Workspace<'a>,
+                                  source: Option<Box<Source + 'a>>,
+                                  method: Method,
+                                  specs: &[PackageIdSpec])
+                                  -> CargoResult<(PackageSet<'a>, Resolve)> {
     let mut registry = PackageRegistry::new(ws.config())?;
     if let Some(source) = source {
         registry.add_preloaded(source);
@@ -68,16 +80,6 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
         ops::load_pkg_lockfile(ws)?
     };
 
-    let method = if all_features {
-        Method::Everything
-    } else {
-        Method::Required {
-            dev_deps: true, // TODO: remove this option?
-            features: &features,
-            uses_default_features: !no_default_features,
-        }
-    };
-
     let resolved_with_overrides =
     ops::resolve_with_previous(&mut registry,
                                ws,
@@ -236,6 +238,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
                 let base = Method::Required {
                     dev_deps,
                     features: &[],
+                    all_features: false,
                     uses_default_features: true,
                 };
                 let member_id = member.package_id();

tests/testsuite/build.rs

@@ -4290,3 +4290,28 @@ fn no_linkable_target() {
                 [WARNING] The package `the_lib` provides no linkable [..] \
 while compiling `foo`. [..] in `the_lib`'s Cargo.toml. [..]"));
 }
+
+#[test]
+fn avoid_dev_deps() {
+    Package::new("foo", "1.0.0").publish();
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dev-dependencies]
+            baz = "1.0.0"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    // --bins is needed because of #5134
+    assert_that(p.cargo("build").arg("--bins"),
+        execs().with_status(101));
+    assert_that(p.cargo("build").arg("--bins")
+                .masquerade_as_nightly_cargo()
+                .arg("-Zavoid-dev-deps"),
+        execs().with_status(0));
+}

tests/testsuite/install.rs

@@ -3,6 +3,7 @@ use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 
 use cargo::util::ProcessBuilder;
+use cargotest::ChannelChanger;
 use cargotest::install::{cargo_home, has_installed_exe};
 use cargotest::support::git;
 use cargotest::support::paths;
@@ -904,6 +905,62 @@ fn use_path_workspace() {
     assert_eq!(lock, lock2, "different lockfiles");
 }
 
+#[test]
+fn dev_dependencies_no_check() {
+    Package::new("foo", "1.0.0").publish();
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dev-dependencies]
+            baz = "1.0.0"
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    // --bins is needed because of #5134
+    assert_that(p.cargo("build").arg("--bins"),
+        execs().with_status(101));
+    assert_that(p.cargo("install").arg("--bins"),
+        execs().with_status(0));
+}
+
+#[test]
+fn dev_dependencies_lock_file_untouched() {
+    Package::new("foo", "1.0.0").publish();
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [dev-dependencies]
+            bar = { path = "a" }
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file("a/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("a/src/lib.rs", "")
+        .build();
+
+    // --bins is needed because of #5134
+    assert_that(p.cargo("build").arg("--bins"),
+        execs().with_status(0));
+    let lock = p.read_lockfile();
+    assert_that(p.cargo("install").arg("--bins"),
+        execs().with_status(0));
+    let lock2 = p.read_lockfile();
+    assert!(lock == lock2, "different lockfiles");
+}
+
 #[test]
 fn vers_precise() {
     pkg("foo", "0.1.1");