WIP: Features vs packages

src/bin/command_prelude.rs

@@ -3,9 +3,9 @@ use std::fs;
 
 use clap::{self, SubCommand};
 use cargo::CargoResult;
-use cargo::core::Workspace;
-use cargo::ops::{CompileFilter, CompileMode, CompileOptions, MessageFormat, NewOptions, Packages,
-                 VersionControl};
+use cargo::core::{Workspace, Package, PackageIdSpec};
+use cargo::ops::{CompileFilter, CompileMode, CompileOptions, MessageFormat, NewOptions,
+                 VersionControl, RequestedPackages};
 use cargo::util::paths;
 use cargo::util::important_paths::find_root_manifest_for_wd;
 
@@ -240,15 +240,85 @@ pub trait ArgMatchesExt {
     }
 
     fn compile_options<'a>(
+        &self,
+        ws: &Workspace<'a>,
+        mode: CompileMode,
+    ) -> CargoResult<CompileOptions<'a>> {
+        self._compile_options(ws.config(), Some(ws), mode)
+    }
+
+    fn compile_options_for_single_package<'a>(
+        &self,
+        ws: &Workspace<'a>,
+        mode: CompileMode,
+    ) -> CargoResult<CompileOptions<'a>> {
+        let compile_opts = self.compile_options(ws, mode)?;
+        if compile_opts.requested.specs.len() != 1 {
+            ws.current()?;
+            unreachable!("More than one package requested => current should be Err")
+        }
+        Ok(compile_opts)
+    }
+
+    fn compile_options_for_install<'a>(
         &self,
         config: &'a Config,
+    ) -> CargoResult<CompileOptions<'a>> {
+        self._compile_options(config, None, CompileMode::Build)
+    }
+
+    fn _compile_options<'a>(
+        &self,
+        config: &'a Config,
+        ws: Option<&Workspace<'a>>,
         mode: CompileMode,
     ) -> CargoResult<CompileOptions<'a>> {
-        let spec = Packages::from_flags(
-            self._is_present("all"),
-            self._values_of("exclude"),
-            self._values_of("package"),
-        )?;
+
+        let requested = match ws {
+            // Will determine precise set later, see RequestedPackages::set_package
+            None => RequestedPackages::default(),
+            Some(ws) => {
+                let all = self._is_present("all");
+                let exclude = self._values_of("exclude");
+                let package = self._values_of("package");
+                let specs = match (all, exclude.is_empty(), package.is_empty()) {
+                    (_, _, true) =>
+                        (if all { ws.members() } else { ws.default_members() })
+                            .map(Package::package_id)
+                            .map(PackageIdSpec::from_package_id)
+                            .filter(|p| exclude.iter().position(|x| *x == p.name()).is_none())
+                            .collect(),
+                    (false, true, false) => {
+                        package.iter()
+                            .map(|p| PackageIdSpec::parse(p))
+                            .collect::<CargoResult<Vec<_>>>()?
+                    },
+                    (false, false, _) => bail!("--exclude can only be used together with --all"),
+                    (true, _, false) => bail!("--package cannot be used together with --all"),
+                };
+
+                if specs.len() > 1 && self._is_present("features") {
+                    bail!("cannot specify features for more than one package")
+                }
+                if specs.is_empty() {
+                    if ws.is_virtual() {
+                        bail!(
+                            "manifest path `{}` contains no package: The manifest is virtual, \
+                             and the workspace has no members.",
+                             ws.root().display()
+                        )
+                    }
+                    bail!("no packages to compile")
+                }
+
+                RequestedPackages {
+                    specs,
+                    features: self._values_of("features"),
+                    all_features: self._is_present("all-features"),
+                    no_default_features: self._is_present("no-default-features"),
+                }
+            }
+        };
 
         let message_format = match self._value_of("message-format") {
             None => MessageFormat::Human,
@@ -270,7 +340,7 @@ pub trait ArgMatchesExt {
             features: self._values_of("features"),
             all_features: self._is_present("all-features"),
             no_default_features: self._is_present("no-default-features"),
-            spec,
+            requested,
             mode,
             release: self._is_present("release"),
             filter: CompileFilter::new(
@@ -293,16 +363,6 @@ pub trait ArgMatchesExt {
         Ok(opts)
     }
 
-    fn compile_options_for_single_package<'a>(
-        &self,
-        config: &'a Config,
-        mode: CompileMode,
-    ) -> CargoResult<CompileOptions<'a>> {
-        let mut compile_opts = self.compile_options(config, mode)?;
-        compile_opts.spec = Packages::Packages(self._values_of("package"));
-        Ok(compile_opts)
-    }
-
     fn new_options(&self, config: &Config) -> CargoResult<NewOptions> {
         let vcs = self._value_of("vcs").map(|vcs| match vcs {
             "git" => VersionControl::Git,

src/bin/commands/bench.rs

@@ -71,7 +71,7 @@ Compilation can be customized with the `bench` profile in the manifest.
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
-    let mut compile_opts = args.compile_options(config, CompileMode::Bench)?;
+    let mut compile_opts = args.compile_options(&ws, CompileMode::Bench)?;
     compile_opts.release = true;
 
     let ops = TestOptions {

src/bin/commands/build.rs

@@ -50,7 +50,7 @@ the --release flag will use the `release` profile instead.
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
-    let mut compile_opts = args.compile_options(config, CompileMode::Build)?;
+    let mut compile_opts = args.compile_options(&ws, CompileMode::Build)?;
     compile_opts.export_dir = args.value_of_path("out-dir", config);
     if compile_opts.export_dir.is_some() && !config.cli_unstable().unstable_options {
         Err(format_err!(

src/bin/commands/check.rs

@@ -65,7 +65,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         }
     };
     let mode = CompileMode::Check { test };
-    let compile_opts = args.compile_options(config, mode)?;
+    let compile_opts = args.compile_options(&ws, mode)?;
     ops::compile(&ws, &compile_opts)?;
     Ok(())
 }

src/bin/commands/doc.rs

@@ -48,7 +48,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let mode = CompileMode::Doc {
         deps: !args.is_present("no-deps"),
     };
-    let compile_opts = args.compile_options(config, mode)?;
+    let compile_opts = args.compile_options(&ws, mode)?;
     let doc_opts = DocOptions {
         open_result: args.is_present("open"),
         compile_opts,

src/bin/commands/install.rs

@@ -1,7 +1,7 @@
 use command_prelude::*;
 
 use cargo::core::{GitReference, SourceId};
-use cargo::ops::{self, CompileMode};
+use cargo::ops;
 use cargo::util::ToUrl;
 
 pub fn cli() -> App {
@@ -72,7 +72,7 @@ continuous integration systems.",
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
-    let mut compile_opts = args.compile_options(config, CompileMode::Build)?;
+    let mut compile_opts = args.compile_options_for_install(config)?;
     compile_opts.release = !args.is_present("debug");
 
     let krates = args.values_of("crate")
@@ -114,7 +114,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             &source,
             from_cwd,
             version,
-            &compile_opts,
+            &mut compile_opts,
             args.is_present("force"),
         )?;
     }

src/bin/commands/run.rs

@@ -37,7 +37,7 @@ run. If you're passing arguments to both Cargo and the binary, the ones after
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
 
-    let mut compile_opts = args.compile_options_for_single_package(config, CompileMode::Build)?;
+    let mut compile_opts = args.compile_options_for_single_package(&ws, CompileMode::Build)?;
     if !args.is_present("example") && !args.is_present("bin") {
         compile_opts.filter = CompileFilter::Default {
             required_features_filterable: false,

src/bin/commands/rustc.rs

@@ -61,7 +61,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             return Err(CliError::new(err, 101));
         }
     };
-    let mut compile_opts = args.compile_options_for_single_package(config, mode)?;
+    let mut compile_opts = args.compile_options_for_single_package(&ws, mode)?;
     let target_args = values(args, "args");
     compile_opts.target_rustc_args = if target_args.is_empty() {
         None

src/bin/commands/rustdoc.rs

@@ -49,7 +49,7 @@ the `cargo help pkgid` command.
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
     let mut compile_opts =
-        args.compile_options_for_single_package(config, CompileMode::Doc { deps: false })?;
+        args.compile_options_for_single_package(&ws, CompileMode::Doc { deps: false })?;
     let target_args = values(args, "args");
     compile_opts.target_rustdoc_args = if target_args.is_empty() {
         None

src/bin/commands/test.rs

@@ -90,7 +90,7 @@ To get the list of all options available for the test binaries use this:
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let ws = args.workspace(config)?;
 
-    let mut compile_opts = args.compile_options(config, CompileMode::Test)?;
+    let mut compile_opts = args.compile_options(&ws, CompileMode::Test)?;
     let doc = args.is_present("doc");
     if doc {
         compile_opts.mode = ops::CompileMode::Doctest;

src/cargo/ops/cargo_compile.rs

@@ -50,7 +50,7 @@ pub struct CompileOptions<'a> {
     /// Flag if the default feature should be built for the root package
     pub no_default_features: bool,
     /// A set of packages to build.
-    pub spec: Packages,
+    pub requested: RequestedPackages,
     /// Filter to apply to the root package to select which targets will be
     /// built.
     pub filter: CompileFilter,
@@ -74,6 +74,7 @@ pub struct CompileOptions<'a> {
 }
 
 impl<'a> CompileOptions<'a> {
+    // Used in RLS
     pub fn default(config: &'a Config, mode: CompileMode) -> CompileOptions<'a> {
         CompileOptions {
             config,
@@ -82,7 +83,7 @@ impl<'a> CompileOptions<'a> {
             features: Vec::new(),
             all_features: false,
             no_default_features: false,
-            spec: ops::Packages::Packages(Vec::new()),
+            requested: RequestedPackages::default(),
             mode,
             release: false,
             filter: CompileFilter::Default {
@@ -112,61 +113,49 @@ pub enum MessageFormat {
     Json,
 }
 
-#[derive(Clone, PartialEq, Eq, Debug)]
-pub enum Packages {
-    Default,
-    All,
-    OptOut(Vec<String>),
-    Packages(Vec<String>),
+#[derive(Default, Debug)]
+pub struct RequestedPackages {
+    pub specs: Vec<PackageIdSpec>,
+    /// Extra features to build for the root package
+    // invariant: features.len() > 0 => specs.len() == 1
+    pub features: Vec<String>,
+    /// Flag whether all available features should be built for the root package
+    pub all_features: bool,
+    /// Flag if the default feature should be built for the root package
+    pub no_default_features: bool,
 }
 
-impl Packages {
-    pub fn from_flags(all: bool, exclude: Vec<String>, package: Vec<String>) -> CargoResult<Self> {
-        Ok(match (all, exclude.len(), package.len()) {
-            (false, 0, 0) => Packages::Default,
-            (false, 0, _) => Packages::Packages(package),
-            (false, _, _) => bail!("--exclude can only be used together with --all"),
-            (true, 0, _) => Packages::All,
-            (true, _, _) => Packages::OptOut(exclude),
-        })
+impl RequestedPackages {
+    pub fn single(id: &PackageId) -> RequestedPackages {
+        RequestedPackages {
+            specs: vec![PackageIdSpec::from_package_id(id)],
+            features: Vec::new(),
+            all_features: false,
+            no_default_features: false,
+        }
     }
 
-    pub fn into_package_id_specs(&self, ws: &Workspace) -> CargoResult<Vec<PackageIdSpec>> {
-        let specs = match *self {
-            Packages::All => ws.members()
-                .map(Package::package_id)
-                .map(PackageIdSpec::from_package_id)
-                .collect(),
-            Packages::OptOut(ref opt_out) => ws.members()
-                .map(Package::package_id)
-                .map(PackageIdSpec::from_package_id)
-                .filter(|p| opt_out.iter().position(|x| *x == p.name()).is_none())
+    pub fn whole_workspace(ws: &Workspace) -> RequestedPackages {
+        RequestedPackages {
+            specs: ws.members().map(|pkg| PackageIdSpec::from_package_id(pkg.package_id()))
                 .collect(),
-            Packages::Packages(ref packages) if packages.is_empty() => ws.current_opt()
-                .map(Package::package_id)
-                .map(PackageIdSpec::from_package_id)
-                .into_iter()
-                .collect(),
-            Packages::Packages(ref packages) => packages
-                .iter()
-                .map(|p| PackageIdSpec::parse(p))
-                .collect::<CargoResult<Vec<_>>>()?,
-            Packages::Default => ws.default_members()
-                .map(Package::package_id)
-                .map(PackageIdSpec::from_package_id)
-                .collect(),
-        };
-        if specs.is_empty() {
-            if ws.is_virtual() {
-                bail!(
-                    "manifest path `{}` contains no package: The manifest is virtual, \
-                     and the workspace has no members.",
-                    ws.root().display()
-                )
-            }
-            bail!("no packages to compile")
+            features: Vec::new(),
+            all_features: true,
+            no_default_features: false,
         }
-        Ok(specs)
+    }
+
+    pub fn contains(&self, id: &PackageId) -> bool {
+        self.specs.iter().any(|s| s.matches(id))
+    }
+
+    // Normally, we determine the set of requested packages from the current
+    // workspace when parsing cli flags. For `cargo install` however, we don't
+    // have workspace available at that time, so we need this method to set them
+    // later, after we create a temporary workspace.
+    pub fn set_package(&mut self, pkg: &Package) {
+        let id = pkg.package_id();
+        self.specs = vec![PackageIdSpec::from_package_id(id)];
     }
 }
 
@@ -231,10 +220,10 @@ pub fn compile_ws<'a>(
         config,
         jobs,
         ref target,
-        ref spec,
         ref features,
         all_features,
         no_default_features,
+        ref requested,
         release,
         mode,
         message_format,
@@ -270,18 +259,17 @@ pub fn compile_ws<'a>(
 
     let profiles = ws.profiles();
 
-    let specs = spec.into_package_id_specs(ws)?;
     let features = Method::split_features(features);
     let method = Method::Required {
         dev_deps: ws.require_optional_deps() || filter.need_dev_deps(mode),
         features: &features,
         all_features,
         uses_default_features: !no_default_features,
     };
-    let resolve = ops::resolve_ws_with_method(ws, source, method, &specs)?;
+    let resolve = ops::resolve_ws_with_method(ws, source, method, requested)?;
     let (packages, resolve_with_overrides) = resolve;
 
-    let to_builds = specs
+    let to_builds = requested.specs
         .iter()
         .map(|p| {
             let pkgid = p.query(resolve_with_overrides.iter())?;