Auto merge of #5491 - ehuss:profile-spec, r=alexcrichton

Add package spec support to profile overrides.

Note: It errors out if multiple overrides match the same package.  This could potentially merge the overrides together with a hierarchy similar to CSS specificity.  However, that seems overkill for now.

src/cargo/core/compiler/context/unit_dependencies.rs

@@ -329,7 +329,7 @@ fn new_unit<'a>(
     mode: CompileMode,
 ) -> Unit<'a> {
     let profile = bcx.profiles.get_profile(
-        &pkg.name(),
+        &pkg.package_id(),
         bcx.ws.is_member(pkg),
         profile_for,
         mode,

src/cargo/core/package_id_spec.rs

@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use std::fmt;
 
 use semver::Version;
+use serde::{de, ser};
 use url::Url;
 
 use core::PackageId;
@@ -17,7 +18,7 @@ use util::errors::{CargoResult, CargoResultExt};
 /// If any of the optional fields are omitted, then the package id may be ambiguous, there may be
 /// more than one package/version/url combo that will match. However, often just the name is
 /// sufficient to uniquely define a package id.
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)]
 pub struct PackageIdSpec {
     name: String,
     version: Option<Version>,
@@ -253,6 +254,25 @@ impl fmt::Display for PackageIdSpec {
     }
 }
 
+impl ser::Serialize for PackageIdSpec {
+    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        self.to_string().serialize(s)
+    }
+}
+
+impl<'de> de::Deserialize<'de> for PackageIdSpec {
+    fn deserialize<D>(d: D) -> Result<PackageIdSpec, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        let string = String::deserialize(d)?;
+        PackageIdSpec::parse(&string).map_err(de::Error::custom)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use core::{PackageId, SourceId};

src/cargo/core/profiles.rs

@@ -3,9 +3,9 @@ use std::{cmp, fmt, hash};
 
 use core::compiler::CompileMode;
 use core::interning::InternedString;
-use core::Shell;
+use core::{PackageId, PackageIdSpec, PackageSet, Shell};
 use util::lev_distance::lev_distance;
-use util::toml::{StringOrBool, TomlProfile, U32OrBool};
+use util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, U32OrBool};
 use util::CargoResult;
 
 /// Collection of all user profiles.
@@ -55,7 +55,7 @@ impl Profiles {
     /// workspace.
     pub fn get_profile(
         &self,
-        pkg_name: &str,
+        pkg_id: &PackageId,
         is_member: bool,
         profile_for: ProfileFor,
         mode: CompileMode,
@@ -86,7 +86,7 @@ impl Profiles {
             CompileMode::Bench => &self.bench,
             CompileMode::Doc { .. } => &self.doc,
         };
-        let mut profile = maker.profile_for(pkg_name, is_member, profile_for);
+        let mut profile = maker.profile_for(Some(pkg_id), is_member, profile_for);
         // `panic` should not be set for tests/benches, or any of their
         // dependencies.
         if profile_for == ProfileFor::TestDependency || mode.is_any_test() {
@@ -112,18 +112,14 @@ impl Profiles {
     /// select for the package that was actually built.
     pub fn base_profile(&self, release: bool) -> Profile {
         if release {
-            self.release.profile_for("", true, ProfileFor::Any)
+            self.release.profile_for(None, true, ProfileFor::Any)
         } else {
-            self.dev.profile_for("", true, ProfileFor::Any)
+            self.dev.profile_for(None, true, ProfileFor::Any)
         }
     }
 
     /// Used to check for overrides for non-existing packages.
-    pub fn validate_packages(
-        &self,
-        shell: &mut Shell,
-        packages: &HashSet<&str>,
-    ) -> CargoResult<()> {
+    pub fn validate_packages(&self, shell: &mut Shell, packages: &PackageSet) -> CargoResult<()> {
         self.dev.validate_packages(shell, packages)?;
         self.release.validate_packages(shell, packages)?;
         self.test.validate_packages(shell, packages)?;
@@ -149,7 +145,12 @@ struct ProfileMaker {
 }
 
 impl ProfileMaker {
-    fn profile_for(&self, pkg_name: &str, is_member: bool, profile_for: ProfileFor) -> Profile {
+    fn profile_for(
+        &self,
+        pkg_id: Option<&PackageId>,
+        is_member: bool,
+        profile_for: ProfileFor,
+    ) -> Profile {
         let mut profile = self.default;
         if let Some(ref toml) = self.toml {
             merge_profile(&mut profile, toml);
@@ -160,19 +161,38 @@ impl ProfileMaker {
             }
             if let Some(ref overrides) = toml.overrides {
                 if !is_member {
-                    if let Some(star) = overrides.get("*") {
-                        merge_profile(&mut profile, star);
+                    if let Some(all) = overrides.get(&ProfilePackageSpec::All) {
+                        merge_profile(&mut profile, all);
                     }
                 }
-                if let Some(byname) = overrides.get(pkg_name) {
-                    merge_profile(&mut profile, byname);
+                if let Some(pkg_id) = pkg_id {
+                    let mut matches = overrides.iter().filter_map(
+                        |(key, spec_profile)| match key {
+                            &ProfilePackageSpec::All => None,
+                            &ProfilePackageSpec::Spec(ref s) => if s.matches(pkg_id) {
+                                Some(spec_profile)
+                            } else {
+                                None
+                            },
+                        },
+                    );
+                    if let Some(spec_profile) = matches.next() {
+                        merge_profile(&mut profile, spec_profile);
+                        // `validate_packages` should ensure that there are
+                        // no additional matches.
+                        assert!(
+                            matches.next().is_none(),
+                            "package `{}` matched multiple profile overrides",
+                            pkg_id
+                        );
+                    }
                 }
             }
         }
         profile
     }
 
-    fn validate_packages(&self, shell: &mut Shell, packages: &HashSet<&str>) -> CargoResult<()> {
+    fn validate_packages(&self, shell: &mut Shell, packages: &PackageSet) -> CargoResult<()> {
         let toml = match self.toml {
             Some(ref toml) => toml,
             None => return Ok(()),
@@ -181,23 +201,88 @@ impl ProfileMaker {
             Some(ref overrides) => overrides,
             None => return Ok(()),
         };
-        for key in overrides.keys().filter(|k| k.as_str() != "*") {
-            if !packages.contains(key.as_str()) {
+        // Verify that a package doesn't match multiple spec overrides.
+        let mut found = HashSet::new();
+        for pkg_id in packages.package_ids() {
+            let matches: Vec<&PackageIdSpec> = overrides
+                .keys()
+                .filter_map(|key| match key {
+                    &ProfilePackageSpec::All => None,
+                    &ProfilePackageSpec::Spec(ref spec) => if spec.matches(pkg_id) {
+                        Some(spec)
+                    } else {
+                        None
+                    },
+                })
+                .collect();
+            match matches.len() {
+                0 => {}
+                1 => {
+                    found.insert(matches[0].clone());
+                }
+                _ => {
+                    let specs = matches
+                        .iter()
+                        .map(|spec| spec.to_string())
+                        .collect::<Vec<_>>()
+                        .join(", ");
+                    bail!(
+                        "multiple profile overrides in profile `{}` match package `{}`\n\
+                         found profile override specs: {}",
+                        self.default.name,
+                        pkg_id,
+                        specs
+                    );
+                }
+            }
+        }
+
+        // Verify every override matches at least one package.
+        let missing_specs = overrides.keys().filter_map(|key| {
+            if let &ProfilePackageSpec::Spec(ref spec) = key {
+                if !found.contains(spec) {
+                    return Some(spec);
+                }
+            }
+            None
+        });
+        for spec in missing_specs {
+            // See if there is an exact name match.
+            let name_matches: Vec<String> = packages
+                .package_ids()
+                .filter_map(|pkg_id| {
+                    if pkg_id.name().as_str() == spec.name() {
+                        Some(pkg_id.to_string())
+                    } else {
+                        None
+                    }
+                })
+                .collect();
+            if name_matches.len() == 0 {
                 let suggestion = packages
-                    .iter()
-                    .map(|p| (lev_distance(key, p), p))
+                    .package_ids()
+                    .map(|p| (lev_distance(spec.name(), &p.name()), p.name()))
                     .filter(|&(d, _)| d < 4)
                     .min_by_key(|p| p.0)
                     .map(|p| p.1);
                 match suggestion {
                     Some(p) => shell.warn(format!(
-                        "package `{}` for profile override not found\n\nDid you mean `{}`?",
-                        key, p
+                        "profile override spec `{}` did not match any packages\n\n\
+                         Did you mean `{}`?",
+                        spec, p
                     ))?,
-                    None => {
-                        shell.warn(format!("package `{}` for profile override not found", key))?
-                    }
-                };
+                    None => shell.warn(format!(
+                        "profile override spec `{}` did not match any packages",
+                        spec
+                    ))?,
+                }
+            } else {
+                shell.warn(format!(
+                    "version or URL in profile override spec `{}` does not \
+                     match any of the packages: {}",
+                    spec,
+                    name_matches.join(", ")
+                ))?;
             }
         }
         Ok(())

src/cargo/ops/cargo_clean.rs

@@ -60,15 +60,15 @@ pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> {
                     for profile_for in ProfileFor::all_values() {
                         let profile = if mode.is_run_custom_build() {
                             profiles.get_profile_run_custom_build(&profiles.get_profile(
-                                &pkg.name(),
+                                pkg.package_id(),
                                 ws.is_member(pkg),
                                 *profile_for,
                                 CompileMode::Build,
                                 opts.release,
                             ))
                         } else {
                             profiles.get_profile(
-                                &pkg.name(),
+                                pkg.package_id(),
                                 ws.is_member(pkg),
                                 *profile_for,
                                 *mode,

src/cargo/ops/cargo_compile.rs

@@ -254,11 +254,7 @@ pub fn compile_ws<'a>(
     }
 
     let profiles = ws.profiles();
-    let package_names = packages
-        .package_ids()
-        .map(|pid| pid.name().as_str())
-        .collect::<HashSet<_>>();
-    profiles.validate_packages(&mut config.shell(), &package_names)?;
+    profiles.validate_packages(&mut config.shell(), &packages)?;
 
     let mut extra_compiler_args = None;
 
@@ -494,7 +490,7 @@ fn generate_targets<'a>(
             default_arch_kind
         };
         let profile = profiles.get_profile(
-            &pkg.name(),
+            pkg.package_id(),
             ws.is_member(pkg),
             profile_for,
             target_mode,

src/cargo/util/toml/mod.rs

@@ -380,10 +380,44 @@ pub struct TomlProfile {
     pub panic: Option<String>,
     pub overflow_checks: Option<bool>,
     pub incremental: Option<bool>,
-    pub overrides: Option<BTreeMap<String, TomlProfile>>,
+    pub overrides: Option<BTreeMap<ProfilePackageSpec, TomlProfile>>,
     pub build_override: Option<Box<TomlProfile>>,
 }
 
+#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
+pub enum ProfilePackageSpec {
+    Spec(PackageIdSpec),
+    All,
+}
+
+impl ser::Serialize for ProfilePackageSpec {
+    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
+    where
+        S: ser::Serializer,
+    {
+        match *self {
+            ProfilePackageSpec::Spec(ref spec) => spec.serialize(s),
+            ProfilePackageSpec::All => "*".serialize(s),
+        }
+    }
+}
+
+impl<'de> de::Deserialize<'de> for ProfilePackageSpec {
+    fn deserialize<D>(d: D) -> Result<ProfilePackageSpec, D::Error>
+    where
+        D: de::Deserializer<'de>,
+    {
+        let string = String::deserialize(d)?;
+        if string == "*" {
+            Ok(ProfilePackageSpec::All)
+        } else {
+            PackageIdSpec::parse(&string)
+                .map_err(de::Error::custom)
+                .map(|s| ProfilePackageSpec::Spec(s))
+        }
+    }
+}
+
 impl TomlProfile {
     fn validate(
         &self,