Auto merge of #4743 - SimonSapin:default-members, r=matklad

Add a workspace.default-members config that overrides implied --all

Fixes #4507.

src/bin/bench.rs

@@ -109,8 +109,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
     let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
     let ws = Workspace::new(&root, config)?;
 
-    let spec = Packages::from_flags(ws.is_virtual(),
-                                    options.flag_all,
+    let spec = Packages::from_flags(options.flag_all,
                                     &options.flag_exclude,
                                     &options.flag_package)?;
 

src/bin/build.rs

@@ -100,8 +100,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
     let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
     let ws = Workspace::new(&root, config)?;
 
-    let spec = Packages::from_flags(ws.is_virtual(),
-                                    options.flag_all,
+    let spec = Packages::from_flags(options.flag_all,
                                     &options.flag_exclude,
                                     &options.flag_package)?;
 

src/bin/check.rs

@@ -106,8 +106,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
     let root = find_root_manifest_for_wd(options.flag_manifest_path, config.cwd())?;
     let ws = Workspace::new(&root, config)?;
 
-    let spec = Packages::from_flags(ws.is_virtual(),
-                                    options.flag_all,
+    let spec = Packages::from_flags(options.flag_all,
                                     &options.flag_exclude,
                                     &options.flag_package)?;
 

src/bin/test.rs

@@ -147,8 +147,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {
                                          options.flag_all_targets);
     }
 
-    let spec = Packages::from_flags(ws.is_virtual(),
-                                    options.flag_all,
+    let spec = Packages::from_flags(options.flag_all,
                                     &options.flag_exclude,
                                     &options.flag_package)?;
 

src/cargo/core/workspace.rs

@@ -46,6 +46,18 @@ pub struct Workspace<'cfg> {
     // set above.
     members: Vec<PathBuf>,
 
+    // The subset of `members` that are used by the
+    // `build`, `check`, `test`, and `bench` subcommands
+    // when no package is selected with `--package` / `-p` and `--all`
+    // is not used.
+    //
+    // This is set by the `default-members` config
+    // in the `[workspace]` section.
+    // When unset, this is the same as `members` for virtual workspaces
+    // (`--all` is implied)
+    // or only the root package for non-virtual workspaces.
+    default_members: Vec<PathBuf>,
+
     // True, if this is a temporary workspace created for the purposes of
     // cargo install or cargo package.
     is_ephemeral: bool,
@@ -90,6 +102,7 @@ pub enum WorkspaceConfig {
 pub struct WorkspaceRootConfig {
     root_dir: PathBuf,
     members: Option<Vec<String>>,
+    default_members: Option<Vec<String>>,
     exclude: Vec<String>,
 }
 
@@ -121,6 +134,7 @@ impl<'cfg> Workspace<'cfg> {
             root_manifest: None,
             target_dir: target_dir,
             members: Vec::new(),
+            default_members: Vec::new(),
             is_ephemeral: false,
             require_optional_deps: true,
         };
@@ -157,6 +171,7 @@ impl<'cfg> Workspace<'cfg> {
             root_manifest: None,
             target_dir: None,
             members: Vec::new(),
+            default_members: Vec::new(),
             is_ephemeral: true,
             require_optional_deps: require_optional_deps,
         };
@@ -170,6 +185,7 @@ impl<'cfg> Workspace<'cfg> {
                 ws.config.target_dir()?
             };
             ws.members.push(ws.current_manifest.clone());
+            ws.default_members.push(ws.current_manifest.clone());
         }
         Ok(ws)
     }
@@ -267,6 +283,14 @@ impl<'cfg> Workspace<'cfg> {
         }
     }
 
+    /// Returns an iterator over default packages in this workspace
+    pub fn default_members<'a>(&'a self) -> Members<'a, 'cfg> {
+        Members {
+            ws: self,
+            iter: self.default_members.iter(),
+        }
+    }
+
     pub fn is_ephemeral(&self) -> bool {
         self.is_ephemeral
     }
@@ -345,23 +369,51 @@ impl<'cfg> Workspace<'cfg> {
             None => {
                 debug!("find_members - only me as a member");
                 self.members.push(self.current_manifest.clone());
+                self.default_members.push(self.current_manifest.clone());
                 return Ok(())
             }
         };
 
-        let members_paths = {
+        let members_paths;
+        let default_members_paths;
+        {
             let root_package = self.packages.load(&root_manifest_path)?;
             match *root_package.workspace_config() {
-                WorkspaceConfig::Root(ref root_config) => root_config.members_paths()?,
+                WorkspaceConfig::Root(ref root_config) => {
+                    members_paths = root_config.members_paths(
+                        root_config.members.as_ref().unwrap_or(&vec![])
+                    )?;
+                    default_members_paths = if let Some(ref default) = root_config.default_members {
+                        Some(root_config.members_paths(default)?)
+                    } else {
+                        None
+                    }
+                }
                 _ => bail!("root of a workspace inferred but wasn't a root: {}",
                            root_manifest_path.display()),
             }
-        };
+        }
 
         for path in members_paths {
             self.find_path_deps(&path.join("Cargo.toml"), &root_manifest_path, false)?;
         }
 
+        if let Some(default) = default_members_paths {
+            for path in default {
+                let manifest_path = paths::normalize_path(&path.join("Cargo.toml"));
+                if !self.members.contains(&manifest_path) {
+                    bail!("package `{}` is listed in workspace’s default-members \
+                           but is not a member.",
+                          path.display())
+                }
+                self.default_members.push(manifest_path)
+            }
+        } else if self.is_virtual() {
+            self.default_members = self.members.clone()
+        } else {
+            self.default_members.push(self.current_manifest.clone())
+        }
+
         self.find_path_deps(&root_manifest_path, &root_manifest_path, false)
     }
 
@@ -370,7 +422,7 @@ impl<'cfg> Workspace<'cfg> {
                       root_manifest: &Path,
                       is_path_dep: bool) -> CargoResult<()> {
         let manifest_path = paths::normalize_path(manifest_path);
-        if self.members.iter().any(|p| p == &manifest_path) {
+        if self.members.contains(&manifest_path) {
             return Ok(())
         }
         if is_path_dep
@@ -632,11 +684,13 @@ impl WorkspaceRootConfig {
     pub fn new(
         root_dir: &Path,
         members: &Option<Vec<String>>,
+        default_members: &Option<Vec<String>>,
         exclude: &Option<Vec<String>>,
     ) -> WorkspaceRootConfig {
         WorkspaceRootConfig {
             root_dir: root_dir.to_path_buf(),
             members: members.clone(),
+            default_members: default_members.clone(),
             exclude: exclude.clone().unwrap_or_default(),
         }
     }
@@ -665,21 +719,19 @@ impl WorkspaceRootConfig {
         self.members.is_some()
     }
 
-    fn members_paths(&self) -> CargoResult<Vec<PathBuf>> {
+    fn members_paths(&self, globs: &[String]) -> CargoResult<Vec<PathBuf>> {
         let mut expanded_list = Vec::new();
 
-        if let Some(globs) = self.members.clone() {
-            for glob in globs {
-                let pathbuf = self.root_dir.join(glob);
-                let expanded_paths = Self::expand_member_path(&pathbuf)?;
-
-                // If glob does not find any valid paths, then put the original
-                // path in the expanded list to maintain backwards compatibility.
-                if expanded_paths.is_empty() {
-                    expanded_list.push(pathbuf);
-                } else {
-                    expanded_list.extend(expanded_paths);
-                }
+        for glob in globs {
+            let pathbuf = self.root_dir.join(glob);
+            let expanded_paths = Self::expand_member_path(&pathbuf)?;
+
+            // If glob does not find any valid paths, then put the original
+            // path in the expanded list to maintain backwards compatibility.
+            if expanded_paths.is_empty() {
+                expanded_list.push(pathbuf);
+            } else {
+                expanded_list.extend(expanded_paths);
             }
         }
 

src/cargo/ops/cargo_compile.rs

@@ -106,26 +106,23 @@ pub enum MessageFormat {
 
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum Packages<'a> {
+    Default,
     All,
     OptOut(&'a [String]),
     Packages(&'a [String]),
 }
 
 impl<'a> Packages<'a> {
-    pub fn from_flags(virtual_ws: bool, all: bool, exclude: &'a [String], package: &'a [String])
+    pub fn from_flags(all: bool, exclude: &'a [String], package: &'a [String])
         -> CargoResult<Self>
     {
-        let all = all || (virtual_ws && package.is_empty());
-
-        let packages = match (all, &exclude) {
-            (true, exclude) if exclude.is_empty() => Packages::All,
-            (true, exclude) => Packages::OptOut(exclude),
-            (false, exclude) if !exclude.is_empty() => bail!("--exclude can only be used together \
-                                                           with --all"),
-            _ => Packages::Packages(package),
-        };
-
-        Ok(packages)
+        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),
+        })
     }
 
     pub fn into_package_id_specs(self, ws: &Workspace) -> CargoResult<Vec<PackageIdSpec>> {
@@ -152,6 +149,12 @@ impl<'a> Packages<'a> {
             Packages::Packages(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()
+            }
         };
         Ok(specs)
     }

src/cargo/ops/cargo_run.rs

@@ -11,7 +11,8 @@ pub fn run(ws: &Workspace,
     let config = ws.config();
 
     let pkg = match options.spec {
-        Packages::All => unreachable!("cargo run supports single package only"),
+        Packages::All |
+        Packages::Default |
         Packages::OptOut(_) => unreachable!("cargo run supports single package only"),
         Packages::Packages(xs) => match xs.len() {
             0 => ws.current()?,

src/cargo/util/toml/mod.rs

@@ -450,6 +450,8 @@ pub struct TomlProject {
 #[derive(Debug, Deserialize, Serialize)]
 pub struct TomlWorkspace {
     members: Option<Vec<String>>,
+    #[serde(rename = "default-members")]
+    default_members: Option<Vec<String>>,
     exclude: Option<Vec<String>>,
 }
 
@@ -681,7 +683,9 @@ impl TomlManifest {
                                       project.workspace.as_ref()) {
             (Some(config), None) => {
                 WorkspaceConfig::Root(
-                    WorkspaceRootConfig::new(&package_root, &config.members, &config.exclude)
+                    WorkspaceRootConfig::new(
+                        &package_root, &config.members, &config.default_members, &config.exclude,
+                    )
                 )
             }
             (None, root) => {
@@ -785,7 +789,9 @@ impl TomlManifest {
         let workspace_config = match me.workspace {
             Some(ref config) => {
                 WorkspaceConfig::Root(
-                    WorkspaceRootConfig::new(&root, &config.members, &config.exclude)
+                    WorkspaceRootConfig::new(
+                        &root, &config.members, &config.default_members, &config.exclude,
+                    )
                 )
             }
             None => {

src/doc/manifest.md

@@ -520,9 +520,19 @@ crate will be treated as a normal package, as well as a workspace. If the
 manifest*.
 
 When working with *virtual manifests*, package-related cargo commands, like
-`cargo build`, won't be available anymore. But, most of such commands support
-the `--all` option, will execute the command for all the non-virtual manifest in
-the workspace.
+`cargo build`, default to the set of packages specified by the `default-members`
+configuration:
+
+```toml
+[workspace]
+members = ["path/to/member1", "path/to/member2", "path/to/member3/*"]
+
+# The members that commands like `cargo build` apply to by deault.
+# This must expand to a subset of `members`.
+# Optional key, defaults to the same as `members`
+# (as if `--all` were used on the command line).
+default-members = ["path/to/member2", "path/to/member3/*"]
+```
 
 # The project layout