From 26cd514925de0005bf1e41070d5fc95776f625fd Mon Sep 17 00:00:00 2001 From: Wildan M Date: Mon, 26 Jan 2026 20:41:07 +0700 Subject: [PATCH 1/3] Improve binary recipe deps detection --- src/bin/repo.rs | 273 ++++++++++++++++++++++++------------------------ src/recipe.rs | 14 ++- 2 files changed, 145 insertions(+), 142 deletions(-) diff --git a/src/bin/repo.rs b/src/bin/repo.rs index 0fabdd45c..003834cc3 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -12,7 +12,6 @@ use cookbook::cook::tree::{self, WalkTreeEntry}; use cookbook::log_to_pty; use cookbook::recipe::{CookRecipe, recipes_flatten_package_names, recipes_mark_as_deps}; use pkg::PackageName; -use pkg::package::PackageError; use ratatui::Terminal; use ratatui::layout::{Constraint, Direction, Layout, Position, Rect}; use ratatui::prelude::TermionBackend; @@ -21,7 +20,7 @@ use ratatui::text::{Line, Span, Text}; use ratatui::widgets::{Block, Borders, Clear, List, ListItem, ListState, Paragraph, Wrap}; use redox_installer::PackageConfig; use std::borrow::Cow; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::io::{Read, Write, stderr, stdin, stdout}; use std::path::PathBuf; use std::process::Command; @@ -370,26 +369,6 @@ fn publish_packages(recipe_names: &Vec, repo_path: &PathBuf) -> anyh run_command(command, &None).map_err(|e| anyhow!(e)) } -fn add_dependencies_recursive(recipe: &cookbook::recipe::Recipe, force_source: &mut HashSet) { - for dep in &recipe.build.dependencies { - if !force_source.contains(dep) { - force_source.insert(dep.clone()); - if let Ok(dep_recipe) = CookRecipe::from_name(dep.clone()) { - add_dependencies_recursive(&dep_recipe.recipe, force_source); - } - } - } - - for dep in &recipe.build.dev_dependencies { - if !force_source.contains(dep) { - force_source.insert(dep.clone()); - if let Ok(dep_recipe) = CookRecipe::from_name(dep.clone()) { - add_dependencies_recursive(&dep_recipe.recipe, force_source); - } - } - } -} - fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec)> { let mut config = CliConfig::new()?; let mut command: Option = None; @@ -460,47 +439,57 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec pkg::recipes::list(""), - Some(prefix) => pkg::recipes::list("") - .into_iter() - .filter(|p| p.starts_with(prefix)) - .collect(), - } - .iter() - // TODO: Allow selecting recipes from category as host? - .map(|f| CookRecipe::from_path(f, !command.is_cleaning(), false)) - .collect::, PackageError>>()? - } else { - if recipe_names.is_empty() { + if command.is_informational() { + // avoid extra data that clobber stdout + config.cook.verbose = false; + } + + // 1. Get the list of packages + // 2. Put them into list to build or download + // 3. Expand with package deps for both list + // 4. Expand build deps for things to build + // 5. Merge both list + + // early overrides for "ignore" and "local", also preloaded for recipes when doing all clean + let mut preloaded_recipes: BTreeMap = BTreeMap::new(); + + if recipe_names.is_empty() { + if config.all || config.category.is_some() { + if !recipe_names.is_empty() { + bail!("Do not specify recipe names when using the --all or --category flag."); + } + if config.all && config.category.is_some() { + bail!("Do not specify both --all and --category flag."); + } + if config.all && !command.is_cleaning() { + // because read_recipe is false by logic below + // some recipes on wip folders are invalid anyway + bail!( + "Refusing to run an unrealistic command to {} all recipes", + command.to_string() + ); + } + let all_recipes_path = match &config.category { + None => pkg::recipes::list(""), + Some(prefix) => pkg::recipes::list("") + .into_iter() + .filter(|p| p.starts_with(prefix)) + .collect(), + }; + + for path in all_recipes_path { + // TODO: Allow selecting recipes from category as host? + let recipe = CookRecipe::from_path(&path, !command.is_cleaning(), false)?; + let recipe_name = recipe.name.clone(); + preloaded_recipes.insert(recipe_name.clone(), recipe); + recipe_names.push(recipe_name); + } + } else { if let Some(conf) = config.filesystem.as_ref() { recipe_names = conf .packages - .iter() - .filter_map(|(f, v)| { - match v { - PackageConfig::Build(rule) if rule == "ignore" => { - return None; - } - _ => {} - } - PackageName::new(f).ok() - }) + .keys() + .filter_map(|k| PackageName::new(k.to_string()).ok()) .collect(); } else { bail!( @@ -508,95 +497,101 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec = std::collections::HashSet::new(); - let force_binary_recipes: std::collections::HashSet = std::collections::HashSet::new(); - let mut should_drop_host_packages = true; - - // When repo_binary=1, collect "source" recipes and their dependencies - if repo_binary { - for recipe in recipes.iter() { - if let Some(conf) = conf.packages.get(recipe.name.as_str()) { - let rule = match conf { - PackageConfig::Build(rule) => rule.as_str(), - _ => "binary", - }; - if rule == "source" { - force_source_recipes.insert(recipe.name.clone()); - add_dependencies_recursive(&recipe.recipe, &mut force_source_recipes); - should_drop_host_packages = false; - } - } - } - } - - for recipe in recipes.iter_mut() { - let last_rule = match (force_source_recipes.contains(&recipe.name), force_binary_recipes.contains(&recipe.name)) { - (true, true) => { - // both lists: flip logic - if repo_binary { "source" } else { "binary" } - }, - (true, false) => "source", - (false, true) => "binary", - (false, false) => { - // check config or use default - if let Some(conf) = conf.packages.get(recipe.name.as_str()) { - match conf { - PackageConfig::Build(rule) => { - let rule_str = rule.as_str(); - if should_drop_host_packages && (rule_str == "source" || rule_str == "local") { - should_drop_host_packages = false; - } - rule_str - }, - _ => { - if repo_binary { "binary" } else { "source" } - } - } + + // Derive "source" + "local" and "binary" + "ignore" + let mut source_recipe_names: Vec = Vec::new(); + let mut binary_recipe_names: Vec = Vec::new(); + + for recipe_name in recipe_names.iter() { + let rule = match conf.packages.get(recipe_name.as_str()) { + Some(PackageConfig::Build(rule)) => rule.as_str(), + _ => { + if repo_binary { + "binary" } else { - if repo_binary { "binary" } else { "source" } + "source" } } }; - recipe - .apply_filesystem_config(last_rule) - .map_err(|e| anyhow!(e))?; - } - // If there's no building from source, drop all host toolchain - // TODO: This is more of a hack to make CI passing - if should_drop_host_packages && config.with_package_deps { - recipes = recipes.into_iter().filter(|p| !p.name.is_host()).collect(); - } - } - if command.is_informational() { - // avoid extra data that clobber stdout - config.cook.verbose = false; + if rule == "source" || rule == "local" { + source_recipe_names.push(recipe_name.clone()); + } else { + binary_recipe_names.push(recipe_name.clone()); + } + if rule == "local" || rule == "ignore" { + // these don't inherit into their deps + let mut recipe = CookRecipe::from_name(recipe_name.clone())?; + recipe.apply_filesystem_config(rule)?; + preloaded_recipes.insert(recipe_name.clone(), recipe); + } + } + if config.with_package_deps { + source_recipe_names = + CookRecipe::get_package_deps_recursive(&source_recipe_names, true)?; + binary_recipe_names = + CookRecipe::get_package_deps_recursive(&binary_recipe_names, true)?; + } + + let mut recipes = if command.is_building() { + CookRecipe::get_build_deps_recursive(&source_recipe_names, true)? + } else { + CookRecipe::from_list(source_recipe_names.clone())? + }; + + recipes.extend(CookRecipe::from_list(binary_recipe_names.clone())?); + recipes = recipes_flatten_package_names(recipes); + + for recipe in recipes.iter_mut() { + if let Some(preloaded) = preloaded_recipes.remove(&recipe.name) { + // can come from --category flag, which doesn't have specific rule + if !preloaded.rule.is_empty() { + recipe.apply_filesystem_config(&preloaded.rule)?; + continue; + } + } + let rule = match ( + source_recipe_names.contains(&recipe.name), + binary_recipe_names.contains(&recipe.name), + ) { + (true, true) => { + // both lists: flip logic + if repo_binary { "source" } else { "binary" } + } + (true, false) => "source", + (false, true) => "binary", + (false, false) => { + // should not be possible to go here + if repo_binary { "binary" } else { "source" } + } + }; + recipe.apply_filesystem_config(rule)?; + } + recipes + } else { + CookRecipe::from_list(recipe_names.clone())? + }; + + if command.is_pushing() || !config.with_package_deps { + // In CliCommand::Cook, is_deps==true will make it skip checking source + recipes_mark_as_deps(&recipe_names, &mut recipes); } Ok((config, command, recipes)) diff --git a/src/recipe.rs b/src/recipe.rs index 006f75051..7ae97e872 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -267,6 +267,14 @@ impl CookRecipe { Self::new(name, dir.to_path_buf(), recipe) } + pub fn from_list(names: Vec) -> Result, PackageError> { + let mut packages = Vec::new(); + for name in names { + packages.push(Self::from_name(name)?); + } + Ok(packages) + } + pub fn from_path(dir: &Path, read_recipe: bool, is_host: bool) -> Result { let file = dir.join("recipe.toml"); let mut name: PackageName = dir.file_name().unwrap().try_into()?; @@ -438,7 +446,7 @@ impl CookRecipe { self.dir.join("target").join(self.target) } - pub fn apply_filesystem_config(&mut self, rule: &str) -> Result<(), String> { + pub fn apply_filesystem_config(&mut self, rule: &str) -> Result<(), anyhow::Error> { match rule { // build from source as usual "source" => {} @@ -457,12 +465,12 @@ impl CookRecipe { self.recipe.build.set_as_none(); } rule => { - return Err(format!( + anyhow::bail!( // Fail fast because we could risk losing local changes if "local" was typo'ed "Invalid pkg config {} = \"{}\"\nExpecting either 'source', 'local', 'binary' or 'ignore'", self.name.as_str(), rule - )); + ); } } self.rule = rule.to_string(); From 7bd5e1e087d64bc1cba8ed66a0b608e1b327e6c7 Mon Sep 17 00:00:00 2001 From: Wildan M Date: Mon, 26 Jan 2026 21:36:53 +0700 Subject: [PATCH 2/3] Build filesystem complete map --- src/bin/repo.rs | 84 +++++++++++++++++++++++++++++++------------------ src/recipe.rs | 10 ++++++ 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/src/bin/repo.rs b/src/bin/repo.rs index 003834cc3..b6267ef2f 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -516,33 +516,46 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec = Vec::new(); - let mut binary_recipe_names: Vec = Vec::new(); - - for recipe_name in recipe_names.iter() { - let rule = match conf.packages.get(recipe_name.as_str()) { - Some(PackageConfig::Build(rule)) => rule.as_str(), - _ => { - if repo_binary { - "binary" - } else { - "source" - } - } + // This is the complete map from filesystem config + let mut source_names: Vec = Vec::new(); + let mut binary_names: Vec = Vec::new(); + let mut special_rules: HashMap = HashMap::new(); + let default_rule = if repo_binary { "binary" } else { "source" }; + for (recipe_name_str, recipe_config) in conf.packages.iter() { + let Ok(recipe_name) = PackageName::new(recipe_name_str) else { + continue; + }; + let rule = match recipe_config { + PackageConfig::Build(rule) => rule, + _ => default_rule, }; if rule == "source" || rule == "local" { - source_recipe_names.push(recipe_name.clone()); + source_names.push(recipe_name.clone()); } else { - binary_recipe_names.push(recipe_name.clone()); + binary_names.push(recipe_name.clone()); } if rule == "local" || rule == "ignore" { - // these don't inherit into their deps - let mut recipe = CookRecipe::from_name(recipe_name.clone())?; - recipe.apply_filesystem_config(rule)?; - preloaded_recipes.insert(recipe_name.clone(), recipe); + special_rules.insert(recipe_name, rule.to_string()); } } + source_names = CookRecipe::get_all_deps_names_recursive(&source_names, true)?; + binary_names = CookRecipe::get_all_deps_names_recursive(&binary_names, false)?; + let source_names: HashSet = source_names.into_iter().collect(); + let binary_names: HashSet = binary_names.into_iter().collect(); + + // These are list that derived from recipe_names + let mut source_recipe_names: Vec = Vec::new(); + let mut binary_recipe_names: Vec = Vec::new(); + for recipe_name in recipe_names.iter() { + if source_names.contains(recipe_name) { + source_recipe_names.push(recipe_name.clone()); + } + if binary_names.contains(recipe_name) { + binary_recipe_names.push(recipe_name.clone()); + } + } + if config.with_package_deps { source_recipe_names = CookRecipe::get_package_deps_recursive(&source_recipe_names, true)?; @@ -556,20 +569,23 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec { // both lists: flip logic @@ -579,14 +595,22 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec "binary", (false, false) => { // should not be possible to go here - if repo_binary { "binary" } else { "source" } + default_rule } }; recipe.apply_filesystem_config(rule)?; } + recipes } else { - CookRecipe::from_list(recipe_names.clone())? + if config.with_package_deps { + recipe_names = CookRecipe::get_package_deps_recursive(&recipe_names, true)?; + } + if command.is_building() { + CookRecipe::get_build_deps_recursive(&recipe_names, true)? + } else { + CookRecipe::from_list(recipe_names.clone())? + } }; if command.is_pushing() || !config.with_package_deps { diff --git a/src/recipe.rs b/src/recipe.rs index 7ae97e872..63fc5a09e 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -427,6 +427,16 @@ impl CookRecipe { Ok(packages.into_iter().map(|p| p.name).collect()) } + pub fn get_all_deps_names_recursive( + names: &[PackageName], + include_dev: bool, + ) -> Result, PackageError> { + let packages = + Self::new_recursive(names, true, include_dev, true, true, true, true, WALK_DEPTH)?; + + Ok(packages.into_iter().map(|p| p.name).collect()) + } + pub fn reload_recipe(&mut self) -> Result<(), PackageError> { self.recipe = Self::from_path(&self.dir, true, self.name.is_host())?.recipe; let _ = self.apply_filesystem_config(&self.rule.clone()); From 1a3ac0293762e6e525f54676aa020912a62e8626 Mon Sep 17 00:00:00 2001 From: Wildan M Date: Mon, 26 Jan 2026 23:00:10 +0700 Subject: [PATCH 3/3] Handle ignore separately --- src/bin/repo.rs | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/bin/repo.rs b/src/bin/repo.rs index b6267ef2f..913e885d9 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -444,13 +444,6 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec = BTreeMap::new(); if recipe_names.is_empty() { @@ -501,10 +494,7 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec = Vec::new(); let mut binary_names: Vec = Vec::new(); @@ -532,10 +522,10 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec = Vec::new(); let mut binary_recipe_names: Vec = Vec::new(); + let mut ignore_recipe_names: Vec = Vec::new(); for recipe_name in recipe_names.iter() { if source_names.contains(recipe_name) { source_recipe_names.push(recipe_name.clone()); - } - if binary_names.contains(recipe_name) { + } else if binary_names.contains(recipe_name) { binary_recipe_names.push(recipe_name.clone()); + } else { + if special_rules + .get(recipe_name) + .is_some_and(|s| s == "ignore") + { + ignore_recipe_names.push(recipe_name.clone()); + } else if repo_binary { + binary_recipe_names.push(recipe_name.clone()); + } else { + source_recipe_names.push(recipe_name.clone()); + } } } @@ -575,7 +576,10 @@ fn parse_args(args: Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec) -> anyhow::Result<(CliConfig, CliCommand, Vec "source", (false, true) => "binary", - (false, false) => { - // should not be possible to go here - default_rule - } + (false, false) => default_rule, }; recipe.apply_filesystem_config(rule)?; }