Repository: EmbarkStudios/cargo-about (language: rust)

Issue:
Use licensee priority list
I added

```rust
#[serde(deserialize_with = "deserialize_licensee")]
pub accepted: Vec<spdx::Licensee>,
```

to Config, now the sanitize just needs to take that into account and only use the first license that causes its expression to evaluate to true, this can be just a loop for now around `evaluate()`.


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index a8f5c46..ecb195e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -125,12 +125,13 @@ dependencies = [
  "chrono 0.4.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "fern 0.5.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "handlebars 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "license 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "rayon 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde 1.0.102 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "spdx 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "spdx 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "structopt 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "toml 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "walkdir 2.2.9 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -359,6 +360,11 @@ name = "libc"
 version = "0.2.65"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "license"
+version = "0.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "log"
 version = "0.4.8"
@@ -657,7 +663,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "spdx"
-version = "0.2.3"
+version = "0.2.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -894,6 +900,7 @@ dependencies = [
 "checksum jobserver 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)" = "f2b1d42ef453b30b7387e113da1c83ab1605d90c5b4e0eb8e96d016ed3b8c160"
 "checksum lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 "checksum libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)" = "1a31a0627fdf1f6a39ec0dd577e101440b7db22672c0901fe00a9a6fbb5c24e8"
+"checksum license 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "913c0d16df4e4fd69313baa771ca6998c89338932a057346a3a057ef2a744980"
 "checksum log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)" = "14b6052be84e6b71ab17edffc2eeabf5c2c3ae1fdb464aae35ac50c67a44e1f7"
 "checksum maplit 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d"
 "checksum maybe-uninit 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00"
@@ -932,7 +939,7 @@ dependencies = [
 "checksum sha-1 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "23962131a91661d643c98940b20fcaffe62d776a823247be80a48fcb8b6fce68"
 "checksum smallvec 0.6.13 (registry+https://github.com/rust-lang/crates.io-index)" = "f7b0758c52e15a8b5e3691eae6cc559f08eee9406e548a4477ba4e67770a82b6"
 "checksum smallvec 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4ecf3b85f68e8abaa7555aa5abdb1153079387e60b718283d732f03897fcfc86"
-"checksum spdx 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "f4348fc0f87adf1ccbfc0bb009afb4620a46a2f8d1bdac2d9314a63169816d57"
+"checksum spdx 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "842b7e1cc2b1d258990b0a225757ceaa6f44e66c49ae2ef175365093818f14ac"
 "checksum strsim 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8ea5119cdb4c55b55d432abb513a0429384878c15dde60cc77b1c99de1a95a6a"
 "checksum structopt 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6d4f66a4c0ddf7aee4677995697366de0749b0139057342eccbb609b12d0affc"
 "checksum structopt-derive 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8fe0c13e476b4e21ff7f5c4ace3818b6d7bdc16897c31c73862471bc1663acae"
diff --git a/Cargo.toml b/Cargo.toml
index f135378..1aed39f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,7 +32,8 @@ rayon = "1.2.0"
 serde = { version = "1.0.102", features = ["derive"] }
 serde_json = "1.0.41"
 smallvec = "1.0.0"
-spdx = "0.2.3"
+spdx = "0.2.4"
 structopt = "0.3.3"
 toml = "0.5.5"
 walkdir = "2.2.9"
+license = "0.10.1"
diff --git a/src/cargo-about/generate.rs b/src/cargo-about/generate.rs
index 4bd85aa..1231c37 100644
--- a/src/cargo-about/generate.rs
+++ b/src/cargo-about/generate.rs
@@ -109,27 +109,26 @@ pub fn cmd(
         }
     };
 
-    let mut summary = licenses::Gatherer::with_store(std::sync::Arc::new(store))
+    let summary = licenses::Gatherer::with_store(std::sync::Arc::new(store))
         .with_confidence_threshold(args.threshold)
         .gather(&krates.krates, &cfg);
 
-    licenses::sanitize(&mut summary)?;
-
-    let output = generate(&summary, &registry, &template)?;
+    let resolved = licenses::Resolved::resolve(&summary.nfos, &cfg.accepted)?;
+    let output = generate(&summary.nfos, &resolved, &registry, &template)?;
 
     println!("{}", output);
 
     Ok(())
 }
 
-#[derive(Serialize)]
+#[derive(Clone, Serialize)]
 struct UsedBy<'a> {
     #[serde(rename = "crate")]
     krate: &'a Krate,
     path: Option<PathBuf>,
 }
 
-#[derive(Serialize)]
+#[derive(Clone, Serialize)]
 struct License<'a> {
     /// The full name of the license
     name: String,
@@ -138,7 +137,7 @@ struct License<'a> {
     /// The full license text
     text: String,
     /// The path where the license text was sourced from
-    source_path: PathBuf,
+    source_path: Option<PathBuf>,
     /// The list of crates this license was applied to
     used_by: Vec<UsedBy<'a>>,
 }
@@ -158,66 +157,76 @@ struct Input<'a> {
 }
 
 fn generate(
-    summary: &licenses::Summary<'_>,
+    nfos: &[licenses::KrateLicense<'_>],
+    resolved: &licenses::Resolved,
     hbs: &Handlebars,
     template_name: &str,
 ) -> Result<String, Error> {
     let licenses = {
         let mut licenses = HashMap::new();
-
-        for k in &summary.nfos {
-            if let licenses::LicenseInfo::Expr(ref expr) = k.lic_info {
-                for req in expr.requirements().filter_map(|r| {
-                    if let spdx::LicenseItem::SPDX { id, .. } = r.req.license {
-                        Some(id)
-                    } else {
-                        None
-                    }
-                }) {
-                    if let Some(nfo) = k.license_files.iter().find(|lf| {
-                        if lf.id != req {
-                            return false;
-                        }
-
-                        match lf.info {
-                            licenses::LicenseFileInfo::Text(_)
-                            | licenses::LicenseFileInfo::AddendumText(_, _) => true,
-                            _ => false,
-                        }
-                    }) {
-                        let entry = licenses.entry(req.name).or_insert_with(HashMap::new);
-
-                        let contents = match nfo.info {
-                            licenses::LicenseFileInfo::Text(ref s) => s,
-                            licenses::LicenseFileInfo::AddendumText(ref s, _) => s,
-                            _ => unreachable!(),
-                        };
-
-                        let lic = entry.entry(contents).or_insert_with(|| License {
-                            name: req.full_name.to_owned(),
-                            id: req.name.to_owned(),
-                            text: contents.clone(),
-                            used_by: Vec::new(),
-                            source_path: nfo.path.clone(),
-                        });
-
-                        match nfo.info {
-                            licenses::LicenseFileInfo::Text(_) => {
-                                lic.used_by.push(UsedBy {
-                                    krate: k.krate,
-                                    path: None,
-                                });
+        for (krate_id, license_list) in &resolved.0 {
+            let krate_license = &nfos[*krate_id];
+            let license_iter = license_list
+                .iter()
+                .filter_map(|license| match license.license {
+                    spdx::LicenseItem::SPDX { id, .. } => {
+                        let file = krate_license.license_files.iter().find_map(move |lf| {
+                            if lf.id != id {
+                                return None;
+                            }
+                            match &lf.info {
+                                licenses::LicenseFileInfo::Text(text)
+                                | licenses::LicenseFileInfo::AddendumText(text, _) => {
+                                    let license = License {
+                                        name: lf.id.full_name.to_owned(),
+                                        id: lf.id.name.to_owned(),
+                                        text: text.clone(),
+                                        source_path: None,
+                                        used_by: Vec::new(),
+                                    };
+                                    Some(license)
+                                }
+                                _ => None,
                             }
-                            licenses::LicenseFileInfo::AddendumText(_, ref p) => {
-                                lic.used_by.push(UsedBy {
-                                    krate: k.krate,
-                                    path: Some(p.clone()),
-                                })
+                        });
+                        file.or_else(|| {
+                            let license = license::from_id(id.name).map(|lic| License {
+                                name: lic.name().to_string(),
+                                id: lic.id().to_string(),
+                                text: lic.text().to_string(),
+                                source_path: None,
+                                used_by: Vec::new(),
+                            });
+                            if license.is_none() {
+                                log::warn!(
+                                    "No licene file or license text found for {} in crate {}",
+                                    id.name,
+                                    krate_license.krate.name
+                                );
                             }
-                            _ => unreachable!(),
-                        }
+                            license
+                        })
+                    }
+                    _ => {
+                        log::warn!(
+                            "{} has no license file for crate '{}'",
+                            license,
+                            krate_license.krate.name
+                        );
+                        None
                     }
-                }
+                });
+
+            for license in license_iter {
+                let entry = licenses
+                    .entry(license.name.clone())
+                    .or_insert_with(HashMap::new);
+
+                let lic = entry.entry(license.text.clone()).or_insert_with(|| license);
+                lic.used_by.push(UsedBy {
+                    krate: krate_license.krate,
+                    path: None,
+                });
             }
         }
 
@@ -250,6 +259,8 @@ fn generate(
     }
 
     overview.iter_mut().for_each(|i| i.count = i.indices.len());
+    // Show the most used licenses first
+    overview.sort_by(|a, b| b.count.cmp(&a.count));
 
     let nput = Input { licenses, overview };
 
diff --git a/src/licenses/config.rs b/src/licenses/config.rs
index c355b82..0909f69 100644
--- a/src/licenses/config.rs
+++ b/src/licenses/config.rs
@@ -31,6 +31,43 @@ where
     deserializer.deserialize_any(Visitor)
 }
 
+fn deserialize_licensee<'de, D>(
+    deserializer: D,
+) -> std::result::Result<Vec<spdx::Licensee>, D::Error>
+where
+    D: de::Deserializer<'de>,
+{
+    struct Visitor;
+
+    impl<'de> de::Visitor<'de> for Visitor {
+        type Value = Vec<spdx::Licensee>;
+
+        fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+            formatter.write_str("array of SPDX licensees")
+        }
+
+        fn visit_seq<S>(self, mut seq: S) -> Result<Self::Value, S::Error>
+        where
+            S: de::SeqAccess<'de>,
+        {
+            let mut vec = Vec::new();
+
+            // Update the max while there are additional values.
+            while let Some(v) = seq.next_element()? {
+                let lic = spdx::Licensee::parse(v).map_err(|e| {
+                    de::Error::custom(format!("'{}' is not a valid SPDX licensee: {}", v, e))
+                })?;
+
+                vec.push(lic);
+            }
+
+            Ok(vec)
+        }
+    }
+
+    deserializer.deserialize_seq(Visitor)
+}
+
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "kebab-case")]
 pub struct Additional {
@@ -62,6 +99,9 @@ pub struct KrateConfig {
 
 #[derive(Deserialize, Debug, Default)]
 pub struct Config {
+    /// The list of licenses we will use for all crates, in priority order
+    #[serde(deserialize_with = "deserialize_licensee")]
+    pub accepted: Vec<spdx::Licensee>,
     #[serde(flatten)]
     pub inner: BTreeMap<String, KrateConfig>,
 }
diff --git a/src/licenses/mod.rs b/src/licenses/mod.rs
index fbb561c..f4e7d83 100644
--- a/src/licenses/mod.rs
+++ b/src/licenses/mod.rs
@@ -1,7 +1,8 @@
 use crate::Krate;
-use anyhow::{bail, Error};
+use anyhow::{bail, Context, Error};
 use rayon::prelude::*;
-use spdx::LicenseId;
+use spdx::{LicenseId, LicenseItem, LicenseReq, Licensee};
+use std::fmt;
 use std::{
     path::{Path, PathBuf},
     sync::Arc,
@@ -162,7 +163,7 @@ impl Gatherer {
                 let root_path = krate.manifest_path.parent().unwrap();
                 let krate_cfg = cfg.inner.get(&krate.name);
 
-                let license_files = match scan_files(
+                let mut license_files = match scan_files(
                     &root_path,
                     &strategy,
                     threshold,
@@ -181,6 +182,38 @@ impl Gatherer {
                     }
                 };
 
+                // Condense each license down to the best candidate if
+                // multiple are found
+
+                license_files.sort_by(|a, b| {
+                    use std::cmp::Ordering as Ord;
+                    match a.id.cmp(&b.id) {
+                        Ord::Equal => {
+                            // We want the highest confidence on top
+                            b.confidence
+                                .partial_cmp(&a.confidence)
+                                .expect("uhoh looks like we've got a NaN")
+                        }
+                        o => o,
+                    }
+                });
+
+                let mut id = None;
+                license_files.retain(|lf| match id {
+                    Some(cur) => {
+                        if cur != lf.id {
+                            id = Some(lf.id);
+                            true
+                        } else {
+                            false
+                        }
+                    }
+                    None => {
+                        id = Some(lf.id);
+                        true
+                    }
+                });
+
                 KrateLicense {
                     krate,
                     lic_info: info,
@@ -482,101 +515,99 @@ fn scan_text(contents: &str, strat: &askalono::ScanStrategy, threshold: f32) ->
     }
 }
 
-pub fn sanitize(summary: &mut Summary) -> Result<(), Error> {
-    let num_errors = summary
-        .nfos
-        .par_iter_mut()
-        .fold(
-            || 0,
-            |acc, krate_license| {
-                // Check that the licenses found by scanning the crate contents match what was stated
-                // in the license expression
-                match krate_license.lic_info {
-                    LicenseInfo::Expr(ref expr) => {
-                        let spdx_reqs = expr
-                            .requirements()
-                            .filter_map(|req| {
-                                if let spdx::LicenseItem::SPDX { id, .. } = req.req.license {
-                                    Some(id)
-                                } else {
-                                    None
-                                }
-                            })
-                            .collect::<smallvec::SmallVec<[LicenseId; 2]>>();
-
-                        log::info!(
-                            "crate {}({}) has license(s) {:?} in its `license` field",
-                            krate_license.krate.name,
-                            krate_license.krate.version,
-                            spdx_reqs
-                        );
+type KrateId = usize;
 
-                        for lf in &krate_license.license_files {
-                            if !lf.info.is_addendum() && !spdx_reqs.contains(&lf.id) {
-                                log::warn!(
-                                    "mismatching license found for {}: license '{}' in path '{}'",
-                                    krate_license.krate.name,
-                                    lf.id.name,
-                                    lf.path.display()
-                                );
-                            }
-                        }
-
-                        acc
-                    }
-                    LicenseInfo::Unknown => {
-                        let mut found = smallvec::SmallVec::<[(LicenseId, u32); 2]>::new();
-
-                        for lf in &krate_license.license_files {
-                            match found.iter_mut().find(|lic| lic.0 == lf.id) {
-                                Some(lic) => lic.1 += 1,
-                                None => found.push((lf.id, 1)),
-                            }
-                        }
-
-                        let expr_s = {
-                            let mut expr_s = String::new();
-
-                            for (i, name) in found.iter().map(|l| l.0.name).enumerate() {
-                                if i > 0 {
-                                    expr_s.push_str(" AND ");
-                                }
+pub struct ResolveError<'a> {
+    pub krate: &'a Krate,
+    pub required: Vec<LicenseReq>,
+}
 
-                                expr_s.push_str(name);
-                            }
+impl fmt::Display for ResolveError<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Krate '{}' requires", self.krate.name)?;
+        f.debug_list().entries(self.required.iter()).finish()?;
+        writeln!(
+            f,
+            " , which were not specified as 'accepted' licenses in the 'about.toml' file"
+        )
+    }
+}
 
-                            expr_s
-                        };
+/// Simple wrapper to display a slice of licensees
+pub struct DisplayList<'a, T>(pub &'a [T]);
 
-                        if found.is_empty() {
-                            log::error!("unable to find any license files for crate {}({})", krate_license.krate.name, krate_license.krate.version);
-                            return acc + 1;
-                        }
+impl<T: fmt::Display> fmt::Display for DisplayList<'_, T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "[")?;
+        for (id, val) in self.0.iter().enumerate() {
+            write!(f, "{}", val)?;
+            if id + 1 < self.0.len() {
+                write!(f, ", ")?;
+            }
+        }
+        write!(f, "]")
+    }
+}
 
-                        let expr = match spdx::Expression::parse(&expr_s) {
-                            Ok(e) => e,
-                            Err(e) => {
-                                log::error!("failed to parse SPDX license expression from synthesized string '{}': {}", expr_s, e);
-                                return acc + 1;
+pub struct Resolved(pub Vec<(KrateId, Vec<LicenseReq>)>);
+
+impl Resolved {
+    /// Find the minimal required licenses for each crate.
+    pub fn resolve<'a>(
+        licenses: &'a [KrateLicense],
+        accepted: &'a [Licensee],
+    ) -> Result<Resolved, Error> {
+        let res: Result<Vec<_>, Error> = licenses
+        .par_iter()
+        .enumerate()
+        .map(move |(id, krate_license)| {
+            // Check that the licenses found by scanning the crate contents match what was stated
+            // in the license expression
+            match krate_license.lic_info {
+                LicenseInfo::Expr(ref expr) => {
+                    let req = expr.requirements().map(|expr| expr.req.clone()).find(|req| {
+                        accepted.iter().any(|licensee| licensee.satisfies(&req))
+                    })
+                    .context(format!(
+                        "Crate '{}': Unable to satisfy [{}], with the following accepted licenses {}", krate_license.krate.name,
+                        expr, DisplayList(accepted)
+                    ))?;
+                    Ok((id, vec![req]))
+                }
+                // If the license is unknown, we will concatenate all the licenses
+                LicenseInfo::Unknown => {
+                    let license_reqs: Vec<_> = krate_license
+                        .license_files
+                        .iter()
+                        .map(|file| {
+                            LicenseReq {
+                                license: LicenseItem::SPDX {
+                                    id: file.id,
+                                    or_later: false,
+                                },
+                                exception: None,
                             }
-                        };
+                        })
+                        .collect();
 
-                        log::warn!("crate {}({}) had no license field, now using SPDX license expression '{}'", krate_license.krate.name, krate_license.krate.version, expr);
-                        krate_license.lic_info = LicenseInfo::Expr(expr);
+                    let failed_licenses: Vec<_> = license_reqs
+                        .iter()
+                        .cloned()
+                        .filter(|license| !accepted.iter().any(|a| a.satisfies(license)))
+                        .collect();
 
-                        acc
-                    },
+                    if failed_licenses.is_empty() {
+                        Ok((id, license_reqs))
+                    } else {
+                        bail!("Crate '{}': These licenses {}, could not be satisfied with the following accepted licenses {}",
+                            krate_license.krate.name,
+                            DisplayList(failed_licenses.as_slice()),
+                            DisplayList(accepted));
+                    }
                 }
-            },
-        )
-        .sum::<u32>();
-
-    if num_errors > 0 {
-        bail!(
-            "encountered {} error sanity checking crate licenses",
-            num_errors
-        );
-    } else {
-        Ok(())
+            }
+        })
+        .collect();
+        Ok(Resolved(res?))
     }
 }


Test patch: