Allow raze settings to be expressed with SemVer

Raze settings come in the form of a [TOML] section like so ```toml [raze.crates.some-crate.'1.2.3'] some_new_setting = True ``` Where the version number (in the above example `1.2.3`) is a literal, hardcoded value. This works but is a little inflexible for dependencies, _especially_ dependencies that are transiant and update a lot (for instance `syn`). Since Cargo follows [Semver] we can use this fact, along with the richness of the exported and serialisable `semver::Version` types to perform section matching with [Semver] in the same fashion as Cargo itself. This means that the above example can be written with semver expressions, for example it can be written as: ```toml [raze.crates.some-crate.'1.2.*'] some_new_setting = True [raze.crates.some-crate.'~1.2.3'] some_new_setting = True [raze.crates.some-crate.'^1.2.3'] some_new_setting = True [raze.crates.some-crate.'=1.6.6'] some_new_setting = True ``` _Note_: Bare versions follow the semantics _as if they had_ specified a `^`, which is in keeping with [Cargo semver semantics] but is distinct from previous behaviour of raze (in which these would be interpreted as exacting or `=`). This is deliberate as we should aim to mirror the semantics of cargo as much as possible to avoid confusion. We presently do not allow for multiple matches in raze settings, this is intentional and is presented to the end user as an error. [TOML]: https://github.com/toml-lang/toml [Semver]: https://semver.org/spec/v2.0.0.html [Cargo semver semantics]: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio
google · Jul 20, 2020 · cc96241 · cc96241
1 parent f77ba88
commit cc96241
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 62 deletions.
diff --git a/impl/Cargo.toml b/impl/Cargo.toml
@@ -30,7 +30,7 @@ cargo_metadata = "0.9.1"
 itertools = "0.8.0"
 slug = "0.1.4"
 tera = "1.0.0"
-semver = "0.9.0"
+semver = { version = "0.9.0", features = ["serde"] }
 serde_derive = "1.0.84"
 serde = "1.0.84"
 toml = "0.4.10"

diff --git a/impl/src/bazel.rs b/impl/src/bazel.rs
@@ -13,7 +13,6 @@
 // limitations under the License.
 
 use anyhow::Result;
-
 use tera::{self, Context, Tera};
 
 use crate::{
@@ -291,6 +290,7 @@ mod tests {
     rendering::{FileOutputs, RenderDetails},
     settings::CrateSettings,
   };
+  use semver::Version;
 
   use super::*;
 
@@ -316,7 +316,7 @@ mod tests {
   fn dummy_binary_crate_with_name(buildfile_suffix: &str) -> CrateContext {
     CrateContext {
       pkg_name: "test-binary".to_owned(),
-      pkg_version: "1.1.1".to_owned(),
+      pkg_version: Version::parse("1.1.1").unwrap(),
       edition: "2015".to_owned(),
       features: vec!["feature1".to_owned(), "feature2".to_owned()].to_owned(),
       expected_build_path: format!("vendor/test-binary-1.1.1/{}", buildfile_suffix),
@@ -350,7 +350,7 @@ mod tests {
   fn dummy_library_crate_with_name(buildfile_suffix: &str) -> CrateContext {
     CrateContext {
       pkg_name: "test-library".to_owned(),
-      pkg_version: "1.1.1".to_owned(),
+      pkg_version: Version::parse("1.1.1").unwrap(),
       edition: "2015".to_owned(),
       license: LicenseData::default(),
       raze_settings: CrateSettings::default(),

diff --git a/impl/src/context.rs b/impl/src/context.rs
@@ -14,11 +14,12 @@
 
 use crate::settings::CrateSettings;
 use serde_derive::Serialize;
+use semver::Version;
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)]
 pub struct BuildableDependency {
   pub name: String,
-  pub version: String,
+  pub version: Version,
   pub buildable_target: String,
   pub is_proc_macro: bool,
 }
@@ -40,7 +41,7 @@ pub struct BuildableTarget {
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)]
 pub struct Metadep {
   pub name: String,
-  pub min_version: String,
+  pub min_version: Version,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)]
@@ -72,7 +73,7 @@ pub struct SourceDetails {
 #[derive(Debug, Clone, Serialize)]
 pub struct CrateContext {
   pub pkg_name: String,
-  pub pkg_version: String,
+  pub pkg_version: Version,
   pub edition: String,
   pub raze_settings: CrateSettings,
   pub license: LicenseData,

diff --git a/impl/src/planning.rs b/impl/src/planning.rs
@@ -14,6 +14,7 @@
 
 use std::{
   collections::{HashMap, HashSet},
+  iter,
   path::PathBuf,
   str::{self, FromStr},
 };
@@ -131,7 +132,7 @@ struct CrateSubplanner<'planner> {
   crate_catalog_entry: &'planner CrateCatalogEntry,
   source_id: &'planner Option<SourceId>,
   node: &'planner Node,
-  crate_settings: &'planner CrateSettings,
+  crate_settings: Option<&'planner CrateSettings>,
   sha256: &'planner Option<String>,
 }
 
@@ -150,7 +151,7 @@ impl CrateCatalogEntry {
     is_workspace_crate: bool,
   ) -> Self {
     let sanitized_name = package.name.replace("-", "_");
-    let sanitized_version = util::sanitize_ident(&package.version.clone().to_string());
+    let sanitized_version = util::sanitize_ident(&package.version.to_string());
 
     Self {
       package: package.clone(),
@@ -440,14 +441,6 @@ impl<'planner> WorkspaceSubplanner<'planner> {
           return None;
         }
 
-        let crate_settings = self
-          .settings
-          .crates
-          .get(&own_package.name)
-          .and_then(|c| c.get(&own_package.version))
-          .cloned()
-          .unwrap_or_else(CrateSettings::default);
-
         // UNWRAP: Safe given unwrap during serialize step of metadata
         let own_source_id = own_package
           .source
@@ -461,14 +454,41 @@ impl<'planner> WorkspaceSubplanner<'planner> {
           crate_catalog_entry: &own_crate_catalog_entry,
           source_id: &own_source_id,
           node: &node,
-          crate_settings: &crate_settings,
+          crate_settings: self.crate_settings(&own_package).unwrap(),
           sha256: &checksum_opt.map(|c| c.to_owned()),
         };
 
         Some(crate_subplanner.produce_context())
       })
       .collect()
   }
+
+  fn crate_settings(&self, package: &Package) -> Result<Option<&CrateSettings>> {
+    self
+      .settings
+      .crates
+      .get(&package.name)
+      .map_or(Ok(None), |settings| {
+        let mut versions = settings
+          .iter()
+          .filter(|(ver_req, _)| ver_req.matches(&package.version))
+          .peekable();
+
+        match versions.next() {
+          None => Ok(None),
+          Some((_, settings)) if versions.peek().is_none() => Ok(Some(settings)),
+          Some(current) => Err(RazeError::Config {
+            field_path_opt: None,
+            message: format!(
+              "Multiple potential semver matches `[{}]` found for `{}`",
+              iter::once(current).chain(versions).map(|x| x.0).join(", "),
+              &package.name
+            )
+          })
+        }
+      })
+      .map_err(|e| e.into())
+  }
 }
 
 impl<'planner> CrateSubplanner<'planner> {
@@ -499,7 +519,7 @@ impl<'planner> CrateSubplanner<'planner> {
 
     Ok(CrateContext {
       pkg_name: package.name.clone(),
-      pkg_version: package.version.to_string(),
+      pkg_version: package.version.clone(),
       edition: package.edition.clone(),
       license: self.produce_license(),
       features: self.node.features.clone(),
@@ -512,7 +532,7 @@ impl<'planner> CrateSubplanner<'planner> {
       aliased_dependencies: aliased_deps,
       workspace_path_to_crate: self.crate_catalog_entry.workspace_path(&self.settings),
       build_script_target: build_script_target_opt,
-      raze_settings: self.crate_settings.clone(),
+      raze_settings: self.crate_settings.cloned().unwrap_or_default(),
       source_details: self.produce_source_details(),
       expected_build_path: self.crate_catalog_entry.local_build_path(&self.settings),
       sha256: self.sha256.clone(),
@@ -551,9 +571,8 @@ impl<'planner> CrateSubplanner<'planner> {
 
     let all_skipped_deps = self
       .crate_settings
-      .skipped_deps
       .iter()
-      .cloned()
+      .flat_map(|pkg| pkg.skipped_deps.iter())
       .collect::<HashSet<_>>();
 
     for dep_id in &self.node.dependencies {
@@ -592,7 +611,7 @@ impl<'planner> CrateSubplanner<'planner> {
 
       let buildable_dependency = BuildableDependency {
         name: dep_package.name.clone(),
-        version: dep_package.version.to_string(),
+        version: dep_package.version.clone(),
         buildable_target: buildable_target.clone(),
         is_proc_macro,
       };
@@ -728,7 +747,7 @@ impl<'planner> CrateSubplanner<'planner> {
   ) -> Option<BuildableTarget> {
     if !self
       .crate_settings
-      .gen_buildrs
+      .and_then(|x| x.gen_buildrs)
       .unwrap_or(self.settings.default_gen_buildrs)
     {
       return None;
@@ -833,6 +852,7 @@ impl<'planner> CrateSubplanner<'planner> {
 mod checks {
   use std::{
     collections::{HashMap, HashSet},
+    iter::FromIterator,
     env, fs,
   };
 
@@ -924,42 +944,21 @@ mod checks {
     all_crate_settings: &HashMap<String, CrateSettingsPerVersion>,
     all_packages: &[Package],
   ) {
-    let mut known_versions_per_crate = HashMap::new();
-    for &Package {
-      ref name,
-      ref version,
-      ..
-    } in all_packages
-    {
-      known_versions_per_crate
-        .entry(name.clone())
-        .or_insert_with(HashSet::new)
-        .insert(version.clone());
-    }
-
-    for (name, settings_per_version) in all_crate_settings {
-      if !known_versions_per_crate.contains_key(name) {
-        eprintln!(
-          "Found unused raze settings for all of {}-{:?}",
-          name,
-          settings_per_version.keys()
-        );
-        // No version introspection needed -- no known version of this crate
-        continue;
-      }
-
-      // UNWRAP: Guarded above
-      let all_known_versions = known_versions_per_crate.get(name).unwrap();
-
-      for version in settings_per_version.keys() {
-        if !all_known_versions.contains(version) {
-          eprintln!(
-            "Found unused raze settings for {}-{}, but {:?} were known",
-            name, version, all_known_versions
-          )
-        }
-      }
-    }
+     // 1st check names
+     let pkg_names = all_packages.iter().map(|pkg| &pkg.name).collect::<HashSet<_>>();
+     let setting_names = HashSet::from_iter(all_crate_settings.keys());
+     for missing in setting_names.difference(&pkg_names) {
+       eprintln!("Found unused raze crate settings for `{}`", missing);
+     }
+
+     // Then check versions
+     all_crate_settings.iter()
+       .flat_map(|(name, settings)| settings.iter().map(move |x| (x.0, name)))
+       .filter(|(ver_req, _)| !all_packages.iter().any(|pkg| ver_req.matches(&pkg.version)))
+       .for_each(|(ver_req, name)| {
+         eprintln!("Found unused raze settings for version `{}` against crate `{}`",
+                   ver_req, name);
+       });
   }
 }
 

diff --git a/impl/src/settings.rs b/impl/src/settings.rs
@@ -12,11 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use semver::Version;
+use semver::VersionReq;
 use serde_derive::{Deserialize, Serialize};
 use std::collections::HashMap;
 
-pub type CrateSettingsPerVersion = HashMap<Version, CrateSettings>;
+pub type CrateSettingsPerVersion = HashMap<VersionReq, CrateSettings>;
 
 /**
  * A "deserializable struct" for the whole Cargo.toml