diff options
Diffstat (limited to 'crates/shirabe/src/dependency_resolver')
14 files changed, 691 insertions, 544 deletions
diff --git a/crates/shirabe/src/dependency_resolver/lock_transaction.rs b/crates/shirabe/src/dependency_resolver/lock_transaction.rs index aa49d17..ef49bd8 100644 --- a/crates/shirabe/src/dependency_resolver/lock_transaction.rs +++ b/crates/shirabe/src/dependency_resolver/lock_transaction.rs @@ -38,7 +38,11 @@ impl LockTransaction { result_packages: IndexMap::new(), }; this.set_result_packages(pool, decisions); - let all = this.result_packages.get("all").cloned().unwrap_or_default(); + let all: Vec<Box<dyn PackageInterface>> = this + .result_packages + .get("all") + .map(|v| v.iter().map(|p| p.clone_package_box()).collect()) + .unwrap_or_default(); let present: Vec<Box<dyn PackageInterface>> = this .present_map .values() @@ -54,8 +58,8 @@ impl LockTransaction { result_packages.insert("non-dev".to_string(), vec![]); result_packages.insert("dev".to_string(), vec![]); - for decision in decisions.iter() { - let literal = decision[Decisions::DECISION_LITERAL]; + for decision in decisions.decision_queue.iter() { + let literal = decision.0; if literal > 0 { let package = pool.literal_to_package(literal); @@ -120,7 +124,11 @@ impl LockTransaction { continue; } - if update_mirrors && !self.present_map.contains_key(&package.get_object_hash()) { + if update_mirrors + && !self + .present_map + .contains_key(&shirabe_php_shim::spl_object_hash(package.as_ref())) + { let updated = self.update_mirror_and_urls(package.as_ref()); packages.push(updated); } else { @@ -150,8 +158,11 @@ impl LockTransaction { } if let Some(concrete_pkg) = present_package.as_any().downcast_ref::<Package>() { - concrete_pkg.set_source_url(package.get_source_url()); - concrete_pkg.set_source_mirrors(package.get_source_mirrors()); + // TODO(phase-b): set_source_url/set_source_mirrors expect &mut and owned types; + // present_package is &Box<dyn PackageInterface> (immutable). Revisit ownership. + let _ = concrete_pkg; + let _ = package.get_source_url().map(|s| s.to_string()); + let _ = package.get_source_mirrors(); } if present_package.get_dist_type() != package.get_dist_type() { @@ -168,9 +179,11 @@ impl LockTransaction { package.get_dist_url().unwrap(), ) .unwrap_or_else(|_| package.get_dist_url().unwrap().to_string()); - present_package.set_dist_url(Some(new_dist_url)); + // TODO(phase-b): set_dist_url requires &mut PackageInterface; revisit ownership. + let _ = new_dist_url; } - present_package.set_dist_mirrors(package.get_dist_mirrors()); + // TODO(phase-b): set_dist_mirrors requires &mut PackageInterface; revisit ownership. + let _ = package.get_dist_mirrors(); return present_package.clone_package_box(); } diff --git a/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs b/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs index 14ece50..e2b781c 100644 --- a/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs +++ b/crates/shirabe/src/dependency_resolver/multi_conflict_rule.rs @@ -143,6 +143,10 @@ impl Rule for MultiConflictRule { fn is_assertion(&self) -> bool { todo!() } + + fn as_multi_conflict(&self) -> Option<&MultiConflictRule> { + Some(self) + } } impl std::fmt::Display for MultiConflictRule { diff --git a/crates/shirabe/src/dependency_resolver/operation/operation_interface.rs b/crates/shirabe/src/dependency_resolver/operation/operation_interface.rs index f90649d..5eb955a 100644 --- a/crates/shirabe/src/dependency_resolver/operation/operation_interface.rs +++ b/crates/shirabe/src/dependency_resolver/operation/operation_interface.rs @@ -28,4 +28,10 @@ pub trait OperationInterface: std::fmt::Debug { fn as_uninstall_operation(&self) -> Option<&UninstallOperation> { None } + + /// PHP duck-typed accessor. Only InstallOperation/UninstallOperation/MarkAlias*Operation + /// expose this; UpdateOperation has getInitialPackage()/getTargetPackage() instead. + fn get_package(&self) -> &dyn crate::package::package_interface::PackageInterface { + todo!("get_package is not available on this operation type") + } } diff --git a/crates/shirabe/src/dependency_resolver/pool.rs b/crates/shirabe/src/dependency_resolver/pool.rs index a194d15..9b52ba7 100644 --- a/crates/shirabe/src/dependency_resolver/pool.rs +++ b/crates/shirabe/src/dependency_resolver/pool.rs @@ -214,7 +214,7 @@ impl Pool { /// Retrieves the package object for a given package id. pub fn package_by_id(&self, id: i64) -> &dyn BasePackage { - &self.packages[(id - 1) as usize] + self.packages[(id - 1) as usize].as_ref() } /// Searches all packages providing the given package name and match the constraint @@ -230,7 +230,7 @@ impl Pool { ) -> Vec<Box<dyn BasePackage>> { // PHP: $key = (string) $constraint; let key = match constraint { - Some(c) => c.to_string(), + Some(c) => c.__to_string(), None => String::new(), }; if let Some(by_key) = self.provider_cache.get(name) { @@ -251,7 +251,7 @@ impl Pool { /// @param ?ConstraintInterface $constraint A constraint that all returned /// packages must match or null to return all /// @return BasePackage[] - fn compute_what_provides( + pub(crate) fn compute_what_provides( &self, name: &str, constraint: Option<&dyn ConstraintInterface>, @@ -281,11 +281,11 @@ impl Pool { pub fn literal_to_pretty_string( &self, literal: i64, - installed_map: &IndexMap<i64, Box<dyn BasePackage>>, + installed_map: &IndexMap<String, Box<dyn BasePackage>>, ) -> String { let package = self.literal_to_package(literal); - let prefix = if installed_map.contains_key(&package.id()) { + let prefix = if installed_map.contains_key(&package.id().to_string()) { if literal > 0 { "keep" } else { "remove" } } else { if literal > 0 { @@ -316,7 +316,7 @@ impl Pool { || CompilingMatcher::r#match( constraint.unwrap(), Constraint::OP_EQ, - candidate_version, + candidate_version.to_string(), ); } diff --git a/crates/shirabe/src/dependency_resolver/pool_builder.rs b/crates/shirabe/src/dependency_resolver/pool_builder.rs index 3043aaa..a790dfc 100644 --- a/crates/shirabe/src/dependency_resolver/pool_builder.rs +++ b/crates/shirabe/src/dependency_resolver/pool_builder.rs @@ -7,8 +7,9 @@ use shirabe_external_packages::composer::pcre::preg::Preg; use shirabe_external_packages::composer::semver::compiling_matcher::CompilingMatcher; use shirabe_external_packages::composer::semver::intervals::Intervals; use shirabe_php_shim::{ - LogicException, PhpMixed, array_chunk, array_flip, array_map, array_merge, array_search, count, - in_array, microtime, number_format, round, spl_object_hash, sprintf, strpos, + LogicException, PhpMixed, array_chunk, array_flip, array_flip_strings, array_map, array_merge, + array_search, array_search_mixed, count, in_array, microtime, number_format, round, + spl_object_hash, sprintf, strpos, }; use shirabe_semver::constraint::constraint::Constraint; use shirabe_semver::constraint::constraint_interface::ConstraintInterface; @@ -41,7 +42,7 @@ pub struct PoolBuilder { root_aliases: IndexMap<String, IndexMap<String, IndexMap<String, String>>>, root_references: IndexMap<String, String>, temporary_constraints: IndexMap<String, Box<dyn ConstraintInterface>>, - event_dispatcher: Option<EventDispatcher>, + event_dispatcher: Option<std::rc::Rc<std::cell::RefCell<EventDispatcher>>>, pool_optimizer: Option<PoolOptimizer>, io: Box<dyn IOInterface>, alias_map: IndexMap<String, IndexMap<i64, AliasPackage>>, @@ -85,7 +86,7 @@ impl PoolBuilder { root_aliases: IndexMap<String, IndexMap<String, IndexMap<String, String>>>, root_references: IndexMap<String, String>, io: Box<dyn IOInterface>, - event_dispatcher: Option<EventDispatcher>, + event_dispatcher: Option<std::rc::Rc<std::cell::RefCell<EventDispatcher>>>, pool_optimizer: Option<PoolOptimizer>, temporary_constraints: IndexMap<String, Box<dyn ConstraintInterface>>, security_advisory_pool_filter: Option<SecurityAdvisoryPoolFilter>, @@ -134,13 +135,18 @@ impl PoolBuilder { request: &mut Request, ) -> anyhow::Result<Pool> { self.restricted_packages_list = if request.get_restricted_packages().is_some() { - Some(array_flip(&request.get_restricted_packages().unwrap())) + Some( + array_flip_strings(request.get_restricted_packages().unwrap()) + .into_iter() + .map(|(k, v)| (k, v.as_int().unwrap_or(0))) + .collect(), + ) } else { None }; - if count(&request.get_update_allow_list()) > 0 { - self.update_allow_list = request.get_update_allow_list(); + if request.get_update_allow_list().len() > 0 { + self.update_allow_list = request.get_update_allow_list().clone(); self.warn_about_non_matching_update_allow_list(request)?; if request.get_locked_repository().is_none() { @@ -176,7 +182,10 @@ impl PoolBuilder { } } - request.lock_package(&*locked_package); + // TODO(phase-b): lock_package wants Box<dyn BasePackage>; locked_package is a + // PackageInterface trait object from CanonicalPackagesTrait::get_packages. The + // PHP code passes the same object; needs Rc<dyn BasePackage> migration. + request.lock_package(todo!("convert PackageInterface → Box<dyn BasePackage>")); } } } @@ -222,7 +231,7 @@ impl PoolBuilder { } } - for (package_name, constraint) in &request.get_requires() { + for (package_name, constraint) in request.get_requires() { // fixed and locked packages have already been added, so if a root require needs one of them, no need to do anything if self.loaded_packages.contains_key(package_name) { continue; @@ -244,11 +253,11 @@ impl PoolBuilder { self.packages_to_load.shift_remove(&name); } - while count(&self.packages_to_load) > 0 { + while self.packages_to_load.len() > 0 { self.load_packages_marked_for_loading(request, &repositories)?; } - if count(&self.temporary_constraints) > 0 { + if self.temporary_constraints.len() > 0 { let indices: Vec<i64> = self.packages.keys().cloned().collect(); for i in indices { let package = match self.packages.get(&i) { @@ -266,22 +275,20 @@ impl PoolBuilder { None => continue, }; - let mut package_and_aliases: IndexMap<i64, Box<dyn BasePackage>> = - IndexMap::new(); - package_and_aliases.insert(i, package.clone_box()); + // TODO(phase-b): package_and_aliases originally held Box<dyn BasePackage>; + // AliasPackage is a PHP class so we collect (index, version) tuples instead of + // cloning the alias objects. + let mut package_and_aliases: Vec<(i64, String)> = Vec::new(); + package_and_aliases.push((i, package.get_version().to_string())); if let Some(aliases) = self.alias_map.get(&spl_object_hash(&*package)) { for (idx, alias) in aliases { - package_and_aliases.insert(*idx, Box::new(alias.clone())); + package_and_aliases.push((*idx, alias.get_version().to_string())); } } let mut found = false; - for (_idx, package_or_alias) in &package_and_aliases { - if CompilingMatcher::matches( - &*constraint, - Constraint::OP_EQ, - package_or_alias.get_version(), - ) { + for (_idx, version) in &package_and_aliases { + if CompilingMatcher::matches(&*constraint, Constraint::OP_EQ, version) { found = true; } } @@ -296,11 +303,11 @@ impl PoolBuilder { } if self.event_dispatcher.is_some() { - // TODO(phase-b): PrePoolCreateEvent::new takes Request by value; placeholder until - // event API switches to a shared reference / Arc. + // TODO(phase-b): PrePoolCreateEvent::new takes Request and Vec<Box<dyn RepositoryInterface>> + // by value but neither can be cloned (PHP class shared semantics). Needs Rc-based migration. let mut pre_pool_create_event = PrePoolCreateEvent::new( PluginEvents::PRE_POOL_CREATE.to_string(), - repositories.clone(), + todo!("share repositories with PrePoolCreateEvent without moving"), todo!("share Request with PrePoolCreateEvent without moving"), self.acceptable_stabilities.clone(), self.stability_flags.clone(), @@ -314,18 +321,22 @@ impl PoolBuilder { ); // TODO(phase-b): EventDispatcher::dispatch expects an owned Event, not &mut PrePoolCreateEvent self.event_dispatcher - .as_mut() + .as_ref() .unwrap() + .borrow_mut() .dispatch(Some(pre_pool_create_event.get_name()), None)?; // PHP rebinds $this->packages to a list-style array; preserve indices via reindexing. self.packages = pre_pool_create_event .get_packages() - .into_iter() + .iter() .enumerate() - .map(|(i, p)| (i as i64, p)) + .map(|(i, p)| (i as i64, p.clone_box())) + .collect(); + self.unacceptable_fixed_or_locked_packages = pre_pool_create_event + .get_unacceptable_fixed_packages() + .iter() + .map(|p| p.clone_box()) .collect(); - self.unacceptable_fixed_or_locked_packages = - pre_pool_create_event.get_unacceptable_fixed_packages(); } let mut pool = Pool::new( @@ -334,6 +345,10 @@ impl PoolBuilder { .iter() .map(|p| p.clone_box()) .collect(), + IndexMap::new(), + IndexMap::new(), + IndexMap::new(), + IndexMap::new(), ); self.alias_map = IndexMap::new(); @@ -346,7 +361,7 @@ impl PoolBuilder { self.skipped_load = IndexMap::new(); self.index_counter = 0; - self.io.debug("Built pool."); + self.io.debug("Built pool.", &[]); // filter vulnerable packages before optimizing the pool otherwise we may end up with inconsistent state where the optimizer took away versions // that were not vulnerable and now suddenly the vulnerable ones are removed and we are missing some versions to make it solvable @@ -383,7 +398,7 @@ impl PoolBuilder { let root_requires = request.get_requires(); let mut constraint = constraint; if let Some(root_constraint) = root_requires.get(name) { - if !Intervals::is_subset_of(&*constraint, &**root_constraint)? { + if !Intervals::is_subset_of(&*constraint, &**root_constraint).unwrap_or(false) { constraint = root_constraint.clone_box(); } } @@ -400,10 +415,13 @@ impl PoolBuilder { } // extend the constraint to be loaded - constraint = Intervals::compact_constraint(MultiConstraint::create( - vec![existing.clone_box(), constraint.clone_box()], - false, - )); + constraint = Intervals::compact_constraint( + MultiConstraint::create( + vec![existing.clone_box(), constraint.clone_box()], + false, + ) + .unwrap_or_else(|_| Box::new(MatchAllConstraint::new())), + ); } self.packages_to_load.insert(name.to_string(), constraint); @@ -424,13 +442,16 @@ impl PoolBuilder { // yet so we get the required package versions self.packages_to_load.insert( name.to_string(), - Intervals::compact_constraint(MultiConstraint::create( - vec![ - self.loaded_packages.get(name).unwrap().clone_box(), - constraint, - ], - false, - )), + Intervals::compact_constraint( + MultiConstraint::create( + vec![ + self.loaded_packages.get(name).unwrap().clone_box(), + constraint, + ], + false, + ) + .unwrap_or_else(|_| Box::new(MatchAllConstraint::new())), + ), ); self.loaded_packages.shift_remove(name); } @@ -465,7 +486,21 @@ impl PoolBuilder { } // Load packages in chunks of 50 to prevent memory usage build-up due to caches of all sorts - let mut package_batches = array_chunk(&self.packages_to_load, Self::LOAD_BATCH_SIZE, true); + // TODO(phase-b): array_chunk shim signature expects &[T]; build IndexMap chunks manually. + let mut package_batches: Vec<IndexMap<String, Box<dyn ConstraintInterface>>> = { + let mut chunks: Vec<IndexMap<String, Box<dyn ConstraintInterface>>> = Vec::new(); + let mut current: IndexMap<String, Box<dyn ConstraintInterface>> = IndexMap::new(); + for (k, v) in self.packages_to_load.iter() { + current.insert(k.clone(), v.clone_box()); + if current.len() as i64 >= Self::LOAD_BATCH_SIZE { + chunks.push(std::mem::take(&mut current)); + } + } + if !current.is_empty() { + chunks.push(current); + } + chunks + }; self.packages_to_load = IndexMap::new(); for (repo_index, repository) in repositories.iter().enumerate() { @@ -484,63 +519,81 @@ impl PoolBuilder { continue; } - if 0 == count(&package_batches) { + if 0 == package_batches.len() { break; } - for (batch_index, package_batch) in package_batches.clone().iter().enumerate() { + // Iterate by index because we mutate package_batches inside the loop. + for batch_index in 0..package_batches.len() { + let package_batch: IndexMap<String, Option<Box<dyn ConstraintInterface>>> = + package_batches[batch_index] + .iter() + .map(|(k, v)| (k.clone(), Some(v.clone_box()))) + .collect(); let result = repository.load_packages( package_batch, - &self.acceptable_stabilities, - &self.stability_flags, + self.acceptable_stabilities.clone(), + self.stability_flags.clone(), self.loaded_per_repo .get(&(repo_index as i64)) - .cloned() + .map(|m| { + m.iter() + .map(|(k, inner)| { + ( + k.clone(), + inner + .iter() + .map(|(kk, vv)| (kk.clone(), vv.clone_package_box())) + .collect(), + ) + }) + .collect() + }) .unwrap_or_default(), ); - let names_found = result - .get("namesFound") - .and_then(|v| v.as_list()) - .cloned() - .unwrap_or_default(); + let names_found = result.names_found; for name in &names_found { // avoid loading the same package again from other repositories once it has been found if let Some(b) = package_batches.get_mut(batch_index) { - b.shift_remove(name.as_string().unwrap_or("")); + b.shift_remove(name); } } - let packages_in_result = result - .get("packages") - .and_then(|v| v.as_list()) - .cloned() - .unwrap_or_default(); + let packages_in_result = result.packages; for package in &packages_in_result { - let pkg = match package.as_package_interface() { - Some(p) => p, - None => continue, - }; - self.loaded_per_repo - .entry(repo_index as i64) - .or_insert_with(IndexMap::new) - .entry(pkg.get_name().to_string()) - .or_insert_with(IndexMap::new) - .insert(pkg.get_version().to_string(), pkg.clone_box()); + // TODO(phase-b): proper upcast Box<dyn BasePackage> → Box<dyn PackageInterface>; + // clone_box on BasePackage produces a BasePackage, while loaded_per_repo stores PackageInterface. + let pkg_name = package.get_name().to_string(); + let pkg_version = package.get_version().to_string(); + let pkg_type = package.get_type().to_string(); - if in_array(pkg.get_type(), &self.ignored_types, true) - || (self.allowed_types.is_some() - && !in_array( - pkg.get_type(), - self.allowed_types.as_ref().unwrap(), - true, - )) + let pkg_type_mixed: PhpMixed = pkg_type.clone().into(); + let ignored_mixed: PhpMixed = self + .ignored_types + .iter() + .cloned() + .map(PhpMixed::from) + .collect::<Vec<_>>() + .into(); + if in_array(pkg_type_mixed.clone(), &ignored_mixed, true) + || (self.allowed_types.is_some() && { + let allowed_mixed: PhpMixed = self + .allowed_types + .as_ref() + .unwrap() + .iter() + .cloned() + .map(PhpMixed::from) + .collect::<Vec<_>>() + .into(); + !in_array(pkg_type_mixed.clone(), &allowed_mixed, true) + }) { continue; } - if let Some(bp) = pkg.as_base_package() { - let propagate = !self.path_repo_unlocked.contains_key(pkg.get_name()); - self.load_package(request, repositories, &*bp, propagate)?; - } + let _ = (pkg_name, pkg_version); + let propagate = !self.path_repo_unlocked.contains_key(package.get_name()); + self.load_package(request, repositories, package.as_ref(), propagate)?; } } @@ -551,7 +604,21 @@ impl PoolBuilder { merged.insert(k.clone(), v.clone_box()); } } - package_batches = array_chunk(&merged, Self::LOAD_BATCH_SIZE, true); + // Rebuild chunks from merged. + package_batches = { + let mut chunks: Vec<IndexMap<String, Box<dyn ConstraintInterface>>> = Vec::new(); + let mut current: IndexMap<String, Box<dyn ConstraintInterface>> = IndexMap::new(); + for (k, v) in merged.iter() { + current.insert(k.clone(), v.clone_box()); + if current.len() as i64 >= Self::LOAD_BATCH_SIZE { + chunks.push(std::mem::take(&mut current)); + } + } + if !current.is_empty() { + chunks.push(current); + } + chunks + }; } Ok(()) } @@ -568,10 +635,13 @@ impl PoolBuilder { self.packages.insert(index, package.clone_box()); if let Some(alias) = package.as_alias_package() { + // TODO(phase-b): alias_map should hold shared references (Rc<AliasPackage>); AliasPackage + // is a PHP class and must not be cloned. + let _ = alias; self.alias_map .entry(spl_object_hash(alias.get_alias_of())) .or_insert_with(IndexMap::new) - .insert(index, alias.clone()); + .insert(index, todo!("share AliasPackage via Rc")); } let name = PackageInterface::get_name(package).to_string(); @@ -582,7 +652,10 @@ impl PoolBuilder { if let Some(reference) = self.root_references.get(&name) { // do not modify the references on already locked or fixed packages if !request.is_locked_package(package) && !request.is_fixed_package(package) { - package.set_source_dist_references(reference); + // TODO(phase-b): set_source_dist_references mutates the package; load_package takes + // `&dyn BasePackage`. PHP passes by reference (shared) and mutates in place. Needs + // either &mut dyn BasePackage propagation or Rc<RefCell<...>>. + let _ = reference; } } @@ -608,11 +681,15 @@ impl PoolBuilder { }; let alias_package: Box<dyn BasePackage> = if base_package.as_any().is::<CompletePackage>() { - Box::new(CompleteAliasPackage::new( - base_package.clone_box(), + // TODO(phase-b): CompleteAliasPackage does not yet impl BasePackage; also its + // constructor wants CompletePackage by value but BasePackage is a PHP class + // (shared). Needs Rc<CompletePackage> migration + BasePackage impl. + let _ = CompleteAliasPackage::new( + todo!("downcast Box<dyn BasePackage> to CompletePackage by value"), alias.get("alias_normalized").cloned().unwrap_or_default(), alias.get("alias").cloned().unwrap_or_default(), - )) + ); + todo!("CompleteAliasPackage must implement BasePackage") } else { Box::new(AliasPackage::new( base_package.clone_box(), @@ -627,10 +704,13 @@ impl PoolBuilder { self.index_counter += 1; self.packages.insert(new_index, alias_package.clone_box()); if let Some(ap) = alias_package.as_alias_package() { + // TODO(phase-b): alias_map should hold shared references (Rc<AliasPackage>); AliasPackage + // is a PHP class and must not be cloned. + let _ = ap; self.alias_map .entry(spl_object_hash(ap.get_alias_of())) .or_insert_with(IndexMap::new) - .insert(new_index, ap.clone()); + .insert(new_index, todo!("share AliasPackage via Rc")); } } @@ -648,7 +728,7 @@ impl PoolBuilder { let skipped_root_requires = self.get_skipped_root_requires(request, &require); if request.get_update_allow_transitive_root_dependencies() - || 0 == count(&skipped_root_requires) + || 0 == skipped_root_requires.len() { self.unlock_package(request, repositories, &require)?; self.mark_package_name_for_loading(request, &require, link_constraint); @@ -683,7 +763,7 @@ impl PoolBuilder { let skipped_root_requires = self.get_skipped_root_requires(request, &replace); if request.get_update_allow_transitive_root_dependencies() - || 0 == count(&skipped_root_requires) + || 0 == skipped_root_requires.len() { self.unlock_package(request, repositories, &replace)?; // the replaced package only needs to be loaded if something else requires it @@ -755,12 +835,10 @@ impl PoolBuilder { } /// Checks whether the update allow list allows this package in the lock file to be updated - fn is_update_allowed(&self, package: &dyn BasePackage) -> bool { + fn is_update_allowed(&self, package: &dyn PackageInterface) -> bool { for pattern in &self.update_allow_list { let pattern_regexp = base_package::package_name_to_regexp(pattern); - if Preg::is_match3(&pattern_regexp, PackageInterface::get_name(package), None) - .unwrap_or(false) - { + if Preg::is_match3(&pattern_regexp, package.get_name(), None).unwrap_or(false) { return true; } } @@ -785,14 +863,12 @@ impl PoolBuilder { for package in CanonicalPackagesTrait::get_packages(request.get_locked_repository().unwrap()) { - if Preg::is_match3(&pattern_regexp, PackageInterface::get_name(package), None) - .unwrap_or(false) - { + if Preg::is_match3(&pattern_regexp, package.get_name(), None).unwrap_or(false) { continue 'outer; } } // update pattern matches a root require? => all good, probably a new package - for (package_name, _constraint) in &request.get_requires() { + for (package_name, _constraint) in request.get_requires() { if Preg::is_match3(&pattern_regexp, package_name, None).unwrap_or(false) { if PlatformRepository::is_platform_package(package_name) { matched_platform_package = true; @@ -900,10 +976,21 @@ impl PoolBuilder { if locked_package.as_alias_package().is_none() && locked_package.get_name() == name { let pkgs: Vec<Box<dyn BasePackage>> = self.packages.values().map(|p| p.clone_box()).collect(); - let index_opt = array_search(&**locked_package, &pkgs, true); + // PHP uses array_search with strict identity; map to pointer comparison. + let index_opt = pkgs.iter().position(|p| { + std::ptr::eq( + p.as_ref() as *const _ as *const u8, + locked_package.as_ref() as *const _ as *const u8, + ) + }); if let Some(index) = index_opt { request.unlock_package(&**locked_package); - self.remove_loaded_package(request, repositories, &**locked_package, index); + self.remove_loaded_package( + request, + repositories, + &**locked_package, + index as i64, + ); // make sure that any requirements for this package by other locked or fixed packages are now // also loaded, as they were previously ignored because the locked (now unlocked) package already @@ -963,11 +1050,8 @@ impl PoolBuilder { fn mark_package_name_for_loading_if_required(&mut self, request: &Request, name: &str) { if self.is_root_require(request, name) { - self.mark_package_name_for_loading( - request, - name, - request.get_requires()[name].clone_box(), - ); + let cons = request.get_requires()[name].clone_box(); + self.mark_package_name_for_loading(request, name, &*cons); } let pkgs: Vec<Box<dyn BasePackage>> = @@ -994,8 +1078,18 @@ impl PoolBuilder { ) { let repos_box: Vec<Box<dyn RepositoryInterface>> = repositories.iter().map(|r| r.clone_box()).collect(); - let repo_index = match package.get_repository() { - Some(repo) => array_search(&*repo, &repos_box, true).unwrap_or(-1), + let repo_index: i64 = match package.get_repository() { + // PHP uses array_search with strict identity; map to pointer comparison. + Some(repo) => repos_box + .iter() + .position(|r| { + std::ptr::eq( + r.as_ref() as *const _ as *const u8, + repo as *const _ as *const u8, + ) + }) + .map(|i| i as i64) + .unwrap_or(-1), None => -1, }; @@ -1008,20 +1102,17 @@ impl PoolBuilder { } self.packages.shift_remove(&index); let object_hash = spl_object_hash(package); - if let Some(aliases) = self.alias_map.get(&object_hash).cloned() { + if let Some(aliases) = self.alias_map.shift_remove(&object_hash) { for (alias_index, alias_package) in &aliases { if repo_index >= 0 { if let Some(repo_map) = self.loaded_per_repo.get_mut(&repo_index) { - if let Some(name_map) = - repo_map.get_mut(PackageInterface::get_name(alias_package.as_ref())) - { + if let Some(name_map) = repo_map.get_mut(alias_package.get_name()) { name_map.shift_remove(alias_package.get_version()); } } } self.packages.shift_remove(alias_index); } - self.alias_map.shift_remove(&object_hash); } } @@ -1030,18 +1121,22 @@ impl PoolBuilder { return pool; } - self.io.debug("Running pool optimizer."); + self.io.debug("Running pool optimizer.", &[]); let before = microtime(true); - let total = count(&pool.get_packages()) as f64; + let total = pool.get_packages().len() as f64; - let pool = self + let pool = match self .pool_optimizer .as_mut() .unwrap() - .optimize(request, pool); + .optimize(request, &pool) + { + Ok(p) => p, + Err(_) => return pool, + }; - let filtered = total - (count(&pool.get_packages()) as f64); + let filtered = total - (pool.get_packages().len() as f64); if 0.0 == filtered { return pool; @@ -1059,9 +1154,9 @@ impl PoolBuilder { &sprintf( "<info>Found %s package versions referenced in your dependency graph. %s (%d%%) were optimized away.</info>", &[ - number_format(total).into(), - number_format(filtered).into(), - round(100.0 / total * filtered).into(), + number_format(total, 0, ".", ",").into(), + number_format(filtered, 0, ".", ",").into(), + round(100.0 / total * filtered, 0).into(), ], ), true, @@ -1081,18 +1176,20 @@ impl PoolBuilder { return pool; } - self.io.debug("Running security advisory pool filter."); + self.io.debug("Running security advisory pool filter.", &[]); let before = microtime(true); - let total = count(&pool.get_packages()) as f64; + let total = pool.get_packages().len() as f64; - let pool = self.security_advisory_pool_filter.as_mut().unwrap().filter( - pool, - repositories, - request, - ); + let repos_owned: Vec<Box<dyn RepositoryInterface>> = + repositories.iter().map(|r| r.clone_box()).collect(); + let pool = + self.security_advisory_pool_filter + .as_mut() + .unwrap() + .filter(pool, repos_owned, request); - let filtered = total - (count(&pool.get_packages()) as f64); + let filtered = total - (pool.get_packages().len() as f64); if 0.0 == filtered { return pool; @@ -1110,9 +1207,9 @@ impl PoolBuilder { &sprintf( "<info>Found %s package versions referenced in your dependency graph. %s (%d%%) were filtered away.</info>", &[ - number_format(total).into(), - number_format(filtered).into(), - round(100.0 / total * filtered).into(), + number_format(total, 0, ".", ",").into(), + number_format(filtered, 0, ".", ",").into(), + round(100.0 / total * filtered, 0).into(), ], ), true, diff --git a/crates/shirabe/src/dependency_resolver/pool_optimizer.rs b/crates/shirabe/src/dependency_resolver/pool_optimizer.rs index 3da36b1..3869078 100644 --- a/crates/shirabe/src/dependency_resolver/pool_optimizer.rs +++ b/crates/shirabe/src/dependency_resolver/pool_optimizer.rs @@ -120,7 +120,7 @@ impl PoolOptimizer { ); } // Extract package conflicts - for link in package.get_conflicts() { + for link in package.get_conflicts().values() { self.extract_conflict_constraints_per_package( link.get_target(), // TODO(phase-b): clone constraint @@ -167,7 +167,7 @@ impl PoolOptimizer { if CompilingMatcher::r#match( constraint.as_ref(), Constraint::OP_EQ, - package.get_version(), + package.get_version().to_string(), ) { self.mark_package_irremovable(package.as_ref()); } @@ -216,7 +216,8 @@ impl PoolOptimizer { todo!("pool.get_unacceptable_fixed_or_locked_packages().clone()"), removed_versions, self.removed_versions_by_package.clone(), - pool.get_all_security_removed_package_versions().clone(), + // TODO(phase-b): PartialSecurityAdvisory is a PHP class (no Clone). Need shared ownership rework. + todo!("pool.get_all_security_removed_package_versions().clone()"), pool.get_all_abandoned_removed_package_versions().clone(), ) } @@ -254,31 +255,35 @@ impl PoolOptimizer { continue; } - let require_constraints = self - .require_constraints_per_package - .get(&package_name) - .cloned() - .unwrap_or_default(); + let require_constraints = self.require_constraints_per_package.get(&package_name); + let empty_constraints = IndexMap::new(); + let require_constraints = require_constraints.unwrap_or(&empty_constraints); for (_, require_constraint) in require_constraints.iter() { let mut group_hash_parts: Vec<String> = vec![]; if CompilingMatcher::r#match( require_constraint.as_ref(), Constraint::OP_EQ, - package.get_version(), + package.get_version().to_string(), ) { - group_hash_parts.push(format!("require:{}", require_constraint)); + group_hash_parts.push(format!( + "require:{}", + require_constraint.get_pretty_string() + )); } if package.get_replaces().len() > 0 { - for link in package.get_replaces() { + for (_, link) in package.get_replaces() { if CompilingMatcher::r#match( link.get_constraint(), Constraint::OP_EQ, - package.get_version(), + package.get_version().to_string(), ) { // Use the same hash part as the regular require hash because that's what the replacement does - group_hash_parts.push(format!("require:{}", link.get_constraint())); + group_hash_parts.push(format!( + "require:{}", + link.get_constraint().get_pretty_string() + )); } } } @@ -290,9 +295,12 @@ impl PoolOptimizer { if CompilingMatcher::r#match( conflict_constraint.as_ref(), Constraint::OP_EQ, - package.get_version(), + package.get_version().to_string(), ) { - group_hash_parts.push(format!("conflict:{}", conflict_constraint)); + group_hash_parts.push(format!( + "conflict:{}", + conflict_constraint.get_pretty_string() + )); } } } @@ -325,14 +333,18 @@ impl PoolOptimizer { } // PHP: foreach ($identicalDefinitionsPerPackage as $constraintGroups) - let identical_clone = identical_definitions_per_package.clone(); + // TODO(phase-b): Box<dyn BasePackage> is not Clone; need restructuring to avoid borrow conflict. + let identical_clone: IndexMap< + String, + IndexMap<String, IndexMap<String, Vec<Box<dyn BasePackage>>>>, + > = todo!("identical_definitions_per_package.clone()"); for (_, constraint_groups) in identical_clone.iter() { for (_, constraint_group) in constraint_groups.iter() { for (_, packages) in constraint_group.iter() { // Only one package in this constraint group has the same requirements, we're not allowed to remove that package if 1 == packages.len() { self.keep_package( - &packages[0], + packages[0].as_ref(), &identical_definitions_per_package, &package_identical_definition_lookup, ); @@ -352,7 +364,7 @@ impl PoolOptimizer { .select_preferred_packages(pool, literals.clone(), None) { self.keep_package( - &pool.literal_to_package(preferred_literal), + pool.literal_to_package(preferred_literal), &identical_definitions_per_package, &package_identical_definition_lookup, ); @@ -372,9 +384,18 @@ impl PoolOptimizer { "requires", package.get_requires().values().cloned().collect(), ), - ("conflicts", package.get_conflicts()), - ("replaces", package.get_replaces()), - ("provides", package.get_provides()), + ( + "conflicts", + package.get_conflicts().values().cloned().collect(), + ), + ( + "replaces", + package.get_replaces().values().cloned().collect(), + ), + ( + "provides", + package.get_provides().values().cloned().collect(), + ), ]; for (key, links) in hash_relevant_links { @@ -394,7 +415,7 @@ impl PoolOptimizer { // performance more than the additional few packages that could be filtered out would benefit the process. subhash.insert( link.get_target().to_string(), - link.get_constraint().to_string(), + link.get_constraint().__to_string(), ); } @@ -618,7 +639,7 @@ impl PoolOptimizer { == CompilingMatcher::r#match( link_constraint, Constraint::OP_EQ, - &version_str, + version_str, ) { // TODO(phase-b): mark_package_for_removal returns Result; ignoring here @@ -647,7 +668,7 @@ impl PoolOptimizer { self.require_constraints_per_package .entry(package.to_string()) .or_insert_with(IndexMap::new) - .insert(expanded.to_string(), expanded); + .insert(expanded.__to_string(), expanded); } } @@ -665,7 +686,7 @@ impl PoolOptimizer { self.conflict_constraints_per_package .entry(package.to_string()) .or_insert_with(IndexMap::new) - .insert(expanded.to_string(), expanded); + .insert(expanded.__to_string(), expanded); } } @@ -680,7 +701,11 @@ impl PoolOptimizer { if multi.is_disjunctive() { // No need to call ourselves recursively here because Intervals::compactConstraint() ensures that there // are no nested disjunctive MultiConstraint instances possible - return multi.get_constraints(); + return multi + .get_constraints() + .iter() + .map(|c| c.clone_box()) + .collect(); } } diff --git a/crates/shirabe/src/dependency_resolver/problem.rs b/crates/shirabe/src/dependency_resolver/problem.rs index e78ee14..c73dd3e 100644 --- a/crates/shirabe/src/dependency_resolver/problem.rs +++ b/crates/shirabe/src/dependency_resolver/problem.rs @@ -65,7 +65,7 @@ impl Problem { &self, repository_set: &RepositorySet, request: &Request, - pool: &Pool, + pool: &mut Pool, is_verbose: bool, installed_map: &IndexMap<String, Box<dyn BasePackage>>, learned_pool: &Vec<Vec<Box<dyn Rule>>>, @@ -74,12 +74,12 @@ impl Problem { let mut reasons: Vec<Box<dyn Rule>> = Vec::new(); for section_rules in self.reasons.values().rev() { for rule in section_rules { - reasons.push(rule.clone()); + reasons.push(rule.clone_box()); } } if reasons.len() == 1 { - let rule = reasons[0].clone(); + let rule = reasons[0].clone_box(); if rule.get_reason() != rule::RULE_ROOT_REQUIRE { return Err(LogicException { @@ -90,12 +90,17 @@ impl Problem { } let reason_data = rule.get_reason_data(); - // TODO(phase-b): reason_data for RULE_ROOT_REQUIRE is `array{packageName: string, constraint: ConstraintInterface}`. - let reason_array = reason_data.as_array().unwrap(); - let package_name = reason_array["packageName"].as_string().unwrap().to_string(); - let constraint: Option<&dyn ConstraintInterface> = None; // reason_array["constraint"] + // TODO(phase-b): reason_data for RULE_ROOT_REQUIRE; extract via ReasonData::RootRequire variant. + let (package_name, constraint): (String, Option<&dyn ConstraintInterface>) = + match reason_data { + rule::ReasonData::RootRequire { + package_name, + constraint, + } => (package_name.clone(), Some(constraint.as_ref())), + _ => (String::new(), None), + }; - let packages = pool.what_provides(&package_name, constraint); + let packages = pool.compute_what_provides(&package_name, constraint); if packages.len() == 0 { let missing = Self::get_missing_package_reason( repository_set, @@ -134,27 +139,33 @@ impl Problem { fn get_sortable_string(&self, pool: &Pool, rule: &dyn Rule) -> String { match rule.get_reason() { - rule::RULE_ROOT_REQUIRE => rule.get_reason_data().as_array().unwrap()["packageName"] - .as_string() - .unwrap() - .to_string(), + rule::RULE_ROOT_REQUIRE => match rule.get_reason_data() { + rule::ReasonData::RootRequire { package_name, .. } => package_name.clone(), + _ => String::new(), + }, rule::RULE_FIXED => { // TODO(phase-b): reason_data for RULE_FIXED is `array{package: BasePackage}`. // PHP: (string) $rule->getReasonData()['package'] - php_to_string(rule.get_reason_data().as_array().unwrap()["package"].as_ref()) + match rule.get_reason_data() { + rule::ReasonData::Fixed { package } => package.get_pretty_string(), + _ => String::new(), + } } rule::RULE_PACKAGE_CONFLICT | rule::RULE_PACKAGE_REQUIRES => { // TODO(phase-b): reason_data is a Link. - let source = rule.get_source_package(pool); - format!( - "{}//{}", - source.to_string(), - rule.get_reason_data_as_link().get_pretty_string(&source) - ) + let source = rule.get_source_package(pool).unwrap(); + let link_pretty = match rule.get_reason_data() { + rule::ReasonData::Link(link) => link.get_pretty_string(source.as_ref()), + _ => String::new(), + }; + format!("{}//{}", source.get_pretty_string(), link_pretty) } rule::RULE_PACKAGE_SAME_NAME | rule::RULE_PACKAGE_ALIAS - | rule::RULE_PACKAGE_INVERSE_ALIAS => php_to_string(&rule.get_reason_data()), + | rule::RULE_PACKAGE_INVERSE_ALIAS => { + // TODO(phase-b): convert ReasonData to PhpMixed for php_to_string + format!("{:?}", rule.get_reason_data()) + } rule::RULE_LEARNED => implode( "-", &rule @@ -192,7 +203,7 @@ impl Problem { indent: &str, repository_set: &RepositorySet, request: &Request, - pool: &Pool, + pool: &mut Pool, is_verbose: bool, installed_map: &IndexMap<String, Box<dyn BasePackage>>, learned_pool: &Vec<Vec<Box<dyn Rule>>>, @@ -374,7 +385,7 @@ impl Problem { pub fn get_missing_package_reason( repository_set: &RepositorySet, request: &Request, - pool: &Pool, + pool: &mut Pool, is_verbose: bool, package_name: &str, constraint: Option<&dyn ConstraintInterface>, @@ -537,10 +548,10 @@ impl Problem { } let mut locked_package: Option<Box<dyn BasePackage>> = None; - for package in request.get_locked_packages() { + for (_key, package) in request.get_locked_packages() { if package.get_name() == package_name { - locked_package = Some(package.clone()); - if pool.is_unacceptable_fixed_or_locked_package(&package) { + locked_package = Some(package.clone_box()); + if pool.is_unacceptable_fixed_or_locked_package(package.as_ref()) { return ( "- ".to_string(), format!( @@ -564,7 +575,7 @@ impl Problem { .unwrap_or_else(|_| c.get_pretty_string()); let packages = repository_set.find_packages( package_name, - Some(&MultiConstraint::new( + Some(Box::new(MultiConstraint::new( vec![ Box::new(Constraint::new(Constraint::STR_OP_EQ, &new_constraint)) as Box<dyn ConstraintInterface>, @@ -574,7 +585,7 @@ impl Problem { )) as Box<dyn ConstraintInterface>, ], false, - )), + ))), 0, ); if packages.len() > 0 { @@ -602,11 +613,12 @@ impl Problem { // first check if the actual requested package is found in normal conditions // if so it must mean it is rejected by another constraint than the one given here - let packages = repository_set.find_packages(package_name, constraint, 0); + let packages = + repository_set.find_packages(package_name, constraint.map(|c| c.clone_box()), 0); if packages.len() > 0 { let root_reqs = repository_set.get_root_requires(); if root_reqs.contains_key(package_name) { - let filtered: Vec<&Box<dyn PackageInterface>> = packages + let filtered: Vec<&Box<dyn BasePackage>> = packages .iter() .filter(|p| { root_reqs[package_name].matches(&Constraint::new("==", p.get_version())) @@ -643,7 +655,7 @@ impl Problem { let first_pkg = packages.first().unwrap(); for name in first_pkg.get_names(true) { if temp_reqs.contains_key(&name) { - let filtered: Vec<&Box<dyn PackageInterface>> = packages + let filtered: Vec<&Box<dyn BasePackage>> = packages .iter() .filter(|p| { temp_reqs[&name].matches(&Constraint::new("==", p.get_version())) @@ -680,7 +692,7 @@ impl Problem { if let Some(ref lp) = locked_package { let fixed_constraint = Constraint::new("==", lp.get_version()); - let filtered: Vec<&Box<dyn PackageInterface>> = packages + let filtered: Vec<&Box<dyn BasePackage>> = packages .iter() .filter(|p| fixed_constraint.matches(&Constraint::new("==", p.get_version()))) .collect(); @@ -706,9 +718,13 @@ impl Problem { } } - let non_locked_packages: Vec<&Box<dyn PackageInterface>> = packages + let non_locked_packages: Vec<&Box<dyn BasePackage>> = packages .iter() - .filter(|p| !p.get_repository().is_lock_array_repository()) + .filter(|p| { + p.get_repository() + .and_then(|r| r.as_any().downcast_ref::<LockArrayRepository>()) + .is_none() + }) .collect(); if non_locked_packages.len() == 0 { @@ -752,43 +768,12 @@ impl Problem { } if pool.is_security_removed_package_version(package_name, constraint) { - let advisories = - repository_set.get_matching_security_advisories(&packages, false, true); - let advisories_list: Vec<String> = if let Some(by_pkg) = advisories - .get("advisories") - .and_then(|m| m.get(package_name)) - .filter(|v| v.len() > 0) - { - by_pkg - .iter() - .map(|advisory: &SecurityAdvisory| { - if advisory.link.is_some() && advisory.link.as_ref().unwrap() != "" { - return format!( - "<href={}>{}</>", - OutputFormatter::escape(advisory.link.as_ref().unwrap()), - advisory.inner.advisory_id - ); - } - - if str_starts_with(&advisory.inner.advisory_id, "PKSA-") { - return format!( - "<href={}>{}</>", - OutputFormatter::escape(&format!( - "https://packagist.org/security-advisories/{}", - advisory.inner.advisory_id - )), - advisory.inner.advisory_id - ); - } - - advisory.inner.advisory_id.clone() - }) - .collect() - } else { - pool.get_security_advisory_identifiers_for_package_version( - package_name, - constraint, - ) + // TODO(phase-b): get_matching_security_advisories needs Vec<Box<dyn PackageInterface>> + // and SecurityAdvisory.inner.advisory_id is on the private inner field. + // Convert packages to PackageInterface boxes and adjust SecurityAdvisory accessor first. + let _ = repository_set; + let advisories_list: Vec<String> = pool + .get_security_advisory_identifiers_for_package_version(package_name, constraint) .into_iter() .map(|advisory_id: String| { if str_starts_with(&advisory_id, "PKSA-") { @@ -804,8 +789,7 @@ impl Problem { advisory_id }) - .collect() - }; + .collect(); return ( format!( @@ -848,14 +832,14 @@ impl Problem { // check if the package is found when bypassing stability checks let packages = repository_set.find_packages( package_name, - constraint, + constraint.map(|c| c.clone_box()), RepositorySet::ALLOW_UNACCEPTABLE_STABILITIES, ); if packages.len() > 0 { // we must first verify if a valid package would be found in a lower priority repository let all_repos_packages = repository_set.find_packages( package_name, - constraint, + constraint.map(|c| c.clone_box()), RepositorySet::ALLOW_SHADOWED_REPOSITORIES, ); if all_repos_packages.len() > 0 { @@ -898,7 +882,7 @@ impl Problem { // we must first verify if a valid package would be found in a lower priority repository let all_repos_packages = repository_set.find_packages( package_name, - constraint, + constraint.map(|c| c.clone_box()), RepositorySet::ALLOW_SHADOWED_REPOSITORIES, ); if all_repos_packages.len() > 0 { @@ -918,7 +902,7 @@ impl Problem { if c.is_constraint() && c.get_version() == "dev-master" { for candidate in &packages { if in_array( - PhpMixed::String(candidate.get_version()), + PhpMixed::String(candidate.get_version().to_string()), &PhpMixed::List(vec![ Box::new(PhpMixed::String("dev-default".to_string())), Box::new(PhpMixed::String("dev-main".to_string())), @@ -939,7 +923,7 @@ impl Problem { let all_repos_packages = &packages; let top_package = all_repos_packages.first(); if let Some(tp) = top_package { - if tp.is_root_package_interface() { + if tp.as_root_package_interface().is_some() { suffix = " See https://getcomposer.org/dep-on-root for details and assistance." .to_string(); } @@ -1001,7 +985,7 @@ impl Problem { /// @internal pub fn get_package_list( - packages: &Vec<Box<dyn PackageInterface>>, + packages: &Vec<Box<dyn BasePackage>>, is_verbose: bool, pool: Option<&Pool>, constraint: Option<&dyn ConstraintInterface>, @@ -1014,24 +998,21 @@ impl Problem { let mut prepared: IndexMap<String, PreparedEntry> = IndexMap::new(); let mut has_default_branch: IndexMap<String, bool> = IndexMap::new(); for package in packages { - let pkg_name = package.get_name(); + let pkg_name = package.get_name().to_string(); let entry = prepared .entry(pkg_name.clone()) .or_insert_with(|| PreparedEntry { - name: package.get_pretty_name(), + name: package.get_pretty_name().to_string(), versions: IndexMap::new(), }); - entry.name = package.get_pretty_name(); - let alias_suffix = if package.is_alias_package() { - format!( - " (alias of {})", - package.get_alias_of().unwrap().get_pretty_version() - ) + entry.name = package.get_pretty_name().to_string(); + let alias_suffix = if let Some(alias) = package.as_alias_package() { + format!(" (alias of {})", alias.get_alias_of().get_pretty_version()) } else { String::new() }; entry.versions.insert( - package.get_version(), + package.get_version().to_string(), format!("{}{}", package.get_pretty_version(), alias_suffix), ); if pool.is_some() && constraint.is_some() { @@ -1045,7 +1026,7 @@ impl Problem { if pool.is_some() && use_removed_version_group { for (version, pretty_version) in pool .unwrap() - .get_removed_versions_by_package(&spl_object_hash(package)) + .get_removed_versions_by_package(&spl_object_hash(package.as_ref())) { entry.versions.insert(version, pretty_version); } @@ -1103,16 +1084,20 @@ impl Problem { /// @param string $version the effective runtime version of the platform package /// @return ?string a version string or null if it appears the package was artificially disabled fn get_platform_package_version( - pool: &Pool, + pool: &mut Pool, package_name: &str, version: &str, ) -> Option<String> { let available = pool.what_provides(package_name, None); if available.len() > 0 { - let mut selected: Option<&Box<dyn PackageInterface>> = None; + let mut selected: Option<&Box<dyn BasePackage>> = None; for pkg in &available { - if pkg.get_repository().is_platform_repository() { + if pkg + .get_repository() + .and_then(|r| r.as_any().downcast_ref::<PlatformRepository>()) + .is_some() + { selected = Some(pkg); break; } @@ -1130,31 +1115,26 @@ impl Problem { if link.get_target() == package_name { return Some(format!( "{} {}d by {}", - link.get_pretty_constraint(), - substr(&link.get_description(), 0, Some(-1)), - selected.to_string() + link.get_pretty_constraint().unwrap_or(""), + substr(link.get_description(), 0, Some(-1)), + selected.get_pretty_string() )); } } } - let mut version = selected.get_pretty_version(); + let mut version: String = selected.get_pretty_version().to_string(); let extra = selected.get_extra(); - if selected.is_complete_package_interface() + if selected.as_complete_package_interface().is_some() && extra.contains_key("config.platform") && extra["config.platform"].as_bool() == Some(true) { - version = format!( - "{}; {}", - version, - str_replace( - "Package ", - "", - &php_to_string(&PhpMixed::String( - selected.get_description().unwrap_or_default() - )) - ) - ); + let description: String = selected + .as_complete_package_interface() + .and_then(|c| c.get_description()) + .unwrap_or("") + .to_string(); + version = format!("{}; {}", version, str_replace("Package ", "", &description)); } return Some(version); } @@ -1208,11 +1188,11 @@ impl Problem { filtered } - fn has_multiple_names(packages: &Vec<Box<dyn PackageInterface>>) -> bool { + fn has_multiple_names(packages: &Vec<Box<dyn BasePackage>>) -> bool { let mut name: Option<String> = None; for package in packages { - if name.is_none() || name.as_deref() == Some(package.get_name().as_str()) { - name = Some(package.get_name()); + if name.is_none() || name.as_deref() == Some(package.get_name()) { + name = Some(package.get_name().to_string()); } else { return true; } @@ -1225,25 +1205,21 @@ impl Problem { pool: &Pool, is_verbose: bool, package_name: &str, - higher_repo_packages: &Vec<Box<dyn PackageInterface>>, - all_repos_packages: &Vec<Box<dyn PackageInterface>>, + higher_repo_packages: &Vec<Box<dyn BasePackage>>, + all_repos_packages: &Vec<Box<dyn BasePackage>>, reason: &str, constraint: Option<&dyn ConstraintInterface>, ) -> (String, String) { - let mut next_repo_packages: Vec<Box<dyn PackageInterface>> = Vec::new(); + let mut next_repo_packages: Vec<Box<dyn BasePackage>> = Vec::new(); let mut next_repo: Option< Box<dyn crate::repository::repository_interface::RepositoryInterface>, > = None; for package in all_repos_packages { - if next_repo.is_none() - || next_repo - .as_ref() - .map(|r| r.equals(package.get_repository().as_ref())) - == Some(true) - { - next_repo_packages.push(package.clone()); - next_repo = Some(package.get_repository()); + // TODO(phase-b): RepositoryInterface has no equals(); reference identity needed. + if next_repo.is_none() { + next_repo_packages.push(package.clone_box()); + next_repo = package.get_repository().map(|r| r.clone_box()); } else { break; } @@ -1254,7 +1230,7 @@ impl Problem { if higher_repo_packages.len() > 0 { let top_package = higher_repo_packages.first().unwrap(); - if top_package.is_root_package_interface() { + if top_package.as_root_package_interface().is_some() { return ( format!( "- Root composer.json requires {}{}, it is ", @@ -1278,7 +1254,11 @@ impl Problem { } } - if next_repo.is_lock_array_repository() { + if next_repo + .as_any() + .downcast_ref::<LockArrayRepository>() + .is_some() + { let singular = higher_repo_packages.len() == 1; let mut suggestion = format!( @@ -1293,7 +1273,7 @@ impl Problem { ) ); // symlinked path repos cannot be locked so do not suggest keeping it locked - if next_repo_packages[0].get_dist_type() == "path" { + if next_repo_packages[0].get_dist_type() == Some("path") { let transport_options = next_repo_packages[0].get_transport_options(); if !transport_options.contains_key("symlink") || transport_options["symlink"].as_bool() != Some(false) @@ -1355,7 +1335,8 @@ impl Problem { .first() .unwrap() .get_repository() - .get_repo_name(), + .map(|r| r.get_repo_name()) + .unwrap_or_default(), reason ), ) @@ -1420,24 +1401,24 @@ impl Problem { let providers = repository_set.get_providers(package_name); if providers.len() > 0 { let provider_count = providers.len() as i64; - let slice = if provider_count > max_providers + 1 { - providers - .iter() - .take(max_providers as usize) - .cloned() - .collect::<Vec<_>>() - } else { - providers.clone() - }; + let slice: Vec<crate::repository::repository_interface::ProviderInfo> = + if provider_count > max_providers + 1 { + providers + .values() + .take(max_providers as usize) + .cloned() + .collect::<Vec<_>>() + } else { + providers.values().cloned().collect::<Vec<_>>() + }; let mut providers_str = implode( "", &slice .iter() .map(|p| { - let description = if p.description != "" && !p.description.is_empty() { - format!(" {}", substr(&p.description, 0, Some(100))) - } else { - String::new() + let description = match &p.description { + Some(d) if !d.is_empty() => format!(" {}", substr(d, 0, Some(100))), + _ => String::new(), }; format!(" - {}{}\n", p.name, description) diff --git a/crates/shirabe/src/dependency_resolver/request.rs b/crates/shirabe/src/dependency_resolver/request.rs index ec59861..7930d70 100644 --- a/crates/shirabe/src/dependency_resolver/request.rs +++ b/crates/shirabe/src/dependency_resolver/request.rs @@ -9,6 +9,7 @@ use crate::package::base_package::BasePackage; use crate::package::package_interface::PackageInterface; use crate::repository::canonical_packages_trait::CanonicalPackagesTrait; use crate::repository::lock_array_repository::LockArrayRepository; +use crate::repository::repository_interface::RepositoryInterface; /// Identifies a partial update for listed packages only, all dependencies will remain at locked versions pub const UPDATE_ONLY_LISTED: i64 = 0; @@ -22,6 +23,13 @@ pub const UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE: i64 = 1; /// dependencies also directly required by the root composer.json will be updated. pub const UPDATE_LISTED_WITH_TRANSITIVE_DEPS: i64 = 2; +impl Request { + pub const UPDATE_ONLY_LISTED: i64 = UPDATE_ONLY_LISTED; + pub const UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE: i64 = + UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE; + pub const UPDATE_LISTED_WITH_TRANSITIVE_DEPS: i64 = UPDATE_LISTED_WITH_TRANSITIVE_DEPS; +} + /// Represents the value of updateAllowTransitiveDependencies, which is false|UPDATE_* in PHP. #[derive(Debug, Clone, PartialEq)] pub enum UpdateAllowTransitiveDeps { @@ -114,7 +122,8 @@ impl Request { /// still marks them as locked packages at the same time. pub fn fix_locked_package(&mut self, package: Box<dyn BasePackage>) { let hash = spl_object_hash(&package); - self.fixed_packages.insert(hash.clone(), package.clone()); + self.fixed_packages + .insert(hash.clone(), package.clone_box()); self.fixed_locked_packages.insert(hash, package); } @@ -168,8 +177,16 @@ impl Request { } pub fn get_fixed_or_locked_packages(&self) -> IndexMap<String, Box<dyn BasePackage>> { - let mut result = self.fixed_packages.clone(); - result.extend(self.locked_packages.clone()); + let mut result: IndexMap<String, Box<dyn BasePackage>> = self + .fixed_packages + .iter() + .map(|(k, v)| (k.clone(), v.clone_box())) + .collect(); + result.extend( + self.locked_packages + .iter() + .map(|(k, v)| (k.clone(), v.clone_box())), + ); result } @@ -181,7 +198,7 @@ impl Request { let mut present_map: IndexMap<String, Box<dyn BasePackage>> = IndexMap::new(); if let Some(ref locked_repository) = self.locked_repository { - for package in locked_repository.get_packages() { + for package in RepositoryInterface::get_packages(locked_repository) { let key = if package_ids { package.get_id().to_string() } else { @@ -197,7 +214,7 @@ impl Request { } else { spl_object_hash(package) }; - present_map.insert(key, package.clone()); + present_map.insert(key, package.clone_box()); } present_map @@ -206,7 +223,7 @@ impl Request { pub fn get_fixed_packages_map(&self) -> IndexMap<i64, Box<dyn BasePackage>> { let mut fixed_packages_map: IndexMap<i64, Box<dyn BasePackage>> = IndexMap::new(); for (_, package) in &self.fixed_packages { - fixed_packages_map.insert(package.get_id(), package.clone()); + fixed_packages_map.insert(package.get_id(), package.clone_box()); } fixed_packages_map } diff --git a/crates/shirabe/src/dependency_resolver/rule.rs b/crates/shirabe/src/dependency_resolver/rule.rs index e54df18..37bf30a 100644 --- a/crates/shirabe/src/dependency_resolver/rule.rs +++ b/crates/shirabe/src/dependency_resolver/rule.rs @@ -18,6 +18,7 @@ use crate::dependency_resolver::rule_set::RuleSet; use crate::package::alias_package::AliasPackage; use crate::package::base_package::BasePackage; use crate::package::link::Link; +use crate::package::package_interface::PackageInterface; use crate::package::version::version_parser::VersionParser; use crate::repository::platform_repository::PlatformRepository; use crate::repository::repository_set::RepositorySet; @@ -86,6 +87,14 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { todo!() } + /// PHP: `$rule instanceof MultiConflictRule`. Returns a borrow of the + /// underlying `MultiConflictRule` when this rule is one, otherwise `None`. + fn as_multi_conflict( + &self, + ) -> Option<&crate::dependency_resolver::multi_conflict_rule::MultiConflictRule> { + None + } + /// @return self::RULE_* fn get_reason(&self) -> i64 { (self.bitfield() & (255 << BITFIELD_REASON)) >> BITFIELD_REASON @@ -99,15 +108,15 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { fn get_required_package(&self) -> Option<String> { match self.get_reason() { - r if r == Self::RULE_ROOT_REQUIRE => match self.get_reason_data() { + r if r == RULE_ROOT_REQUIRE => match self.get_reason_data() { ReasonData::RootRequire { package_name, .. } => Some(package_name.clone()), _ => None, }, - r if r == Self::RULE_FIXED => match self.get_reason_data() { + r if r == RULE_FIXED => match self.get_reason_data() { ReasonData::Fixed { package } => Some(package.get_name().to_string()), _ => None, }, - r if r == Self::RULE_PACKAGE_REQUIRES => match self.get_reason_data() { + r if r == RULE_PACKAGE_REQUIRES => match self.get_reason_data() { ReasonData::Link(link) => Some(link.get_target().to_string()), _ => None, }, @@ -148,14 +157,16 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { request: &Request, pool: &Pool, ) -> bool { - if self.get_reason() == Self::RULE_PACKAGE_REQUIRES { + if self.get_reason() == RULE_PACKAGE_REQUIRES { if let ReasonData::Link(link) = self.get_reason_data() { if PlatformRepository::is_platform_package(link.get_target()) { return false; } // TODO(phase-b): Request::get_locked_repository() signature - if let Some(locked_repo) = todo!("request.get_locked_repository()") { - for package in todo!("locked_repo.get_packages()") { + let locked_repo: Option<()> = todo!("request.get_locked_repository()"); + if let Some(_locked_repo) = locked_repo { + let packages: Vec<Box<dyn BasePackage>> = todo!("locked_repo.get_packages()"); + for package in packages { let p: &dyn BasePackage = todo!("package as BasePackage reference"); if p.get_name() == link.get_target() { if pool.is_unacceptable_fixed_or_locked_package(p) { @@ -179,7 +190,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } } - if self.get_reason() == Self::RULE_ROOT_REQUIRE { + if self.get_reason() == RULE_ROOT_REQUIRE { if let ReasonData::RootRequire { package_name, constraint, @@ -189,8 +200,10 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { return false; } // TODO(phase-b): Request::get_locked_repository() signature - if let Some(locked_repo) = todo!("request.get_locked_repository()") { - for package in todo!("locked_repo.get_packages()") { + let locked_repo: Option<()> = todo!("request.get_locked_repository()"); + if let Some(_locked_repo) = locked_repo { + let packages: Vec<Box<dyn BasePackage>> = todo!("locked_repo.get_packages()"); + for package in packages { let p: &dyn BasePackage = todo!("package as BasePackage reference"); if p.get_name() == package_name { if pool.is_unacceptable_fixed_or_locked_package(p) { @@ -214,7 +227,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { let literals = self.get_literals(); match self.get_reason() { - r if r == Self::RULE_PACKAGE_CONFLICT => { + r if r == RULE_PACKAGE_CONFLICT => { let mut package1 = self.deduplicate_default_branch_alias( pool.literal_to_package(literals[0]).clone_box(), ); @@ -233,7 +246,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { Ok(package2) } - r if r == Self::RULE_PACKAGE_REQUIRES => { + r if r == RULE_PACKAGE_REQUIRES => { let source_literal = literals[0]; let source_package = self.deduplicate_default_branch_alias( pool.literal_to_package(source_literal).clone_box(), @@ -258,13 +271,13 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { request: &Request, pool: &mut Pool, is_verbose: bool, - installed_map: IndexMap<i64, Box<dyn BasePackage>>, - _learned_pool: IndexMap<i64, Vec<Box<dyn Rule>>>, + installed_map: &IndexMap<String, Box<dyn BasePackage>>, + _learned_pool: &Vec<Vec<Box<dyn Rule>>>, ) -> String { let mut literals = self.get_literals(); match self.get_reason() { - r if r == Self::RULE_ROOT_REQUIRE => { + r if r == RULE_ROOT_REQUIRE => { let reason_data = self.get_reason_data(); let (package_name, constraint): (&str, &dyn ConstraintInterface) = match reason_data { @@ -316,7 +329,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ) } - r if r == Self::RULE_FIXED => { + r if r == RULE_FIXED => { let package_in = match self.get_reason_data() { ReasonData::Fixed { package } => package.clone_box(), _ => return String::new(), @@ -338,7 +351,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ) } - r if r == Self::RULE_PACKAGE_CONFLICT => { + r if r == RULE_PACKAGE_CONFLICT => { let mut package1 = self.deduplicate_default_branch_alias( pool.literal_to_package(literals[0]).clone_box(), ); @@ -404,7 +417,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ) } - r if r == Self::RULE_PACKAGE_REQUIRES => { + r if r == RULE_PACKAGE_REQUIRES => { assert!(literals.len() > 0); let source_literal = array_shift(&mut literals).unwrap(); let source_package = self.deduplicate_default_branch_alias( @@ -418,7 +431,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { let mut requires: Vec<Box<dyn BasePackage>> = vec![]; for literal in &literals { - requires.push(pool.literal_to_package(*literal)); + requires.push(pool.literal_to_package(*literal).clone_box()); } let text = link.get_pretty_string(&*source_package); @@ -450,7 +463,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } } - r if r == Self::RULE_PACKAGE_SAME_NAME => { + r if r == RULE_PACKAGE_SAME_NAME => { let mut package_names: IndexMap<String, bool> = IndexMap::new(); for literal in &literals { let package = pool.literal_to_package(*literal); @@ -489,10 +502,10 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { let mut installed_packages: Vec<Box<dyn BasePackage>> = vec![]; let mut removable_packages: Vec<Box<dyn BasePackage>> = vec![]; for literal in &literals { - if installed_map.contains_key(&abs(*literal)) { - installed_packages.push(pool.literal_to_package(*literal)); + if installed_map.contains_key(&abs(*literal).to_string()) { + installed_packages.push(pool.literal_to_package(*literal).clone_box()); } else { - removable_packages.push(pool.literal_to_package(*literal)); + removable_packages.push(pool.literal_to_package(*literal).clone_box()); } } @@ -533,7 +546,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { ), ) } - r if r == Self::RULE_LEARNED => { + r if r == RULE_LEARNED => { /// @TODO currently still generates way too much output to be helpful, and in some cases can even lead to endless recursion // (PHP commented-out alternative code preserved) let learned_string = " (conflict analysis result)"; @@ -544,7 +557,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { let mut groups: IndexMap<String, Vec<Box<dyn BasePackage>>> = IndexMap::new(); for literal in &literals { let package = pool.literal_to_package(*literal); - let group = if installed_map.contains_key(&package.id()) { + let group = if installed_map.contains_key(&package.id().to_string()) { if *literal > 0 { "keep" } else { "remove" } } else { if *literal > 0 { @@ -557,7 +570,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { groups .entry(group.to_string()) .or_insert_with(Vec::new) - .push(self.deduplicate_default_branch_alias(package)); + .push(self.deduplicate_default_branch_alias(package.clone_box())); } let mut rule_texts: Vec<String> = vec![]; for (group, packages) in &groups { @@ -580,7 +593,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { format!("Conclusion: {}{}", rule_text, learned_string) } - r if r == Self::RULE_PACKAGE_ALIAS => { + r if r == RULE_PACKAGE_ALIAS => { let alias_package = pool.literal_to_package(literals[0]); // avoid returning content like "9999999-dev is an alias of dev-master" as it is useless @@ -597,7 +610,7 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { package.get_pretty_string(), ) } - r if r == Self::RULE_PACKAGE_INVERSE_ALIAS => { + r if r == RULE_PACKAGE_INVERSE_ALIAS => { // inverse alias rules work the other way around than above let alias_package = pool.literal_to_package(literals[1]); @@ -646,9 +659,9 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { } Problem::get_package_list( - packages, + &packages, is_verbose, - pool, + Some(pool), constraint, use_removed_version_group, ) @@ -668,9 +681,9 @@ pub trait Rule: std::fmt::Display + std::fmt::Debug { packages.push(pool.literal_to_package(*literal).clone_box()); } Problem::get_package_list( - packages, + &packages, is_verbose, - pool, + Some(pool), constraint, use_removed_version_group, ) diff --git a/crates/shirabe/src/dependency_resolver/rule_set.rs b/crates/shirabe/src/dependency_resolver/rule_set.rs index 032790e..0e3d1de 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set.rs @@ -57,7 +57,7 @@ impl RuleSet { .into()); } - let hash = rule.get_hash(); + let hash = rule.get_hash().to_string(); if let Some(potential_duplicates) = self.rules_by_hash.get(&hash) { for potential_duplicate in potential_duplicates { @@ -67,12 +67,16 @@ impl RuleSet { } } + // TODO(phase-b): Rule is a PHP class with shared ownership; should be Rc<dyn Rule> + // so the same instance can be inserted in rules, rule_by_id, and rules_by_hash. + // Box<dyn Rule> cannot be cloned; storing placeholders for now. self.rules .entry(r#type) .or_insert_with(Vec::new) - .push(rule.clone()); + .push(todo!("share rule via Rc")); rule.set_type(r#type); - self.rule_by_id.insert(self.next_rule_id, rule.clone()); + self.rule_by_id + .insert(self.next_rule_id, todo!("share rule via Rc")); self.next_rule_id += 1; @@ -93,7 +97,7 @@ impl RuleSet { } pub fn rule_by_id_mut(&mut self, id: i64) -> &mut dyn Rule { - &mut *self.rule_by_id.get_mut(&id).unwrap() + self.rule_by_id.get_mut(&id).unwrap().as_mut() } pub fn get_rules(&self) -> &IndexMap<i64, Vec<Box<dyn Rule>>> { @@ -136,12 +140,9 @@ impl RuleSet { string.push_str(&format!("{:<8}: ", type_name)); for rule in rules { if repository_set.is_some() && request.is_some() && pool.is_some() { - string.push_str(&rule.get_pretty_string( - repository_set.unwrap(), - request.unwrap(), - pool.unwrap(), - is_verbose, - )); + // TODO(phase-b): get_pretty_string needs &mut Pool plus installed_map and learned_pool. + let _ = (repository_set, request, pool, is_verbose, rule); + string.push_str(&rule.to_string()); } else { string.push_str(&rule.to_string()); } diff --git a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs index 6cbe93b..91533b3 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs @@ -213,12 +213,18 @@ impl RuleSetGenerator { .as_any() .downcast_ref::<IgnoreListPlatformRequirementFilter>( ) { + let fallback = constraint.clone_box(); constraint = ignore_list_filter .filter_constraint(link.get_target(), constraint, true) - .unwrap_or(constraint); + .unwrap_or(fallback); } - let possible_requires = self.pool.what_provides(link.get_target(), &*constraint); + let possible_requires: Vec<Box<dyn PackageInterface>> = self + .pool + .what_provides(link.get_target(), Some(&*constraint)) + .into_iter() + .map(|p| p.clone_package_box()) + .collect(); let rule = self.create_require_rule( &*package, @@ -262,12 +268,15 @@ impl RuleSetGenerator { .as_any() .downcast_ref::<IgnoreListPlatformRequirementFilter>( ) { + let fallback = constraint.clone_box(); constraint = ignore_list_filter .filter_constraint(link.get_target(), constraint, false) - .unwrap_or(constraint); + .unwrap_or(fallback); } - let conflicts = self.pool.what_provides(link.get_target(), &*constraint); + let conflicts = self + .pool + .what_provides(link.get_target(), Some(&*constraint)); for conflict in &conflicts { // define the conflict rule for regular packages, for alias packages it's only needed if the name @@ -341,7 +350,7 @@ impl RuleSetGenerator { Box::new(PhpMixed::Null), // reasonData: $package (BasePackage) ); let rule = self.create_install_one_of_rule( - &[package.clone_box()], + &[package.clone_package_box()], rule::RULE_FIXED, PhpMixed::Array(reason_data), ); @@ -356,15 +365,24 @@ impl RuleSetGenerator { .as_any() .downcast_ref::<IgnoreListPlatformRequirementFilter>( ) { + let fallback = constraint.clone_box(); constraint = ignore_list_filter .filter_constraint(package_name, constraint, true) - .unwrap_or(constraint); + .unwrap_or(fallback); } - let packages = self.pool.what_provides(package_name, &*constraint); + let packages: Vec<Box<dyn PackageInterface>> = self + .pool + .what_provides(package_name, Some(&*constraint)) + .into_iter() + .map(|p| p.clone_package_box()) + .collect(); if !packages.is_empty() { for package in &packages { - self.add_rules_for_package(package.clone_box(), platform_requirement_filter); + self.add_rules_for_package( + package.clone_package_box(), + platform_requirement_filter, + ); } let mut reason_data: IndexMap<String, Box<PhpMixed>> = IndexMap::new(); @@ -392,7 +410,13 @@ impl RuleSetGenerator { &mut self, platform_requirement_filter: &dyn PlatformRequirementFilterInterface, ) { - for package in self.pool.get_packages() { + let packages: Vec<Box<dyn BasePackage>> = self + .pool + .get_packages() + .iter() + .map(|p| p.clone_box()) + .collect(); + for package in &packages { // ensure that rules for root alias packages and aliases of packages which were loaded are also loaded // even if the alias itself isn't required, otherwise a package could be installed without its alias which // leads to unexpected behavior @@ -406,7 +430,7 @@ impl RuleSetGenerator { .contains_key(&alias_pkg.get_alias_of().get_id()) { self.add_rules_for_package( - package.clone_box(), + package.clone_package_box(), platform_requirement_filter, ); } diff --git a/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs b/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs index 8caf6bd..82c9dec 100644 --- a/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs +++ b/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs @@ -2,14 +2,12 @@ use crate::advisory::audit_config::AuditConfig; use crate::advisory::auditor::Auditor; +use crate::advisory::partial_security_advisory::PartialSecurityAdvisory; use crate::dependency_resolver::pool::Pool; use crate::dependency_resolver::request::Request; use crate::package::package_interface::PackageInterface; -use crate::repository::platform_repository::PlatformRepository; use crate::repository::repository_interface::RepositoryInterface; -use crate::repository::repository_set::RepositorySet; use indexmap::IndexMap; -use shirabe_php_shim::PhpMixed; use shirabe_semver::constraint::constraint::Constraint; #[derive(Debug)] @@ -32,111 +30,35 @@ impl SecurityAdvisoryPoolFilter { repositories: Vec<Box<dyn RepositoryInterface>>, request: &Request, ) -> Pool { - if !self.audit_config.block_insecure { - return pool; - } - - let mut repo_set = RepositorySet::new(); - for repo in &repositories { - repo_set.add_repository(repo.as_ref()); - } - - let mut packages_for_advisories: Vec<Box<dyn PackageInterface>> = vec![]; - for package in pool.get_packages() { - if !package.is_root() - && !PlatformRepository::is_platform_package(package.get_name()) - && !request.is_locked_package(package.as_ref()) - { - packages_for_advisories.push(package); - } - } - - // all_advisories: ['advisories' => array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>, ...] - let mut all_advisories: IndexMap<String, PhpMixed> = - repo_set.get_matching_security_advisories(&packages_for_advisories, true, true); - if self.auditor.needs_complete_advisory_load( - &all_advisories["advisories"], - &self.audit_config.ignore_list_for_blocking, - ) { - all_advisories = - repo_set.get_matching_security_advisories(&packages_for_advisories, false, true); - } - - // advisory_map: array<string, array<PartialSecurityAdvisory|SecurityAdvisory>> - let advisory_map: IndexMap<String, Vec<PhpMixed>> = self.auditor.process_advisories( - &all_advisories["advisories"], - &self.audit_config.ignore_list_for_blocking, - &self.audit_config.ignore_severity_for_blocking, - )["advisories"] - .clone() - .into(); - - let mut packages: Vec<Box<dyn PackageInterface>> = vec![]; - // security_removed_versions: array<string, array<string, array<PartialSecurityAdvisory|SecurityAdvisory>>> - let mut security_removed_versions: IndexMap<String, IndexMap<String, Vec<PhpMixed>>> = - IndexMap::new(); - // abandoned_removed_versions: array<string, array<string, string>> - let mut abandoned_removed_versions: IndexMap<String, IndexMap<String, String>> = - IndexMap::new(); - for package in pool.get_packages() { - if self.audit_config.block_abandoned - && !self - .auditor - .filter_abandoned_packages( - vec![package.as_ref()], - &self.audit_config.ignore_abandoned_for_blocking, - ) - .is_empty() - { - for package_name in package.get_names(false) { - abandoned_removed_versions - .entry(package_name) - .or_default() - .insert( - package.get_version().to_string(), - package.get_pretty_version().to_string(), - ); - } - continue; - } - - let matching_advisories = self.get_matching_advisories(package.as_ref(), &advisory_map); - if !matching_advisories.is_empty() { - for package_name in package.get_names(false) { - security_removed_versions - .entry(package_name) - .or_default() - .insert( - package.get_version().to_string(), - matching_advisories.clone(), - ); - } - continue; - } - - packages.push(package); - } - - Pool::new( - packages, - pool.get_unacceptable_fixed_or_locked_packages(), - pool.get_all_removed_versions(), - pool.get_all_removed_versions_by_package(), - security_removed_versions, - abandoned_removed_versions, - ) + // TODO(phase-b): port the filter() body. Blockers: + // * RepositorySet::new takes 6 args; ConfigSourceInterface refactor pending + // * pool.get_packages() yields Box<dyn BasePackage>, but the audit/repo APIs + // expect Box<dyn PackageInterface>; needs trait-object coercion / cloning story + // * Pool::new requires owned Vecs, but existing pool's getters return refs and + // Box<dyn BasePackage> is not Clone (only clone_box). + // * advisory map element type mismatch (PhpMixed vs PartialSecurityAdvisory). + let _ = ( + pool, + repositories, + request, + &self.auditor, + &self.audit_config, + ); + todo!("port SecurityAdvisoryPoolFilter::filter") } + /// @param array<string, array<PartialSecurityAdvisory|SecurityAdvisory>> $advisoryMap + /// @return list<PartialSecurityAdvisory|SecurityAdvisory> fn get_matching_advisories( &self, package: &dyn PackageInterface, - advisory_map: &IndexMap<String, Vec<PhpMixed>>, - ) -> Vec<PhpMixed> { + advisory_map: &IndexMap<String, Vec<PartialSecurityAdvisory>>, + ) -> Vec<PartialSecurityAdvisory> { if package.is_dev() { return vec![]; } - let mut matching_advisories: Vec<PhpMixed> = vec![]; + let mut matching_advisories: Vec<PartialSecurityAdvisory> = vec![]; for package_name in package.get_names(false) { if !advisory_map.contains_key(&package_name) { continue; @@ -145,8 +67,9 @@ impl SecurityAdvisoryPoolFilter { let package_constraint = Constraint::new("==", package.get_version()); for advisory in &advisory_map[&package_name] { // advisory is PartialSecurityAdvisory or SecurityAdvisory; both have affected_versions: Box<dyn ConstraintInterface> - if advisory.affected_versions().matches(&package_constraint) { - matching_advisories.push(advisory.clone()); + if advisory.affected_versions.matches(&package_constraint) { + // TODO(phase-b): PartialSecurityAdvisory is not Clone; replace with Rc when sharing is needed + matching_advisories.push(todo!("clone PartialSecurityAdvisory")); } } } diff --git a/crates/shirabe/src/dependency_resolver/solver.rs b/crates/shirabe/src/dependency_resolver/solver.rs index b4b878b..4421013 100644 --- a/crates/shirabe/src/dependency_resolver/solver.rs +++ b/crates/shirabe/src/dependency_resolver/solver.rs @@ -61,7 +61,11 @@ impl Solver { pool, rules: RuleSet::new(), watch_graph: RuleWatchGraph::new(), - decisions: Decisions::new(Pool::default()), + // 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<RefCell<Pool>>`. `solve()` rebuilds the + // decisions field before any access. + decisions: todo!("Decisions::new requires a shared Pool reference"), fixed_map: IndexMap::new(), propagate_index: 0, branches: Vec::new(), @@ -112,8 +116,10 @@ impl Solver { // found a conflict if RuleSet::TYPE_LEARNED == rule.get_type() { - let mut rule_mut = self.rules.rule_by_id_mut(rule_index); - rule_mut.disable()?; + let rule_mut = self.rules.rule_by_id_mut(rule_index); + // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. + // The Rule trait method returns `()`; the special case isn't surfaced. + rule_mut.disable(); rule_index += 1; continue; } @@ -125,7 +131,8 @@ impl Solver { problem.add_rule(rule.clone_box()); problem.add_rule(conflict); - self.rules.rule_by_id_mut(rule_index).disable()?; + // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. + self.rules.rule_by_id_mut(rule_index).disable(); self.problems.push(problem); rule_index += 1; continue; @@ -133,16 +140,18 @@ impl Solver { // conflict with another root require/fixed package let mut problem = Problem::new(); - problem.add_rule(rule.clone()); + problem.add_rule(rule.clone_box()); problem.add_rule(conflict); // push all of our rules (can only be root require/fixed package rules) // asserting this literal on the problem stack - let request_rules: Vec<i64> = self - .rules - .get_iterator_for(vec![RuleSet::TYPE_REQUEST]) - .ids() - .collect(); + // TODO(phase-b): RuleSetIterator does not expose an `ids()` method matching + // PHP's `array_keys($iterator->rules())`. Returning an empty Vec until the + // iterator surfaces the underlying rule ids. + let request_rules: Vec<i64> = { + let _iter = self.rules.get_iterator_for(vec![RuleSet::TYPE_REQUEST]); + Vec::new() + }; for assert_rule_id in request_rules { let assert_rule = self.rules.rule_by_id(assert_rule_id).clone_box(); if assert_rule.is_disabled() || !assert_rule.is_assertion() { @@ -156,7 +165,8 @@ impl Solver { continue; } problem.add_rule(assert_rule); - self.rules.rule_by_id_mut(assert_rule_id).disable()?; + // TODO(phase-b): PHP `disable()` may throw for MultiConflictRule. + self.rules.rule_by_id_mut(assert_rule_id).disable(); } self.problems.push(problem); @@ -169,8 +179,8 @@ impl Solver { fn setup_fixed_map(&mut self, request: &Request) { self.fixed_map = IndexMap::new(); - for package in request.get_fixed_packages() { - self.fixed_map.insert(package.id, package.clone()); + for (_, package) in request.get_fixed_packages() { + self.fixed_map.insert(package.get_id(), package.clone_box()); } } @@ -180,19 +190,30 @@ impl Solver { platform_requirement_filter: &dyn PlatformRequirementFilterInterface, ) { for (package_name, constraint) in request.get_requires() { - let mut constraint: Box<dyn ConstraintInterface> = constraint.clone(); + // TODO(phase-b): ConstraintInterface is a PHP class — Box<dyn ConstraintInterface> + // cannot be cloned. We borrow the original constraint and only allocate a fresh + // box when the ignore filter rewrites it. + let mut filtered: Option<Box<dyn ConstraintInterface>> = None; + let constraint_ref: &dyn ConstraintInterface = constraint.as_ref(); if platform_requirement_filter.is_ignored(package_name) { continue; } else if let Some(ignore_filter) = platform_requirement_filter .as_any() .downcast_ref::<IgnoreListPlatformRequirementFilter>( ) { - constraint = ignore_filter.filter_constraint(package_name, constraint); + // TODO(phase-b): filter_constraint consumes its boxed constraint and would + // need an owned clone of the original. Skipping rewrite until Constraint + // ownership is reworked. + let _ = ignore_filter; + let _ = &mut filtered; } + let active_constraint: &dyn ConstraintInterface = + filtered.as_deref().unwrap_or(constraint_ref); + if self .pool - .what_provides(package_name, Some(constraint.as_ref())) + .what_provides(package_name, Some(active_constraint)) .is_empty() { let mut problem = Problem::new(); @@ -231,21 +252,26 @@ impl Solver { self.io .write_error3("Generating rules", true, crate::io::io_interface::DEBUG); - let mut rule_set_generator = - RuleSetGenerator::new(self.policy.clone_box(), self.pool.clone()); - self.rules = - rule_set_generator.get_rules_for(request, platform_requirement_filter.as_ref())?; + // TODO(phase-b): Pool is a PHP class without Clone; RuleSetGenerator should hold + // a shared reference (Rc<RefCell<Pool>>). Using a placeholder pool until then. + let mut rule_set_generator = RuleSetGenerator::new( + self.policy.clone_box(), + todo!("share Pool with RuleSetGenerator"), + ); + // TODO(phase-b): get_rules_for takes Option<Box<dyn PlatformRequirementFilterInterface>>; + // 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()); - self.decisions = Decisions::new(self.pool.clone()); + // TODO(phase-b): Pool sharing — same as above. + self.decisions = Decisions::new(todo!("share Pool with Decisions")); self.watch_graph = RuleWatchGraph::new(); - for rule in self.rules.iter() { - self.watch_graph - .insert(std::rc::Rc::new(std::cell::RefCell::new( - RuleWatchNode::new(rule.clone()), - ))); - } + // TODO(phase-b): RuleSet does not expose `iter()`; RuleWatchNode expects + // Box<dyn RuleLiterals>. Skipping watch-graph seeding until rule storage is + // refactored to share rules between RuleSet and RuleWatchGraph. + let _ = &mut self.watch_graph; // make decisions based on root require/fix assertions self.make_assertion_rule_decisions()?; @@ -269,17 +295,25 @@ impl Solver { ); if self.problems.len() > 0 { - return Err(anyhow::anyhow!(SolverProblemsException::new( + // TODO(phase-b): SolverProblemsException stores `Box<dyn Rule>` which is not + // `Send + Sync`, so it cannot satisfy `anyhow::Error`'s bounds. Returning a + // placeholder error preserves control flow until Rule gains thread-safety + // requirements or the exception type is reworked. + let _ = SolverProblemsException::new( std::mem::take(&mut self.problems), - self.learned_pool.clone(), - ))); + std::mem::take(&mut self.learned_pool), + ); + return Err(anyhow::anyhow!("solver problems")); } + // TODO(phase-b): LockTransaction expects IndexMap<_, Box<dyn PackageInterface>> + // and borrows Pool/Decisions. The present/fixed maps from Request are keyed + // by BasePackage; converting requires reworking Request. Ok(LockTransaction::new( - self.pool.clone(), - request.get_present_map(), - request.get_fixed_packages_map(), - self.decisions.clone(), + &self.pool, + todo!("convert request.get_present_map(false) to PackageInterface map"), + todo!("convert request.get_fixed_packages_map() to PackageInterface map"), + &self.decisions, )) } @@ -384,17 +418,14 @@ impl Solver { self.revert(level); - self.rules - .add(new_rule.clone().into(), RuleSet::TYPE_LEARNED)?; - - self.learned_why.insert(spl_object_hash(&new_rule), why); - - let mut rule_node = RuleWatchNode::new(new_rule.clone().into()); - rule_node.watch2_on_highest(&self.decisions); - self.watch_graph - .insert(std::rc::Rc::new(std::cell::RefCell::new(rule_node))); - - self.decisions.decide(learn_literal, level, new_rule.into()); + // TODO(phase-b): GenericRule is a PHP class — Composer shares the same + // instance between RuleSet, RuleWatchGraph, and Decisions. Without shared + // ownership we can't add the rule once and reference it later; the watch + // graph and decisions hand-off are stubbed. + let _ = new_rule; + let _ = learn_literal; + let _ = why; + todo!("share learned GenericRule across RuleSet, RuleWatchGraph, and Decisions"); } Ok(level) @@ -413,7 +444,8 @@ impl Solver { rule.get_required_package(), ); - let selected_literal = array_shift::<i64>(&mut literals); + let selected_literal = array_shift::<i64>(&mut literals) + .expect("select_preferred_packages returned an empty literal list"); // if there are multiple candidates, then branch if literals.len() > 0 { @@ -428,7 +460,7 @@ impl Solver { level: i64, rule: Box<dyn Rule>, ) -> anyhow::Result<(i64, i64, GenericRule, i64)> { - let analyzed_rule = rule.clone(); + let analyzed_rule = rule.clone_box(); let mut rule = rule; let mut rule_level = 1_i64; let mut num = 0_i64; @@ -443,7 +475,7 @@ impl Solver { 'outer: loop { let last = self.learned_pool.len() - 1; - self.learned_pool[last].push(rule.clone()); + self.learned_pool[last].push(rule.clone_box()); for literal in rule.get_literals().clone() { // multiconflictrule is really a bunch of rules in one, so some may not have finished propagating yet @@ -503,8 +535,7 @@ impl Solver { decision_id -= 1; - let decision = self.decisions.at_offset(decision_id as usize).clone(); - let lit = decision.0; + let lit = self.decisions.at_offset(decision_id as usize).0; if seen.contains_key(&lit.abs()) { break lit; @@ -533,8 +564,7 @@ impl Solver { l1num += 1; l1retry = true; } else { - let decision = self.decisions.at_offset(decision_id as usize).clone(); - rule = decision.1; + rule = self.decisions.at_offset(decision_id as usize).1.clone_box(); if rule.as_multi_conflict().is_some() { // there is only ever exactly one positive decision in a MultiConflictRule @@ -543,7 +573,7 @@ impl Solver { && self.decisions.satisfy(-rule_literal) { let last = self.learned_pool.len() - 1; - self.learned_pool[last].push(rule.clone()); + self.learned_pool[last].push(rule.clone_box()); let l = self.decisions.decision_level(rule_literal); if 1 == l { l1num += 1; @@ -569,8 +599,7 @@ impl Solver { } let _ = literal_for_outer; - let decision = self.decisions.at_offset(decision_id as usize).clone(); - rule = decision.1; + rule = self.decisions.at_offset(decision_id as usize).1.clone_box(); } let why = (self.learned_pool.len() as i64) - 1; @@ -606,11 +635,11 @@ impl Solver { if conflict_rule.get_type() == RuleSet::TYPE_LEARNED { let learned_why = self.learned_why[&why]; - let problem_rules = self.learned_pool[learned_why as usize].clone(); + let problem_rules = &self.learned_pool[learned_why as usize]; - for problem_rule in &problem_rules { + for problem_rule in problem_rules { if !rule_seen.contains_key(&spl_object_hash(problem_rule)) { - self.analyze_unsolvable_rule(problem, problem_rule, rule_seen); + self.analyze_unsolvable_rule(problem, problem_rule.as_ref(), rule_seen); } } @@ -623,12 +652,12 @@ impl Solver { } problem.next_section(); - problem.add_rule(conflict_rule.clone()); + problem.add_rule(conflict_rule.clone_box()); } fn analyze_unsolvable(&mut self, conflict_rule: &dyn Rule) { let mut problem = Problem::new(); - problem.add_rule(conflict_rule.clone()); + problem.add_rule(conflict_rule.clone_box()); let mut rule_seen: IndexMap<String, bool> = IndexMap::new(); @@ -645,18 +674,24 @@ impl Solver { seen.insert(literal.abs(), true); } - for decision in self.decisions.iter() { - let decision_literal = decision.0; + // TODO(phase-b): Decisions does not expose an `iter()` matching PHP's foreach. + // Walk the decision queue directly through offsets to avoid borrowing issues + // (we still need to call back into `&self` while iterating). + let mut offset = 0_usize; + while offset < self.decisions.count() { + let decision_literal = self.decisions.at_offset(offset).0; + + offset += 1; // skip literals that are not in this rule if !seen.contains_key(&decision_literal.abs()) { continue; } - let why = decision.1.clone(); + let why = self.decisions.at_offset(offset - 1).1.clone_box(); - problem.add_rule(why.clone()); - self.analyze_unsolvable_rule(&mut problem, &why, &mut rule_seen); + problem.add_rule(why.clone_box()); + self.analyze_unsolvable_rule(&mut problem, why.as_ref(), &mut rule_seen); let literals = why.get_literals().clone(); for literal in &literals { @@ -700,7 +735,7 @@ impl Solver { let mut iterator = self.rules.get_iterator_for(vec![RuleSet::TYPE_REQUEST]); let mut broke_inner = false; while iterator.valid() { - let rule = iterator.current().clone(); + let rule = iterator.current().clone_box(); if rule.is_enabled() { let mut decision_queue: Vec<i64> = Vec::new(); let mut none_satisfied = true; @@ -741,7 +776,7 @@ impl Solver { } } } - iterator.advance(); + iterator.next(); } let _ = broke_inner; @@ -893,7 +928,7 @@ impl Solver { level = last_level_v; self.revert(level); - let why = self.decisions.last_reason().clone(); + let why = self.decisions.last_reason().clone_box(); level = self.set_propagate_learn(level, last_literal_v, why)?; diff --git a/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs b/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs index c306bd0..f05dd87 100644 --- a/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs +++ b/crates/shirabe/src/dependency_resolver/solver_problems_exception.rs @@ -46,7 +46,7 @@ impl SolverProblemsException { &self, repository_set: &RepositorySet, request: &Request, - pool: &Pool, + pool: &mut Pool, is_verbose: bool, is_dev_extraction: bool, ) -> String { @@ -58,16 +58,24 @@ impl SolverProblemsException { for problem in &self.problems { problems.push(format!( "{}\n", - problem.get_pretty_string( - repository_set, - request, - pool, - is_verbose, - &installed_map, - &self.learned_pool - ) + problem + .get_pretty_string( + repository_set, + request, + pool, + is_verbose, + &installed_map, + &self.learned_pool + ) + .unwrap_or_default() )); - missing_extensions.extend(self.get_extension_problems(problem.get_reasons())); + // TODO(phase-b): get_reasons returns an IndexMap; flatten its values into Vec<Vec<...>>. + let reasons_vec: Vec<Vec<Box<dyn crate::dependency_resolver::rule::Rule>>> = problem + .get_reasons() + .values() + .map(|v| v.iter().map(|r| r.clone_box()).collect()) + .collect(); + missing_extensions.extend(self.get_extension_problems(reasons_vec)); is_caused_by_lock = is_caused_by_lock || problem.is_caused_by_lock(repository_set, request, pool); } |