Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package spec support to profile overrides. #5491

Merged
merged 1 commit into from
May 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 21 additions & 1 deletion src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::fmt;

use semver::Version;
use serde::{de, ser};
use url::Url;

use core::PackageId;
Expand All @@ -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>,
Expand Down Expand Up @@ -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};
Expand Down
139 changes: 112 additions & 27 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand All @@ -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)?;
Expand All @@ -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);
Expand All @@ -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(()),
Expand All @@ -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(())
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down
36 changes: 35 additions & 1 deletion src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading