Merge branch 'fix-ignored-build-dependency' into 'master'

Fix ignored build dependency

Closes #1885

See merge request redox-os/redox!2357

src/bin/repo/main.rs

@@ -12,6 +12,7 @@ use cookbook::cook::tui::join_logs;
 use cookbook::cook::{fetch_repo, ident};
 use cookbook::recipe::{
     CookRecipe, SourceRecipe, recipes_flatten_package_names, recipes_mark_as_deps,
+    validate_filesystem_rules,
 };
 use cookbook::{Error, Result, staged_pkg};
 use pkg::{PackageName, PackageState};
@@ -764,6 +765,8 @@ fn parse_args(args: Vec<String>) -> Result<(CliConfig, CliCommand, Vec<CookRecip
             recipe.apply_filesystem_config(rule)?;
         }
 
+        validate_filesystem_rules(&recipes)?;
+
         recipes
     };
 

src/recipe.rs

@@ -636,6 +636,51 @@ pub fn recipes_flatten_package_names(packages: Vec<CookRecipe>) -> Vec<CookRecip
     }
     new_packages
 }
+/// Fail early if a recipe configured as `ignore` is still required as a
+/// build dependency of another recipe that will be built.
+pub fn validate_filesystem_rules(recipes: &[CookRecipe]) -> crate::Result<()> {
+    let ignored: BTreeSet<String> = recipes
+        .iter()
+        .filter(|r| r.rule == "ignore")
+        .map(|r| r.canon_recipe_name().without_prefix().to_string())
+        .collect();
+    if ignored.is_empty() {
+        return Ok(());
+    }
+    for recipe in recipes {
+        if recipe.rule == "ignore" {
+            continue;
+        }
+        if recipe.recipe.build.kind == BuildKind::None
+            || recipe.recipe.build.kind == BuildKind::Remote
+        {
+            // Not built from source, so build dependencies are irrelevant.
+            continue;
+        }
+        let mut ignored_dependencies: Vec<&str> = Vec::new();
+        for dep in recipe
+            .recipe
+            .build
+            .dependencies
+            .iter()
+            .chain(recipe.recipe.build.dev_dependencies.iter())
+        {
+            let dep = dep.without_prefix();
+
+            if ignored.contains(dep) {
+                ignored_dependencies.push(dep);
+            }
+        }
+        if !ignored_dependencies.is_empty() {
+            bail_other_err!(
+                "Invalid filesystem config: the following packages are configured as \"ignore\" but are required as build dependencies by {:?}: {:?}",
+                ignored_dependencies,
+                recipe.name.as_str(),
+            );
+        }
+    }
+    Ok(())
+}
 
 #[derive(Serialize, Deserialize)]
 pub struct AutoDeps {
@@ -799,4 +844,86 @@ mod tests {
             }
         );
     }
+
+    #[test]
+    fn ignore_required_by_build_dep_is_rejected() {
+        use crate::recipe::{BuildKind, BuildRecipe, CookRecipe, validate_filesystem_rules};
+
+        let mut ignored = CookRecipe::dummy(&PackageName::new("libogg").unwrap());
+        ignored.rule = "ignore".into();
+
+        let mut dependent = CookRecipe::dummy(&PackageName::new("libvorbis").unwrap());
+        dependent.recipe.build = BuildRecipe::new(BuildKind::Custom {
+            script: "make".to_string(),
+        });
+        dependent.recipe.build.dependencies = vec![PackageName::new("libogg").unwrap()];
+
+        let err = validate_filesystem_rules(&[ignored, dependent]).unwrap_err();
+        let msg = format!("{}", err);
+        assert!(
+            msg.contains("libogg") && msg.contains("libvorbis"),
+            "unexpected error: {msg}"
+        );
+    }
+    #[test]
+    fn ignore_multiple_required_by_build_dep_are_rejected() {
+        use crate::recipe::{BuildKind, BuildRecipe, CookRecipe, validate_filesystem_rules};
+
+        let mut ignored_ogg = CookRecipe::dummy(&PackageName::new("libogg").unwrap());
+        ignored_ogg.rule = "ignore".into();
+
+        let mut ignored_foo = CookRecipe::dummy(&PackageName::new("libfoo").unwrap());
+        ignored_foo.rule = "ignore".into();
+
+        let mut dependent = CookRecipe::dummy(&PackageName::new("libvorbis").unwrap());
+
+        dependent.recipe.build = BuildRecipe::new(BuildKind::Custom {
+            script: "make".to_string(),
+        });
+
+        dependent.recipe.build.dependencies = vec![
+            PackageName::new("libogg").unwrap(),
+            PackageName::new("libfoo").unwrap(),
+        ];
+
+        let err = validate_filesystem_rules(&[ignored_ogg, ignored_foo, dependent]).unwrap_err();
+
+        let msg = format!("{err}");
+
+        assert!(msg.contains("libogg"), "unexpected error: {msg}");
+        assert!(msg.contains("libfoo"), "unexpected error: {msg}");
+        assert!(msg.contains("libvorbis"), "unexpected error: {msg}");
+    }
+    #[test]
+    fn ignore_required_by_dev_dep_is_rejected() {
+        use crate::recipe::{BuildKind, BuildRecipe, CookRecipe, validate_filesystem_rules};
+
+        let mut ignored = CookRecipe::dummy(&PackageName::new("libogg").unwrap());
+        ignored.rule = "ignore".into();
+
+        let mut dependent = CookRecipe::dummy(&PackageName::new("libvorbis").unwrap());
+        dependent.recipe.build = BuildRecipe::new(BuildKind::Custom {
+            script: "make".to_string(),
+        });
+        dependent.recipe.build.dev_dependencies = vec![PackageName::new("libogg").unwrap()];
+
+        let err = validate_filesystem_rules(&[ignored, dependent]).unwrap_err();
+        assert!(format!("{}", err).contains("libogg"));
+    }
+
+    #[test]
+    fn ignore_leaf_is_allowed() {
+        use crate::recipe::{BuildKind, BuildRecipe, CookRecipe, validate_filesystem_rules};
+
+        // e.g. pruning `orbterm` from a metapackage: nothing references it.
+        let mut ignored = CookRecipe::dummy(&PackageName::new("orbterm").unwrap());
+        ignored.rule = "ignore".into();
+
+        let mut meta = CookRecipe::dummy(&PackageName::new("desktop-minimal").unwrap());
+        meta.recipe.build = BuildRecipe::new(BuildKind::None);
+        meta.recipe.package.dependencies = vec![PackageName::new("orbital").unwrap()];
+
+        assert!(validate_filesystem_rules(&[ignored, meta]).is_ok());
+        assert!(validate_filesystem_rules(&[]).is_ok());
+    }
 }