In "cargo install" directly from registry, don't require optional dependencies

When building with a directory registry that contains only the subset of
crates required to build an application crate, cargo fails if that
subset doesn't include optional dependencies pulled in for every
possible feature of the root crate, even when the install doesn't enable
those features.  This prevents Linux distributions from building with
a minimal set of dependencies (omitting, for instance, packages for
unstable/nightly features).

Introduce a new workspace flag "require_optional_deps", disabled for
install and enabled for everything else.  Skip the initial
Method::Everything resolve in this case, and modify
resolve_with_previous to support running a Method::Required resolve
without a previous resolution.

This also skips adding path overrides, as those won't make sense (and
won't work) for an install directly from a registry.

Introduce a set of tests for "cargo install" directly from a directory
registry.

Author: joshtriplett

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();