cargo fails if it can't find optional dependencies, even if corresponding feature not enabled

src/cargo/core/workspace.rs

@@ -44,6 +44,11 @@ pub struct Workspace<'cfg> {
     // True, if this is a temporary workspace created for the purposes of
     // cargo install or cargo package.
     is_ephemeral: bool,
+
+    // 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).
+    require_optional_deps: bool,
 }
 
 // Separate structure for tracking loaded packages (to avoid loading anything
@@ -99,6 +104,7 @@ impl<'cfg> Workspace<'cfg> {
             target_dir: target_dir,
             members: Vec::new(),
             is_ephemeral: false,
+            require_optional_deps: true,
         };
         ws.root_manifest = ws.find_root(manifest_path)?;
         ws.find_members()?;
@@ -115,8 +121,8 @@ impl<'cfg> Workspace<'cfg> {
     ///
     /// This is currently only used in niche situations like `cargo install` or
     /// `cargo package`.
-    pub fn ephemeral(package: Package, config: &'cfg Config, target_dir: Option<Filesystem>)
-                     -> CargoResult<Workspace<'cfg>> {
+    pub fn ephemeral(package: Package, config: &'cfg Config, target_dir: Option<Filesystem>,
+                     require_optional_deps: bool) -> CargoResult<Workspace<'cfg>> {
         let mut ws = Workspace {
             config: config,
             current_manifest: package.manifest_path().to_path_buf(),
@@ -128,6 +134,7 @@ impl<'cfg> Workspace<'cfg> {
             target_dir: None,
             members: Vec::new(),
             is_ephemeral: true,
+            require_optional_deps: require_optional_deps,
         };
         {
             let key = ws.current_manifest.parent().unwrap();
@@ -219,6 +226,10 @@ impl<'cfg> Workspace<'cfg> {
         self.is_ephemeral
     }
 
+    pub fn require_optional_deps(&self) -> bool {
+        self.require_optional_deps
+    }
+
     /// Finds the root of a workspace for the crate whose manifest is located
     /// at `manifest_path`.
     ///

src/cargo/ops/cargo_install.rs

@@ -97,7 +97,7 @@ pub fn install(root: Option<&str>,
     };
 
     let ws = match overidden_target_dir {
-        Some(dir) => Workspace::ephemeral(pkg, config, Some(dir))?,
+        Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?,
         None => Workspace::new(pkg.manifest_path(), config)?,
     };
     let pkg = ws.current()?;

src/cargo/ops/cargo_package.rs

@@ -284,7 +284,7 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()>
     let new_pkg = Package::new(new_manifest, &manifest_path);
 
     // Now that we've rewritten all our path dependencies, compile it!
-    let ws = Workspace::ephemeral(new_pkg, config, None)?;
+    let ws = Workspace::ephemeral(new_pkg, config, None, true)?;
     ops::compile_ws(&ws, None, &ops::CompileOptions {
         config: config,
         jobs: opts.jobs,

src/cargo/ops/resolve.rs

@@ -37,16 +37,22 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
         registry.add_preloaded(source);
     }
 
-    // First, resolve the root_package's *listed* dependencies, as well as
-    // downloading and updating all remotes and such.
-    let resolve = resolve_with_registry(ws, &mut registry)?;
+    let resolve = if ws.require_optional_deps() {
+        // First, resolve the root_package's *listed* dependencies, as well as
+        // downloading and updating all remotes and such.
+        let resolve = resolve_with_registry(ws, &mut registry)?;
+
+        // Second, resolve with precisely what we're doing. Filter out
+        // transitive dependencies if necessary, specify features, handle
+        // overrides, etc.
+        let _p = profile::start("resolving w/ overrides...");
 
-    // Second, resolve with precisely what we're doing. Filter out
-    // transitive dependencies if necessary, specify features, handle
-    // overrides, etc.
-    let _p = profile::start("resolving w/ overrides...");
+        add_overrides(&mut registry, ws)?;
 
-    add_overrides(&mut registry, ws)?;
+        Some(resolve)
+    } else {
+        None
+    };
 
     let method = if all_features {
         Method::Everything
@@ -60,7 +66,7 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
 
     let resolved_with_overrides =
     ops::resolve_with_previous(&mut registry, ws,
-                               method, Some(&resolve), None,
+                               method, resolve.as_ref(), None,
                                specs)?;
 
     for &(ref replace_spec, _) in ws.root_replace() {
@@ -159,8 +165,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
             // members in the workspace, so propagate the `Method::Everything`.
             Method::Everything => Method::Everything,
 
-            // If we're not resolving everything though then the workspace is
-            // already resolved and now we're drilling down from that to the
+            // If we're not resolving everything though then we're constructing the
             // exact crate graph we're going to build. Here we don't necessarily
             // want to keep around all workspace crates as they may not all be
             // built/tested.
@@ -176,7 +181,6 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
             // base method with no features specified but using default features
             // for any other packages specified with `-p`.
             Method::Required { dev_deps, .. } => {
-                assert!(previous.is_some());
                 let base = Method::Required {
                     dev_deps: dev_deps,
                     features: &[],

tests/directory.rs

@@ -10,6 +10,7 @@ use std::str;
 
 use rustc_serialize::json;
 
+use cargotest::cargo_process;
 use cargotest::support::{project, execs, ProjectBuilder};
 use cargotest::support::paths;
 use cargotest::support::registry::{Package, cksum};
@@ -105,6 +106,144 @@ fn simple() {
 "));
 }
 
+#[test]
+fn simple_install() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(0).with_stderr(
+"  Installing bar v0.1.0
+   Compiling foo v0.1.0
+   Compiling bar v0.1.0
+    Finished release [optimized] target(s) in [..] secs
+  Installing [..]bar[..]
+warning: be sure to add `[..]` to your PATH to be able to run the installed binaries
+"));
+}
+
+#[test]
+fn simple_install_fail() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+            baz = "9.8.7"
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(101).with_stderr(
+"  Installing bar v0.1.0
+error: failed to compile `bar v0.1.0`, intermediate artifacts can be found at `[..]`
+
+Caused by:
+  no matching package named `baz` found (required by `bar`)
+location searched: registry https://github.com/rust-lang/crates.io-index
+version required: ^9.8.7
+"));
+}
+
+#[test]
+fn install_without_feature_dep() {
+    setup();
+
+    VendorPackage::new("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "pub fn foo() {}")
+        .build();
+
+    VendorPackage::new("bar")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = "0.1.0"
+            baz = { version = "9.8.7", optional = true }
+
+            [features]
+            wantbaz = ["baz"]
+        "#)
+        .file("src/main.rs", r#"
+            extern crate foo;
+
+            pub fn main() {
+                foo::foo();
+            }
+        "#)
+        .build();
+
+    assert_that(cargo_process().arg("install").arg("bar"),
+                execs().with_status(0).with_stderr(
+"  Installing bar v0.1.0
+   Compiling foo v0.1.0
+   Compiling bar v0.1.0
+    Finished release [optimized] target(s) in [..] secs
+  Installing [..]bar[..]
+warning: be sure to add `[..]` to your PATH to be able to run the installed binaries
+"));
+}
+
 #[test]
 fn not_there() {
     setup();