themis: publishable crates shouldn't depend on non-publishable ones (#…

…7707) #7680 bumped the version of non-private crates to `0.15.0`. Unfortunately, the publish process failed, because `near-o11y` which is marked as a private package, is depended on by `near-vm-logic`, which is a non-private crate. This adds a check to `themis`, to ensure we never get this kind of discrepancy merged into master. Example Report: <img width="498" alt="CleanShot 2022-09-27 at 19 53 10@2x" src="https://user-images.githubusercontent.com/16881812/192575063-93c35ffb-e01e-41d9-8d4a-2d904ae85409.png">
near · Sep 27, 2022 · 8402028 · 8402028
1 parent 8aff4b4
commit 8402028
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 11 deletions.
diff --git a/core/o11y/Cargo.toml b/core/o11y/Cargo.toml
@@ -2,11 +2,13 @@
 name = "near-o11y"
 version = "0.0.0"
 authors.workspace = true
-description = "Observability helpers for the near codebase"
+publish = true
+rust-version.workspace = true
 edition.workspace = true
-publish = false
 readme = "README.md"
-rust-version.workspace = true
+license = "MIT OR Apache-2.0"
+repository = "https://github.com/near/nearcore"
+description = "Observability helpers for the near codebase"
 
 [dependencies]
 atty.workspace = true

diff --git a/core/o11y/LICENSE-APACHE b/core/o11y/LICENSE-APACHE
@@ -0,0 +1 @@
+../../licenses/LICENSE-APACHE
diff --git a/core/o11y/LICENSE-MIT b/core/o11y/LICENSE-MIT
@@ -0,0 +1 @@
+../../licenses/LICENSE-MIT
diff --git a/tools/themis/src/main.rs b/tools/themis/src/main.rs
@@ -19,6 +19,7 @@ fn main() -> anyhow::Result<()> {
         rules::publishable_has_license_file,
         rules::publishable_has_description,
         rules::publishable_has_near_link,
+        rules::recursively_publishable,
     ];
 
     let _unused_rules = [

diff --git a/tools/themis/src/rules.rs b/tools/themis/src/rules.rs
@@ -1,15 +1,15 @@
 use super::types::{ComplianceError, Expected, Outlier, Workspace};
-use super::utils;
+use super::{style, utils};
 use anyhow::bail;
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap};
 
 /// Ensure all crates have the `publish = <true/false>` specification
 pub fn has_publish_spec(workspace: &Workspace) -> anyhow::Result<()> {
     let outliers: Vec<_> = workspace
         .members
         .iter()
         .filter(|pkg| pkg.raw["package"].get("publish").is_none())
-        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None })
+        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None, extra: None })
         .collect();
 
     if !outliers.is_empty() {
@@ -29,7 +29,7 @@ pub fn has_rust_version(workspace: &Workspace) -> anyhow::Result<()> {
         .members
         .iter()
         .filter(|pkg| pkg.parsed.rust_version.is_none())
-        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None })
+        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None, extra: None })
         .collect();
 
     if !outliers.is_empty() {
@@ -56,6 +56,7 @@ pub fn is_unversioned(workspace: &Workspace) -> anyhow::Result<()> {
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: Some(pkg.parsed.version.to_string()),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -88,6 +89,7 @@ pub fn has_unified_rust_edition(workspace: &Workspace) -> anyhow::Result<()> {
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: Some(pkg.parsed.edition.clone()),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -116,6 +118,7 @@ pub fn author_is_near(workspace: &Workspace) -> anyhow::Result<()> {
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: Some(format!("{:?}", pkg.parsed.authors)),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -139,7 +142,7 @@ pub fn publishable_has_license(workspace: &Workspace) -> anyhow::Result<()> {
             utils::is_publishable(pkg)
                 && !(pkg.parsed.license.is_some() || pkg.parsed.license_file.is_some())
         })
-        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None })
+        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None, extra: None })
         .collect::<Vec<_>>();
 
     if !outliers.is_empty() {
@@ -172,6 +175,7 @@ pub fn publishable_has_license_file(workspace: &Workspace) -> anyhow::Result<()>
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: pkg.parsed.license_file.as_ref().map(ToString::to_string),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -200,6 +204,7 @@ pub fn publishable_has_unified_license(workspace: &Workspace) -> anyhow::Result<
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: pkg.parsed.license.as_ref().map(ToString::to_string),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -220,7 +225,7 @@ pub fn publishable_has_description(workspace: &Workspace) -> anyhow::Result<()>
         .members
         .iter()
         .filter(|pkg| utils::is_publishable(pkg) && pkg.parsed.description.is_none())
-        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None })
+        .map(|pkg| Outlier { path: pkg.parsed.manifest_path.clone(), found: None, extra: None })
         .collect::<Vec<_>>();
 
     if !outliers.is_empty() {
@@ -247,6 +252,7 @@ pub fn publishable_has_readme(workspace: &Workspace) -> anyhow::Result<()> {
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: pkg.parsed.readme.as_ref().map(ToString::to_string),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -275,6 +281,7 @@ pub fn publishable_has_near_link(workspace: &Workspace) -> anyhow::Result<()> {
         .map(|pkg| Outlier {
             path: pkg.parsed.manifest_path.clone(),
             found: pkg.parsed.repository.as_ref().map(ToString::to_string),
+            extra: None,
         })
         .collect::<Vec<_>>();
 
@@ -289,3 +296,41 @@ pub fn publishable_has_near_link(workspace: &Workspace) -> anyhow::Result<()> {
 
     Ok(())
 }
+
+/// Ensure all publishable crates do not depend on private crates
+pub fn recursively_publishable(workspace: &Workspace) -> anyhow::Result<()> {
+    let mut outliers = BTreeMap::new();
+    for pkg in workspace.members.iter().filter(|pkg| utils::is_publishable(pkg)) {
+        for dep in &pkg.parsed.dependencies {
+            if let Some(dep) = workspace.members.iter().find(|p| p.parsed.name == dep.name) {
+                if !utils::is_publishable(dep) {
+                    outliers
+                        .entry(dep.parsed.manifest_path.clone())
+                        .or_insert_with(Vec::new)
+                        .push(pkg.parsed.name.clone())
+                }
+            }
+        }
+    }
+    if !outliers.is_empty() {
+        bail!(ComplianceError {
+            msg: "These private packages are depended on by publishable packages".to_string(),
+            expected: None,
+            outliers: outliers
+                .into_iter()
+                .map(|(path, found)| Outlier {
+                    path,
+                    found: None,
+                    extra: Some(format!(
+                        "depended on by {}",
+                        utils::human_list(found.iter().map(|name| style::fg(style::Color::White)
+                            + style::bold()
+                            + name
+                            + style::reset()))
+                    )),
+                })
+                .collect(),
+        });
+    }
+    Ok(())
+}
diff --git a/tools/themis/src/types.rs b/tools/themis/src/types.rs
@@ -20,6 +20,7 @@ pub struct Workspace {
 pub struct Outlier {
     pub path: Utf8PathBuf,
     pub found: Option<String>,
+    pub extra: Option<String>,
 }
 
 #[derive(Debug)]
@@ -64,9 +65,9 @@ impl ComplianceError {
             c_none = style::reset()
         );
 
-        for Outlier { path, found } in &self.outliers {
+        for Outlier { path, found, extra: reason } in &self.outliers {
             report.push_str(&format!(
-                "\n {c_path}\u{21b3} {}{c_none}{}",
+                "\n {c_path}\u{21b3} {}{c_none}{}{}",
                 path.strip_prefix(&workspace.root).unwrap(),
                 match found {
                     None => "".to_string(),
@@ -79,6 +80,10 @@ impl ComplianceError {
                         c_none = style::reset()
                     ),
                 },
+                match reason {
+                    None => "".to_string(),
+                    Some(reason) => format!(" ({})", reason),
+                },
                 c_path = style::fg(style::Color::Gray { shade: 12 }),
                 c_none = style::reset(),
             ));

diff --git a/tools/themis/src/utils.rs b/tools/themis/src/utils.rs
@@ -40,3 +40,28 @@ pub fn is_publishable(pkg: &Package) -> bool {
 pub fn exists(pkg: &Package, file: &str) -> bool {
     pkg.parsed.manifest_path.parent().unwrap().join(file).exists()
 }
+
+/// Prints a string-ish iterator as a human-readable list
+///
+/// ```
+/// assert_eq!(
+///     print_list(&["a", "b", "c"]),
+///     "a, b and c"
+/// );
+/// ```
+pub fn human_list<I, T>(i: I) -> String
+where
+    I: Iterator<Item = T>,
+    T: AsRef<str>,
+{
+    let mut items = i.peekable();
+    let mut s = match items.next() {
+        Some(s) => s.as_ref().to_owned(),
+        None => return String::new(),
+    };
+    while let Some(i) = items.next() {
+        s += if items.peek().is_some() { ", " } else { " and " };
+        s += i.as_ref();
+    }
+    return s;
+}