Skip to content

Commit

Permalink
fix: proper metadata parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
@somehowchris
somehowchris committed Apr 16, 2022
1 parent e22e92b commit b564759
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo-all-features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub fn main() {

// checking if cross is installed
if command_target == CommandTarget::Cross && which::which("cross").is_err() {
println!("{}: Could not find `cross` installed. To install it run `cargo install cross` or header over to https://github.com/cross-rs/cross for more information", Paint::red("error").bold());
println!("{}: Could not find `cross` installed. To install it run `cargo install cross` or head over to https://github.com/cross-rs/cross for more information", Paint::red("error").bold());
process::exit(127);
}

Expand Down
2 changes: 1 addition & 1 deletion src/metadata/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use serde::Deserialize;
use std::collections::HashSet;
use validator::Validate;

#[derive(Clone, Deserialize, Validate)]
#[derive(Clone, Deserialize, Validate, Debug)]
pub struct CargoAllFeatures {
pub skip_feature_sets: Option<Vec<FeatureList>>,
pub skip_optional_dependencies: Option<bool>,
Expand Down
2 changes: 1 addition & 1 deletion src/metadata/dependency.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use serde::Deserialize;

#[derive(Clone, Deserialize)]
#[derive(Clone, Deserialize, Debug)]
pub struct Dependency {
pub name: String,
pub rename: Option<String>,
Expand Down
135 changes: 85 additions & 50 deletions src/metadata/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,59 @@ use std::path;
use validator::Validate;
use validator::ValidationError;

#[derive(Clone, Deserialize, Validate)]
#[derive(Clone, Deserialize, Validate, Debug)]
#[validate(schema(function = "Self::validate"))]
pub struct Package {
pub id: String,
pub name: String,
pub manifest_path: path::PathBuf,
pub dependencies: Vec<Dependency>,
pub features: HashMap<String, FeatureList>,
pub metadata: Option<PackageMetaData>,
}

#[derive(Clone, Deserialize, Validate, Debug)]
pub struct PackageMetaData {
#[serde(rename = "cargo-all-features")]
pub cargo_all_features: Option<CargoAllFeatures>,
}

impl Package {
// Validation used by validator
pub fn validate(&self) -> Result<(), ValidationError> {
if let Some(config) = &self.cargo_all_features {
if let Some(allow_ist) = &config.allowlist {
if !allow_ist.is_empty() {
if let Some(deny_list) = &config.denylist {
if !deny_list.is_empty() {
let mut error = ValidationError::new(
"Package has both `allowlist` and `denylist` keys",
);
error.add_param("name".into(), &self.name);
if let Some(metadata) = &self.metadata {
if let Some(config) = metadata.cargo_all_features.as_ref() {
if let Some(allow_ist) = &config.allowlist {
if !allow_ist.is_empty() {
if let Some(deny_list) = &config.denylist {
if !deny_list.is_empty() {
let mut error = ValidationError::new(
"Package has both `allowlist` and `denylist` keys",
);
error.add_param("name".into(), &self.name);

return Err(error);
return Err(error);
}
}
}
if let Some(extra_features) = &config.extra_features {
if !extra_features.is_empty() {
if let Some(extra_features) = &config.extra_features {
if !extra_features.is_empty() {
let mut error = ValidationError::new(
"Package has both `allowlist` and `extra_features` keys",
);
error.add_param("name".into(), &self.name);

return Err(error);
}
}
if config.skip_optional_dependencies.is_some() {
let mut error = ValidationError::new(
"Package has both `allowlist` and `extra_features` keys",
"Package has both `allowlist` and `skip_optional_dependencies` keys",
);
error.add_param("name".into(), &self.name);

return Err(error);
}
}
if config.skip_optional_dependencies.is_some() {
let mut error = ValidationError::new(
"Package has both `allowlist` and `skip_optional_dependencies` keys",
);
error.add_param("name".into(), &self.name);

return Err(error);
}
}
}
}
Expand Down Expand Up @@ -94,9 +101,14 @@ impl Package {

// Closure to check if is in deny list
let filter_denylist = |f: &String| {
self.cargo_all_features
self.metadata
.as_ref()
.map(|mf| !mf.denylist.as_ref().map(|e| e.contains(f)).unwrap_or(false))
.map(|meta| {
meta.cargo_all_features
.as_ref()
.map(|mf| !mf.denylist.as_ref().map(|e| e.contains(f)).unwrap_or(false))
.unwrap_or(true)
})
.unwrap_or(true)
};

Expand Down Expand Up @@ -127,22 +139,32 @@ impl Package {
// Clippy screams due to the `.map` and `.unwrap_or` as they seem complex
#[allow(clippy::blocks_in_if_conditions)]
if self
.cargo_all_features
.metadata
.as_ref()
.map(|config| {
config
.allowlist
.map(|meta| {
meta.cargo_all_features
.as_ref()
.map(|e| e.is_empty())
.map(|config| {
config
.allowlist
.as_ref()
.map(|e| e.is_empty())
.unwrap_or(true)
})
.unwrap_or(true)
})
.unwrap_or(true)
{
// This handles the pre-1.60 case
if !self
.cargo_all_features
.metadata
.as_ref()
.map(|config| config.skip_optional_dependencies.unwrap_or(false))
.map(|meta| {
meta.cargo_all_features
.as_ref()
.map(|config| config.skip_optional_dependencies.unwrap_or(false))
.unwrap_or(false)
})
.unwrap_or(false)
&& !found_dep
{
Expand All @@ -154,25 +176,36 @@ impl Package {
// This handles the post-1.60 case
.filter(|f| {
!self
.cargo_all_features
.metadata
.as_ref()
.map(|config| config.skip_optional_dependencies.unwrap_or(false))
.map(|meta| {
meta.cargo_all_features
.as_ref()
.map(|config| {
config.skip_optional_dependencies.unwrap_or(false)
})
.unwrap_or(false)
})
.unwrap_or(false)
|| !optional_denylist.contains(f)
}),
);

if let Some(config) = self.cargo_all_features.as_ref() {
if let Some(extra_features) = &config.extra_features {
features.par_extend(extra_features.par_iter().filter(|e| filter_denylist(e)));
if let Some(metadata) = self.metadata.as_ref() {
if let Some(config) = &metadata.cargo_all_features {
if let Some(extra_features) = &config.extra_features {
features
.par_extend(extra_features.par_iter().filter(|e| filter_denylist(e)));
}
}
}
} else {
// allowlist cannot be mixed with denylist or any of the other above options,
// no need to filter
if let Some(config) = self.cargo_all_features.as_ref() {
if let Some(allow_list) = &config.allowlist {
features.par_extend(allow_list.par_iter())
if let Some(metadata) = self.metadata.as_ref() {
if let Some(config) = &metadata.cargo_all_features {
if let Some(allow_list) = &config.allowlist {
features.par_extend(allow_list.par_iter())
}
}
}
};
Expand All @@ -181,17 +214,19 @@ impl Package {

for n in 0..=features.len() {
'outer: for feature_set in features.iter().combinations(n) {
if let Some(config) = self.cargo_all_features.as_ref() {
if let Some(skip_feature_sets) = &config.skip_feature_sets {
'inner: for skip_feature_set in skip_feature_sets {
for feature in skip_feature_set.iter() {
if !feature_set.contains(&&feature) {
// skip_feature_set does not match
continue 'inner;
if let Some(metadata) = self.metadata.as_ref() {
if let Some(config) = metadata.cargo_all_features.as_ref() {
if let Some(skip_feature_sets) = &config.skip_feature_sets {
'inner: for skip_feature_set in skip_feature_sets {
for feature in skip_feature_set.iter() {
if !feature_set.contains(&&feature) {
// skip_feature_set does not match
continue 'inner;
}
}
// skip_feature_set matches: do not add it to feature_sets
continue 'outer;
}
// skip_feature_set matches: do not add it to feature_sets
continue 'outer;
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/toolchain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ impl RustUpToolchain {
Some(value) => {
// Split channel or version from triplet
let parts: Vec<&str> = value.par_split('-').collect();
println!("{:?}", parts);
Ok(Self {
channel: parts[0].to_owned(),
triplet: parts[1..].join("-"),
Expand Down

0 comments on commit b564759

Please sign in to comment.