Auto merge of #3249 - matklad:workspace-profiles, r=alexcrichton

Use a single profile set per workspace

This aims to close #3206.

I have not figured out how to do this 100% backward compatibly, that's why I want to discuss this separately, although a related PR (#3221) is already in flight.

The problem is this: suppose that you have a workspace with two members, A and B and that A includes a profiles section and B does not. Now, mentally `cd` inside B and run `cargo build`. Today, Cargo will use a default profile. We want it to use a profile from A. So here the silent behavior switch will inevitably occur :( Looks like we can't detect this situation.

So this PR just switches the behavior to always use root profiles, and to print a warning if a non-root package specifies profiles. Feel free to reuse it in any form for #3221 if that's convenient!

Author: bors

src/cargo/core/manifest.rs

@@ -33,6 +33,7 @@ pub struct Manifest {
 pub struct VirtualManifest {
     replace: Vec<(PackageIdSpec, Dependency)>,
     workspace: WorkspaceConfig,
+    profiles: Profiles,
 }
 
 /// General metadata about a package which is just blindly uploaded to the
@@ -139,7 +140,7 @@ pub struct Profile {
     pub panic: Option<String>,
 }
 
-#[derive(Default, Clone, Debug)]
+#[derive(Default, Clone, Debug, PartialEq, Eq)]
 pub struct Profiles {
     pub release: Profile,
     pub dev: Profile,
@@ -250,10 +251,12 @@ impl Manifest {
 
 impl VirtualManifest {
     pub fn new(replace: Vec<(PackageIdSpec, Dependency)>,
-               workspace: WorkspaceConfig) -> VirtualManifest {
+               workspace: WorkspaceConfig,
+               profiles: Profiles) -> VirtualManifest {
         VirtualManifest {
             replace: replace,
             workspace: workspace,
+            profiles: profiles,
         }
     }
 
@@ -264,6 +267,10 @@ impl VirtualManifest {
     pub fn workspace_config(&self) -> &WorkspaceConfig {
         &self.workspace
     }
+
+    pub fn profiles(&self) -> &Profiles {
+        &self.profiles
+    }
 }
 
 impl Target {

src/cargo/core/workspace.rs

@@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
 use std::slice;
 
 use core::{Package, VirtualManifest, EitherManifest, SourceId};
-use core::{PackageIdSpec, Dependency};
+use core::{PackageIdSpec, Dependency, Profile, Profiles};
 use ops;
 use util::{Config, CargoResult, Filesystem, human};
 use util::paths;
@@ -162,6 +162,14 @@ impl<'cfg> Workspace<'cfg> {
         self.config
     }
 
+    pub fn profiles(&self) -> &Profiles {
+        let root = self.root_manifest.as_ref().unwrap_or(&self.current_manifest);
+        match *self.packages.get(root) {
+            MaybePackage::Package(ref p) => p.manifest().profiles(),
+            MaybePackage::Virtual(ref m) => m.profiles(),
+        }
+    }
+
     /// Returns the root path of this workspace.
     ///
     /// That is, this returns the path of the directory containing the
@@ -432,6 +440,33 @@ impl<'cfg> Workspace<'cfg> {
                   extra);
         }
 
+        if let Some(ref root_manifest) = self.root_manifest {
+            let default_profiles = Profiles {
+                release: Profile::default_release(),
+                dev: Profile::default_dev(),
+                test: Profile::default_test(),
+                test_deps: Profile::default_dev(),
+                bench: Profile::default_bench(),
+                bench_deps: Profile::default_release(),
+                doc: Profile::default_doc(),
+                custom_build: Profile::default_custom_build(),
+            };
+
+            for pkg in self.members().filter(|p| p.manifest_path() != root_manifest) {
+                if pkg.manifest().profiles() != &default_profiles {
+                    let message = &format!("profiles for the non root package will be ignored, \
+                                            specify profiles at the workspace root:\n\
+                                            package:   {}\n\
+                                            workspace: {}",
+                                           pkg.manifest_path().display(),
+                                           root_manifest.display());
+
+                    //TODO: remove `Eq` bound from `Profiles` when the warning is removed.
+                    try!(self.config.shell().warn(&message));
+                }
+            }
+        }
+
         Ok(())
     }
 }

src/cargo/ops/cargo_clean.rs

@@ -32,7 +32,7 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> {
     let resolve = try!(ops::resolve_ws(&mut registry, ws));
     let packages = ops::get_resolved_packages(&resolve, registry);
 
-    let profiles = try!(ws.current()).manifest().profiles();
+    let profiles = ws.profiles();
     let host_triple = try!(opts.config.rustc()).host.clone();
     let mut cx = try!(Context::new(ws, &resolve, &packages, opts.config,
                                    BuildConfig {

src/cargo/ops/cargo_compile.rs

@@ -169,7 +169,7 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
         bail!("jobs must be at least 1")
     }
 
-    let profiles = root_package.manifest().profiles();
+    let profiles = ws.profiles();
     if spec.len() == 0 {
         try!(generate_targets(root_package, profiles, mode, filter, release));
     }

src/cargo/util/toml.rs

@@ -726,6 +726,7 @@ impl TomlManifest {
             platform: None,
             layout: layout,
         }));
+        let profiles = build_profiles(&self.profile);
         let workspace_config = match self.workspace {
             Some(ref config) => {
                 WorkspaceConfig::Root { members: config.members.clone() }
@@ -734,7 +735,7 @@ impl TomlManifest {
                 bail!("virtual manifests must be configured with [workspace]");
             }
         };
-        Ok((VirtualManifest::new(replace, workspace_config), nested_paths))
+        Ok((VirtualManifest::new(replace, workspace_config, profiles), nested_paths))
     }
 
     fn replace(&self, cx: &mut Context)

tests/profiles.rs

@@ -187,3 +187,71 @@ fn top_level_overrides_deps() {
                     prefix = env::consts::DLL_PREFIX,
                     suffix = env::consts::DLL_SUFFIX)));
 }
+
+#[test]
+fn profile_in_non_root_manifest_triggers_a_warning() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [workspace]
+            members = ["bar"]
+
+            [profile.dev]
+            debug = false
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file("bar/Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+            workspace = ".."
+
+            [profile.dev]
+            opt-level = 1
+        "#)
+        .file("bar/src/main.rs", "fn main() {}");
+    p.build();
+
+    assert_that(p.cargo_process("build").cwd(p.root().join("bar")).arg("-v"),
+                execs().with_status(0).with_stderr("\
+[WARNING] profiles for the non root package will be ignored, specify profiles at the workspace root:
+package:   [..]
+workspace: [..]
+[COMPILING] bar v0.1.0 ([..])
+[RUNNING] `rustc [..]`
+[FINISHED] debug [unoptimized] target(s) in [..]"));
+}
+
+#[test]
+fn profile_in_virtual_manifest_works() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["bar"]
+
+            [profile.dev]
+            opt-level = 1
+            debug = false
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .file("bar/Cargo.toml", r#"
+            [project]
+            name = "bar"
+            version = "0.1.0"
+            authors = []
+            workspace = ".."
+        "#)
+        .file("bar/src/main.rs", "fn main() {}");
+    p.build();
+
+    assert_that(p.cargo_process("build").cwd(p.root().join("bar")).arg("-v"),
+                execs().with_status(0).with_stderr("\
+[COMPILING] bar v0.1.0 ([..])
+[RUNNING] `rustc [..]`
+[FINISHED] debug [optimized] target(s) in [..]"));
+}