From 2a1696906344cb4da768a940bf8b1f89bbc82b47 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 22 May 2026 04:19:14 +0900 Subject: refactor: share Pool via Rc Convert Pool to Rc> so Solver, Decisions, and RuleSetGenerator share it, resolving the todo!() placeholders that blocked the dependency resolver (Phase C shared ownership). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../shirabe/src/dependency_resolver/decisions.rs | 11 ++++--- .../src/dependency_resolver/rule_set_generator.rs | 12 ++++++-- .../src/dependency_resolver/rule_watch_graph.rs | 4 +-- crates/shirabe/src/dependency_resolver/solver.rs | 35 ++++++++++------------ 4 files changed, 33 insertions(+), 29 deletions(-) (limited to 'crates/shirabe/src/dependency_resolver') diff --git a/crates/shirabe/src/dependency_resolver/decisions.rs b/crates/shirabe/src/dependency_resolver/decisions.rs index 1fdc1d3..f0842d9 100644 --- a/crates/shirabe/src/dependency_resolver/decisions.rs +++ b/crates/shirabe/src/dependency_resolver/decisions.rs @@ -8,7 +8,7 @@ use shirabe_php_shim::LogicException; use std::fmt; pub struct Decisions { - pub(crate) pool: Pool, + pub(crate) pool: std::rc::Rc>, pub(crate) decision_map: IndexMap, pub(crate) decision_queue: Vec<(i64, Box)>, iterator_cursor: Option, @@ -27,7 +27,7 @@ impl Decisions { pub const DECISION_LITERAL: usize = 0; pub const DECISION_REASON: usize = 1; - pub fn new(pool: Pool) -> Self { + pub fn new(pool: std::rc::Rc>) -> Self { Self { pool, decision_map: IndexMap::new(), @@ -187,10 +187,9 @@ impl Decisions { let previous_decision = self.decision_map.get(&package_id).copied().unwrap_or(0); if previous_decision != 0 { - let literal_string = self - .pool - .literal_to_pretty_string(literal, &IndexMap::new()); - let package = self.pool.literal_to_package(literal); + let pool = self.pool.borrow(); + let literal_string = pool.literal_to_pretty_string(literal, &IndexMap::new()); + let package = pool.literal_to_package(literal); panic!( "{}", SolverBugException::new(format!( diff --git a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs index 57d1d18..2d99d7f 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs @@ -24,14 +24,17 @@ use crate::package::PackageInterface; #[derive(Debug)] pub struct RuleSetGenerator { pub(crate) policy: Box, - pub(crate) pool: Pool, + pub(crate) pool: std::rc::Rc>, pub(crate) rules: RuleSet, pub(crate) added_map: IndexMap>, pub(crate) added_packages_by_names: IndexMap>>, } impl RuleSetGenerator { - pub fn new(policy: Box, pool: Pool) -> Self { + pub fn new( + policy: Box, + pool: std::rc::Rc>, + ) -> Self { Self { policy, pool, @@ -221,6 +224,7 @@ impl RuleSetGenerator { let possible_requires: Vec> = self .pool + .borrow_mut() .what_provides(link.get_target(), Some(&*constraint)) .into_iter() .map(|p| p.clone_package_box()) @@ -276,6 +280,7 @@ impl RuleSetGenerator { let conflicts = self .pool + .borrow_mut() .what_provides(link.get_target(), Some(&*constraint)); for conflict in &conflicts { @@ -327,6 +332,7 @@ impl RuleSetGenerator { // fixed package was not added to the pool as it did not pass the stability requirements, this is fine if self .pool + .borrow() .is_unacceptable_fixed_or_locked_package(package.as_ref()) { continue; @@ -373,6 +379,7 @@ impl RuleSetGenerator { let packages: Vec> = self .pool + .borrow_mut() .what_provides(package_name, Some(&*constraint)) .into_iter() .map(|p| p.clone_package_box()) @@ -412,6 +419,7 @@ impl RuleSetGenerator { ) { let packages: Vec> = self .pool + .borrow() .get_packages() .iter() .map(|p| p.clone_box()) diff --git a/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs b/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs index 639061d..6c41e62 100644 --- a/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs +++ b/crates/shirabe/src/dependency_resolver/rule_watch_graph.rs @@ -44,7 +44,7 @@ impl RuleWatchGraph { self.watch_chains .get_mut(&literal) .unwrap() - .unshift(std::rc::Rc::clone(&node)); + .unshift(node.clone()); } } else { let literals: Vec = node.borrow().get_rule().get_literals().clone(); @@ -55,7 +55,7 @@ impl RuleWatchGraph { self.watch_chains .get_mut(&literal) .unwrap() - .unshift(std::rc::Rc::clone(&node)); + .unshift(node.clone()); } } } diff --git a/crates/shirabe/src/dependency_resolver/solver.rs b/crates/shirabe/src/dependency_resolver/solver.rs index f9721f8..fd30165 100644 --- a/crates/shirabe/src/dependency_resolver/solver.rs +++ b/crates/shirabe/src/dependency_resolver/solver.rs @@ -31,7 +31,7 @@ use crate::package::BasePackage; #[derive(Debug)] pub struct Solver { pub(crate) policy: Box, - pub(crate) pool: Pool, + pub(crate) pool: std::rc::Rc>, pub(crate) rules: RuleSet, @@ -55,17 +55,18 @@ impl Solver { const BRANCH_LITERALS: usize = 0; const BRANCH_LEVEL: usize = 1; - pub fn new(policy: Box, pool: Pool, io: Box) -> Self { + pub fn new( + policy: Box, + pool: std::rc::Rc>, + io: Box, + ) -> Self { + let decisions = Decisions::new(pool.clone()); Self { policy, pool, rules: RuleSet::new(), watch_graph: RuleWatchGraph::new(), - // TODO(phase-b): PHP shares `$pool` between Solver and Decisions by reference. - // Pool has no `Default`/`Clone` impl, so we leave this placeholder until the - // resolver is refactored to use `Rc>`. `solve()` rebuilds the - // decisions field before any access. - decisions: todo!("Decisions::new requires a shared Pool reference"), + decisions, fixed_map: IndexMap::new(), propagate_index: 0, branches: Vec::new(), @@ -81,8 +82,8 @@ impl Solver { self.rules.count() } - pub fn get_pool(&self) -> &Pool { - &self.pool + pub fn get_pool(&self) -> std::rc::Rc> { + self.pool.clone() } // aka solver_makeruledecisions @@ -213,6 +214,7 @@ impl Solver { if self .pool + .borrow_mut() .what_provides(package_name, Some(active_constraint)) .is_empty() { @@ -252,20 +254,15 @@ impl Solver { self.io .write_error3("Generating rules", true, crate::io::DEBUG); - // TODO(phase-b): Pool is a PHP class without Clone; RuleSetGenerator should hold - // a shared reference (Rc>). Using a placeholder pool until then. - let mut rule_set_generator = RuleSetGenerator::new( - self.policy.clone_box(), - todo!("share Pool with RuleSetGenerator"), - ); + let mut rule_set_generator = + RuleSetGenerator::new(self.policy.clone_box(), self.pool.clone()); // TODO(phase-b): get_rules_for takes Option>; // PHP passes the filter directly. Forwarding `None` here keeps the call typecheckable. let _ = platform_requirement_filter.as_ref(); self.rules = rule_set_generator.get_rules_for(request, None)?; drop(rule_set_generator); self.check_for_root_require_problems(request, platform_requirement_filter.as_ref()); - // TODO(phase-b): Pool sharing — same as above. - self.decisions = Decisions::new(todo!("share Pool with Decisions")); + self.decisions = Decisions::new(self.pool.clone()); self.watch_graph = RuleWatchGraph::new(); // TODO(phase-b): RuleSet does not expose `iter()`; RuleWatchNode expects @@ -306,7 +303,7 @@ impl Solver { // and borrows Pool/Decisions. The present/fixed maps from Request are keyed // by BasePackage; converting requires reworking Request. Ok(LockTransaction::new( - &self.pool, + &*self.pool.borrow(), todo!("convert request.get_present_map(false) to PackageInterface map"), todo!("convert request.get_fixed_packages_map() to PackageInterface map"), &self.decisions, @@ -435,7 +432,7 @@ impl Solver { ) -> anyhow::Result { // choose best package to install from decisionQueue let mut literals = self.policy.select_preferred_packages( - &self.pool, + &*self.pool.borrow(), decision_queue, rule.get_required_package(), ); -- cgit v1.3.1