From 1921f173ea219cb4b25847294d2d3fa465550fbb Mon Sep 17 00:00:00 2001 From: nsfisis Date: Mon, 25 May 2026 00:58:20 +0900 Subject: refactor(package): introduce Rc> handles for packages PHP packages have reference semantics, so introduce shared-ownership handles over an AnyPackage enum (PackageInterfaceHandle and friends) and replace Box throughout. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/dependency_resolver/default_policy.rs | 48 ++-- .../dependency_resolver/local_repo_transaction.rs | 6 +- .../src/dependency_resolver/lock_transaction.rs | 78 +++---- .../operation/install_operation.rs | 11 +- .../operation/mark_alias_installed_operation.rs | 10 +- .../operation/mark_alias_uninstalled_operation.rs | 10 +- .../operation/uninstall_operation.rs | 11 +- .../operation/update_operation.rs | 19 +- .../src/dependency_resolver/policy_interface.rs | 10 +- crates/shirabe/src/dependency_resolver/pool.rs | 56 ++--- .../src/dependency_resolver/pool_builder.rs | 248 ++++++++------------- .../src/dependency_resolver/pool_optimizer.rs | 102 ++++----- crates/shirabe/src/dependency_resolver/problem.rs | 114 +++++----- crates/shirabe/src/dependency_resolver/request.rs | 68 +++--- crates/shirabe/src/dependency_resolver/rule.rs | 105 ++++----- .../src/dependency_resolver/rule_set_generator.rs | 117 ++++------ .../security_advisory_pool_filter.rs | 9 +- crates/shirabe/src/dependency_resolver/solver.rs | 25 ++- .../shirabe/src/dependency_resolver/transaction.rs | 125 ++++++----- 19 files changed, 521 insertions(+), 651 deletions(-) (limited to 'crates/shirabe/src/dependency_resolver') diff --git a/crates/shirabe/src/dependency_resolver/default_policy.rs b/crates/shirabe/src/dependency_resolver/default_policy.rs index cc33b81..0c6b2fc 100644 --- a/crates/shirabe/src/dependency_resolver/default_policy.rs +++ b/crates/shirabe/src/dependency_resolver/default_policy.rs @@ -1,6 +1,5 @@ //! ref: composer/src/Composer/DependencyResolver/DefaultPolicy.php -use std::any::Any; use std::cell::RefCell; use indexmap::IndexMap; @@ -10,9 +9,8 @@ use shirabe_semver::constraint::SimpleConstraint; use crate::dependency_resolver::PolicyInterface; use crate::dependency_resolver::Pool; -use crate::package::AliasPackage; -use crate::package::PackageInterface; -use crate::package::{BasePackage, STABILITIES}; +use crate::package::BasePackageHandle; +use crate::package::STABILITIES; use crate::util::Platform; #[derive(Debug)] @@ -46,14 +44,14 @@ impl DefaultPolicy { pub fn compare_by_priority( &self, pool: &Pool, - a: &dyn BasePackage, - b: &dyn BasePackage, + a: &BasePackageHandle, + b: &BasePackageHandle, required_package: Option, ignore_replace: bool, ) -> i64 { - if PackageInterface::get_name(a) == PackageInterface::get_name(b) { - let a_aliased = a.as_any().downcast_ref::().is_some(); - let b_aliased = b.as_any().downcast_ref::().is_some(); + if a.get_name() == b.get_name() { + let a_aliased = a.as_alias().is_some(); + let b_aliased = b.as_alias().is_some(); if a_aliased && !b_aliased { return -1; } @@ -73,10 +71,8 @@ impl DefaultPolicy { if let Some(ref required_package) = required_package { if let Some(pos) = required_package.find('/') { let required_vendor = &required_package[..pos]; - let a_is_same_vendor = - PackageInterface::get_name(a).starts_with(required_vendor); - let b_is_same_vendor = - PackageInterface::get_name(b).starts_with(required_vendor); + let a_is_same_vendor = a.get_name().starts_with(required_vendor); + let b_is_same_vendor = b.get_name().starts_with(required_vendor); if b_is_same_vendor != a_is_same_vendor { return if a_is_same_vendor { -1 } else { 1 }; } @@ -112,7 +108,7 @@ impl DefaultPolicy { .iter() .copied() .filter(|&literal| { - pool.literal_to_package(literal).get_version() == preferred_version + pool.literal_to_package(literal).get_version() == *preferred_version }) .collect(); if !best_literals.is_empty() { @@ -129,10 +125,10 @@ impl DefaultPolicy { continue; } let package = pool.literal_to_package(literal); - if self.version_compare(package, best_package, operator) { + if self.version_compare(&package, &best_package, operator) { best_package = package; best_literals = vec![literal]; - } else if self.version_compare(package, best_package, "==") { + } else if self.version_compare(&package, &best_package, "==") { best_literals.push(literal); } } @@ -144,7 +140,7 @@ impl DefaultPolicy { for &literal in &literals { let package = pool.literal_to_package(literal); - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + if let Some(alias_pkg) = package.as_alias() { if alias_pkg.is_root_package_alias() { has_local_alias = true; break; @@ -159,7 +155,7 @@ impl DefaultPolicy { let mut selected = vec![]; for &literal in &literals { let package = pool.literal_to_package(literal); - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + if let Some(alias_pkg) = package.as_alias() { if alias_pkg.is_root_package_alias() { selected.push(literal); } @@ -168,9 +164,9 @@ impl DefaultPolicy { selected } - pub(crate) fn replaces(&self, source: &dyn BasePackage, target: &dyn BasePackage) -> bool { + pub(crate) fn replaces(&self, source: &BasePackageHandle, target: &BasePackageHandle) -> bool { for link in source.get_replaces().values() { - if link.get_target() == target.get_name() { + if link.get_target() == target.get_name().as_str() { return true; } } @@ -181,8 +177,8 @@ impl DefaultPolicy { impl PolicyInterface for DefaultPolicy { fn version_compare( &self, - a: &dyn PackageInterface, - b: &dyn PackageInterface, + a: &BasePackageHandle, + b: &BasePackageHandle, operator: &str, ) -> bool { if self.prefer_stable { @@ -267,8 +263,8 @@ impl PolicyInterface for DefaultPolicy { } let result = self.compare_by_priority( pool, - pool.literal_to_package(a), - pool.literal_to_package(b), + &pool.literal_to_package(a), + &pool.literal_to_package(b), required_package.clone(), true, ); @@ -300,8 +296,8 @@ impl PolicyInterface for DefaultPolicy { } let result = self.compare_by_priority( pool, - pool.literal_to_package(a), - pool.literal_to_package(b), + &pool.literal_to_package(a), + &pool.literal_to_package(b), required_package.clone(), false, ); diff --git a/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs b/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs index b4aa4a4..7b60522 100644 --- a/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs +++ b/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs @@ -14,9 +14,9 @@ impl LocalRepoTransaction { locked_repository: &dyn RepositoryInterface, local_repository: &dyn InstalledRepositoryInterface, ) -> Self { - // TODO(phase-b): RepositoryInterface::get_packages returns Box - // but Transaction::new wants Box. Upcast each via PackageInterface - // trait once a `into_package_interface` helper is added. + // TODO(phase-c): RepositoryInterface::get_packages yields BasePackageHandle; widen each to + // PackageInterfaceHandle (via .into()) and feed them to Transaction::new once the repository + // getters expose handles here. let _ = (locked_repository, local_repository); Self { inner: Transaction::new(Vec::new(), Vec::new()), diff --git a/crates/shirabe/src/dependency_resolver/lock_transaction.rs b/crates/shirabe/src/dependency_resolver/lock_transaction.rs index 14900d1..09d4571 100644 --- a/crates/shirabe/src/dependency_resolver/lock_transaction.rs +++ b/crates/shirabe/src/dependency_resolver/lock_transaction.rs @@ -1,34 +1,30 @@ //! ref: composer/src/Composer/DependencyResolver/LockTransaction.php -use std::any::Any; - use indexmap::IndexMap; use shirabe_external_packages::composer::pcre::Preg; use crate::dependency_resolver::Decisions; use crate::dependency_resolver::Pool; use crate::dependency_resolver::Transaction; -use crate::package::AliasPackage; -use crate::package::Package; -use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; #[derive(Debug)] pub struct LockTransaction { inner: Transaction, /// packages in current lock file, platform repo or otherwise present /// Indexed by spl_object_hash - pub(crate) present_map: IndexMap>, + pub(crate) present_map: IndexMap, /// Packages which cannot be mapped, platform repo, root package, other fixed repos /// Indexed by package id - pub(crate) unlockable_map: IndexMap>, - pub(crate) result_packages: IndexMap>>, + pub(crate) unlockable_map: IndexMap, + pub(crate) result_packages: IndexMap>, } impl LockTransaction { pub fn new( pool: &Pool, - present_map: IndexMap>, - unlockable_map: IndexMap>, + present_map: IndexMap, + unlockable_map: IndexMap, decisions: &Decisions, ) -> Self { let mut this = Self { @@ -38,22 +34,18 @@ impl LockTransaction { result_packages: IndexMap::new(), }; this.set_result_packages(pool, decisions); - let all: Vec> = this + let all: Vec = this .result_packages .get("all") - .map(|v| v.iter().map(|p| p.clone_package_box()).collect()) + .map(|v| v.iter().cloned().collect()) .unwrap_or_default(); - let present: Vec> = this - .present_map - .values() - .map(|p| p.clone_package_box()) - .collect(); + let present: Vec = this.present_map.values().cloned().collect(); this.inner = Transaction::new(present, all); this } pub fn set_result_packages(&mut self, pool: &Pool, decisions: &Decisions) { - let mut result_packages: IndexMap>> = IndexMap::new(); + let mut result_packages: IndexMap> = IndexMap::new(); result_packages.insert("all".to_string(), vec![]); result_packages.insert("non-dev".to_string(), vec![]); result_packages.insert("dev".to_string(), vec![]); @@ -67,12 +59,12 @@ impl LockTransaction { result_packages .get_mut("all") .unwrap() - .push(package.clone_box()); + .push(package.clone().into()); if !self.unlockable_map.contains_key(&package.get_id()) { result_packages .get_mut("non-dev") .unwrap() - .push(package.clone_box()); + .push(package.clone().into()); } } } @@ -110,7 +102,7 @@ impl LockTransaction { &self, dev_mode: bool, update_mirrors: bool, - ) -> Vec> { + ) -> Vec { let key = if dev_mode { "dev" } else { "non-dev" }; let mut packages = vec![]; @@ -120,26 +112,22 @@ impl LockTransaction { .map(|v| v.as_slice()) .unwrap_or_default(); for package in source { - if package.as_any().downcast_ref::().is_some() { + if package.as_alias().is_some() { continue; } - 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()); + if update_mirrors && !self.present_map.contains_key(&package.ptr_id().to_string()) { + let updated = self.update_mirror_and_urls(package); packages.push(updated); } else { - packages.push(package.clone_package_box()); + packages.push(package.clone()); } } packages } - fn update_mirror_and_urls(&self, package: &dyn PackageInterface) -> Box { + fn update_mirror_and_urls(&self, package: &PackageInterfaceHandle) -> PackageInterfaceHandle { for present_package in self.present_map.values() { if package.get_name() != present_package.get_name() { continue; @@ -157,38 +145,38 @@ impl LockTransaction { continue; } - if let Some(concrete_pkg) = present_package.as_any().downcast_ref::() { - // TODO(phase-b): set_source_url/set_source_mirrors expect &mut and owned types; - // present_package is &Box (immutable). Revisit ownership. + if let Some(concrete_pkg) = present_package.as_package() { + // TODO(phase-c): mirror the source url/mirrors of the present package onto it via + // its handle setters once the per-field copy semantics are reviewed. let _ = concrete_pkg; - let _ = package.get_source_url().map(|s| s.to_string()); + let _ = package.get_source_url(); let _ = package.get_source_mirrors(); } if present_package.get_dist_type() != package.get_dist_type() { - return present_package.clone_package_box(); + return present_package.clone(); } if package.get_dist_url().is_some() && present_package.get_dist_reference().is_some() - && Preg::is_match(r"{^https?://(?:(?:www\.)?bitbucket\.org|(api\.)?github\.com|(?:www\.)?gitlab\.com)/}i", package.get_dist_url().unwrap()).unwrap_or(false) + && Preg::is_match(r"{^https?://(?:(?:www\.)?bitbucket\.org|(api\.)?github\.com|(?:www\.)?gitlab\.com)/}i", &package.get_dist_url().unwrap()).unwrap_or(false) { let new_dist_url = Preg::replace( r"{(?<=/|sha=)[a-f0-9]{40}(?=/|$)}i", - present_package.get_dist_reference().unwrap(), - package.get_dist_url().unwrap(), + &present_package.get_dist_reference().unwrap(), + &package.get_dist_url().unwrap(), ) - .unwrap_or_else(|_| package.get_dist_url().unwrap().to_string()); - // TODO(phase-b): set_dist_url requires &mut PackageInterface; revisit ownership. + .unwrap_or_else(|_| package.get_dist_url().unwrap()); + // TODO(phase-c): apply new_dist_url onto present_package via its handle setter. let _ = new_dist_url; } - // TODO(phase-b): set_dist_mirrors requires &mut PackageInterface; revisit ownership. + // TODO(phase-c): apply dist mirrors onto present_package via its handle setter. let _ = package.get_dist_mirrors(); - return present_package.clone_package_box(); + return present_package.clone(); } - package.clone_package_box() + package.clone() } pub fn get_aliases( @@ -200,11 +188,11 @@ impl LockTransaction { if let Some(all_packages) = self.result_packages.get("all") { for package in all_packages { - if package.as_any().downcast_ref::().is_some() { + if package.as_alias().is_some() { let mut i = 0; while i < remaining_aliases.len() { if remaining_aliases[i].get("package").map(|s| s.as_str()) - == Some(package.get_name()) + == Some(package.get_name().as_str()) { used_aliases.push(remaining_aliases.remove(i)); } else { diff --git a/crates/shirabe/src/dependency_resolver/operation/install_operation.rs b/crates/shirabe/src/dependency_resolver/operation/install_operation.rs index ee5e84d..466c877 100644 --- a/crates/shirabe/src/dependency_resolver/operation/install_operation.rs +++ b/crates/shirabe/src/dependency_resolver/operation/install_operation.rs @@ -3,19 +3,20 @@ use crate::dependency_resolver::operation::OperationInterface; use crate::dependency_resolver::operation::SolverOperation; use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; #[derive(Debug)] pub struct InstallOperation { - pub(crate) package: Box, + pub(crate) package: PackageInterfaceHandle, } impl InstallOperation { - pub fn new(package: Box) -> Self { + pub fn new(package: PackageInterfaceHandle) -> Self { Self { package } } - pub fn get_package(&self) -> &dyn PackageInterface { - self.package.as_ref() + pub fn get_package(&self) -> &PackageInterfaceHandle { + &self.package } pub fn format(package: &dyn PackageInterface, lock: bool) -> String { @@ -43,7 +44,7 @@ impl OperationInterface for InstallOperation { } fn show(&self, lock: bool) -> String { - Self::format(self.package.as_ref(), lock) + Self::format(self.package.as_rc().borrow().as_package_interface(), lock) } fn to_string(&self) -> String { diff --git a/crates/shirabe/src/dependency_resolver/operation/mark_alias_installed_operation.rs b/crates/shirabe/src/dependency_resolver/operation/mark_alias_installed_operation.rs index e176b7f..339f86c 100644 --- a/crates/shirabe/src/dependency_resolver/operation/mark_alias_installed_operation.rs +++ b/crates/shirabe/src/dependency_resolver/operation/mark_alias_installed_operation.rs @@ -42,12 +42,10 @@ impl OperationInterface for MarkAliasInstalledOperation { true, ::DISPLAY_SOURCE_REF_IF_DEV, ), - PackageInterface::get_pretty_name(self.package.get_alias_of()), - PackageInterface::get_full_pretty_version( - self.package.get_alias_of(), - true, - ::DISPLAY_SOURCE_REF_IF_DEV, - ), + self.package.get_alias_of().get_pretty_name(), + self.package + .get_alias_of() + .get_full_pretty_version(true, ::DISPLAY_SOURCE_REF_IF_DEV,), ) } diff --git a/crates/shirabe/src/dependency_resolver/operation/mark_alias_uninstalled_operation.rs b/crates/shirabe/src/dependency_resolver/operation/mark_alias_uninstalled_operation.rs index 141bf4a..a5b7b7d 100644 --- a/crates/shirabe/src/dependency_resolver/operation/mark_alias_uninstalled_operation.rs +++ b/crates/shirabe/src/dependency_resolver/operation/mark_alias_uninstalled_operation.rs @@ -42,12 +42,10 @@ impl OperationInterface for MarkAliasUninstalledOperation { true, ::DISPLAY_SOURCE_REF_IF_DEV, ), - PackageInterface::get_pretty_name(self.package.get_alias_of()), - PackageInterface::get_full_pretty_version( - self.package.get_alias_of(), - true, - ::DISPLAY_SOURCE_REF_IF_DEV, - ), + self.package.get_alias_of().get_pretty_name(), + self.package + .get_alias_of() + .get_full_pretty_version(true, ::DISPLAY_SOURCE_REF_IF_DEV,), ) } diff --git a/crates/shirabe/src/dependency_resolver/operation/uninstall_operation.rs b/crates/shirabe/src/dependency_resolver/operation/uninstall_operation.rs index 5e1f6bc..2757146 100644 --- a/crates/shirabe/src/dependency_resolver/operation/uninstall_operation.rs +++ b/crates/shirabe/src/dependency_resolver/operation/uninstall_operation.rs @@ -3,19 +3,20 @@ use crate::dependency_resolver::operation::OperationInterface; use crate::dependency_resolver::operation::SolverOperation; use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; #[derive(Debug)] pub struct UninstallOperation { - pub(crate) package: Box, + pub(crate) package: PackageInterfaceHandle, } impl UninstallOperation { - pub fn new(package: Box) -> Self { + pub fn new(package: PackageInterfaceHandle) -> Self { Self { package } } - pub fn get_package(&self) -> &dyn PackageInterface { - self.package.as_ref() + pub fn get_package(&self) -> &PackageInterfaceHandle { + &self.package } pub fn format(package: &dyn PackageInterface, _lock: bool) -> String { @@ -42,7 +43,7 @@ impl OperationInterface for UninstallOperation { } fn show(&self, lock: bool) -> String { - Self::format(self.package.as_ref(), lock) + Self::format(self.package.as_rc().borrow().as_package_interface(), lock) } fn to_string(&self) -> String { diff --git a/crates/shirabe/src/dependency_resolver/operation/update_operation.rs b/crates/shirabe/src/dependency_resolver/operation/update_operation.rs index 2ce103b..6881782 100644 --- a/crates/shirabe/src/dependency_resolver/operation/update_operation.rs +++ b/crates/shirabe/src/dependency_resolver/operation/update_operation.rs @@ -3,28 +3,29 @@ use crate::dependency_resolver::operation::OperationInterface; use crate::dependency_resolver::operation::SolverOperation; use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; use crate::package::version::VersionParser; #[derive(Debug)] pub struct UpdateOperation { - pub(crate) initial_package: Box, - pub(crate) target_package: Box, + pub(crate) initial_package: PackageInterfaceHandle, + pub(crate) target_package: PackageInterfaceHandle, } impl UpdateOperation { - pub fn new(initial: Box, target: Box) -> Self { + pub fn new(initial: PackageInterfaceHandle, target: PackageInterfaceHandle) -> Self { Self { initial_package: initial, target_package: target, } } - pub fn get_initial_package(&self) -> &dyn PackageInterface { - self.initial_package.as_ref() + pub fn get_initial_package(&self) -> &PackageInterfaceHandle { + &self.initial_package } - pub fn get_target_package(&self) -> &dyn PackageInterface { - self.target_package.as_ref() + pub fn get_target_package(&self) -> &PackageInterfaceHandle { + &self.target_package } pub fn format( @@ -89,8 +90,8 @@ impl OperationInterface for UpdateOperation { fn show(&self, lock: bool) -> String { Self::format( - self.initial_package.as_ref(), - self.target_package.as_ref(), + self.initial_package.as_rc().borrow().as_package_interface(), + self.target_package.as_rc().borrow().as_package_interface(), lock, ) } diff --git a/crates/shirabe/src/dependency_resolver/policy_interface.rs b/crates/shirabe/src/dependency_resolver/policy_interface.rs index 148e21f..002bfe9 100644 --- a/crates/shirabe/src/dependency_resolver/policy_interface.rs +++ b/crates/shirabe/src/dependency_resolver/policy_interface.rs @@ -1,15 +1,11 @@ //! ref: composer/src/Composer/DependencyResolver/PolicyInterface.php use crate::dependency_resolver::Pool; -use crate::package::PackageInterface; +use crate::package::BasePackageHandle; pub trait PolicyInterface: std::fmt::Debug { - fn version_compare( - &self, - a: &dyn PackageInterface, - b: &dyn PackageInterface, - operator: &str, - ) -> bool; + fn version_compare(&self, a: &BasePackageHandle, b: &BasePackageHandle, operator: &str) + -> bool; fn select_preferred_packages( &self, diff --git a/crates/shirabe/src/dependency_resolver/pool.rs b/crates/shirabe/src/dependency_resolver/pool.rs index 771f363..388f23f 100644 --- a/crates/shirabe/src/dependency_resolver/pool.rs +++ b/crates/shirabe/src/dependency_resolver/pool.rs @@ -3,28 +3,29 @@ use std::fmt; use indexmap::IndexMap; -use shirabe_php_shim::{Countable, STR_PAD_LEFT, abs, spl_object_hash, str_pad}; +use shirabe_php_shim::{Countable, STR_PAD_LEFT, abs, str_pad}; use shirabe_semver::compiling_matcher::CompilingMatcher; use shirabe_semver::constraint::AnyConstraint; use shirabe_semver::constraint::SimpleConstraint; use crate::advisory::PartialSecurityAdvisory; use crate::package::BasePackage; +use crate::package::BasePackageHandle; use crate::package::version::VersionParser; /// A package pool contains all packages for dependency resolution #[derive(Debug)] pub struct Pool { /// @var BasePackage[] - pub(crate) packages: Vec>, + pub(crate) packages: Vec, /// @var array - pub(crate) package_by_name: IndexMap>>, + pub(crate) package_by_name: IndexMap>, /// @var VersionParser pub(crate) version_parser: VersionParser, /// @var array> - pub(crate) provider_cache: IndexMap>>>, + pub(crate) provider_cache: IndexMap>>, /// @var BasePackage[] - pub(crate) unacceptable_fixed_or_locked_packages: Vec>, + pub(crate) unacceptable_fixed_or_locked_packages: Vec, /// @var array> Map of package name => normalized version => pretty version pub(crate) removed_versions: IndexMap>, /// @var array> Map of package object hash => removed normalized versions => removed pretty version @@ -44,8 +45,8 @@ impl Pool { /// @param array>> $securityRemovedVersions /// @param array> $abandonedRemovedVersions pub fn new( - packages: Vec>, - unacceptable_fixed_or_locked_packages: Vec>, + packages: Vec, + unacceptable_fixed_or_locked_packages: Vec, removed_versions: IndexMap>, removed_versions_by_package: IndexMap>, security_removed_versions: IndexMap>>, @@ -197,18 +198,18 @@ impl Pool { } /// @param BasePackage[] $packages - fn set_packages(&mut self, packages: Vec>) { + fn set_packages(&mut self, packages: Vec) { let mut id: i64 = 1; - for mut package in packages { - *package.id_mut() = id; + for package in packages { + package.set_id(id); id += 1; for provided in package.get_names(true) { self.package_by_name .entry(provided) .or_insert_with(Vec::new) - .push(package.clone_box()); + .push(package.clone()); } self.packages.push(package); @@ -216,13 +217,13 @@ impl Pool { } /// @return BasePackage[] - pub fn get_packages(&self) -> &Vec> { + pub fn get_packages(&self) -> &Vec { &self.packages } /// 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].as_ref() + pub fn package_by_id(&self, id: i64) -> BasePackageHandle { + self.packages[(id - 1) as usize].clone() } /// Searches all packages providing the given package name and match the constraint @@ -235,7 +236,7 @@ impl Pool { &mut self, name: &str, constraint: Option<&AnyConstraint>, - ) -> Vec> { + ) -> Vec { // PHP: $key = (string) $constraint; let key = match constraint { Some(c) => c.to_string(), @@ -243,7 +244,7 @@ impl Pool { }; if let Some(by_key) = self.provider_cache.get(name) { if let Some(cached) = by_key.get(&key) { - return cached.iter().map(|p| p.clone_box()).collect(); + return cached.clone(); } } @@ -251,7 +252,7 @@ impl Pool { self.provider_cache .entry(name.to_string()) .or_insert_with(IndexMap::new) - .insert(key, computed.iter().map(|p| p.clone_box()).collect()); + .insert(key, computed.clone()); computed } @@ -263,23 +264,23 @@ impl Pool { &self, name: &str, constraint: Option<&AnyConstraint>, - ) -> Vec> { + ) -> Vec { let Some(candidates) = self.package_by_name.get(name) else { return vec![]; }; - let mut matches: Vec> = vec![]; + let mut matches: Vec = vec![]; for candidate in candidates { - if self.r#match(candidate.as_ref(), name, constraint) { - matches.push(candidate.clone_box()); + if self.r#match(candidate, name, constraint) { + matches.push(candidate.clone()); } } matches } - pub fn literal_to_package(&self, literal: i64) -> &dyn BasePackage { + pub fn literal_to_package(&self, literal: i64) -> BasePackageHandle { let package_id = abs(literal); self.package_by_id(package_id) @@ -289,7 +290,7 @@ impl Pool { pub fn literal_to_pretty_string( &self, literal: i64, - installed_map: &IndexMap>, + installed_map: &IndexMap, ) -> String { let package = self.literal_to_package(literal); @@ -312,7 +313,7 @@ impl Pool { /// @param string $name Name of the package to be matched pub fn r#match( &self, - candidate: &dyn BasePackage, + candidate: &BasePackageHandle, name: &str, constraint: Option<&AnyConstraint>, ) -> bool { @@ -370,17 +371,16 @@ impl Pool { false } - pub fn is_unacceptable_fixed_or_locked_package(&self, package: &dyn BasePackage) -> bool { + pub fn is_unacceptable_fixed_or_locked_package(&self, package: &BasePackageHandle) -> bool { // PHP: \in_array($package, $this->unacceptableFixedOrLockedPackages, true) // strict comparison checks reference identity for objects - let target_hash = spl_object_hash(package); self.unacceptable_fixed_or_locked_packages .iter() - .any(|p| spl_object_hash(p.as_ref()) == target_hash) + .any(|p| p.ptr_eq(package)) } /// @return BasePackage[] - pub fn get_unacceptable_fixed_or_locked_packages(&self) -> &Vec> { + pub fn get_unacceptable_fixed_or_locked_packages(&self) -> &Vec { &self.unacceptable_fixed_or_locked_packages } } diff --git a/crates/shirabe/src/dependency_resolver/pool_builder.rs b/crates/shirabe/src/dependency_resolver/pool_builder.rs index f80ef38..38a083e 100644 --- a/crates/shirabe/src/dependency_resolver/pool_builder.rs +++ b/crates/shirabe/src/dependency_resolver/pool_builder.rs @@ -8,8 +8,8 @@ use shirabe_external_packages::composer::semver::CompilingMatcher; use shirabe_external_packages::composer::semver::Intervals; use shirabe_php_shim::{ 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, + array_search, array_search_mixed, count, in_array, microtime, number_format, round, sprintf, + strpos, }; use shirabe_semver::constraint::AnyConstraint; use shirabe_semver::constraint::MatchAllConstraint; @@ -23,10 +23,12 @@ use crate::dependency_resolver::SecurityAdvisoryPoolFilter; use crate::event_dispatcher::EventDispatcher; use crate::io::IOInterface; use crate::package::AliasPackage; +use crate::package::BasePackageHandle; use crate::package::CompleteAliasPackage; use crate::package::CompletePackage; use crate::package::PackageInterface; -use crate::package::base_package::{self, BasePackage}; +use crate::package::PackageInterfaceHandle; +use crate::package::base_package; use crate::package::version::StabilityFilter; use crate::plugin::PluginEvents; use crate::plugin::PrePoolCreateEvent; @@ -48,11 +50,11 @@ pub struct PoolBuilder { alias_map: IndexMap>, packages_to_load: IndexMap, loaded_packages: IndexMap, - loaded_per_repo: IndexMap>>>, - packages: IndexMap>, - unacceptable_fixed_or_locked_packages: Vec>, + loaded_per_repo: IndexMap>>, + packages: IndexMap, + unacceptable_fixed_or_locked_packages: Vec, update_allow_list: Vec, - skipped_load: IndexMap>>, + skipped_load: IndexMap>, ignored_types: Vec, allowed_types: Option>, /// If provided, only these package names are loaded @@ -161,10 +163,7 @@ impl PoolBuilder { for locked_package in CanonicalPackagesTrait::get_packages(request.get_locked_repository().unwrap()) { - if !self.is_update_allowed(&*locked_package) { - // TODO(phase-b): PackageInterface lacks clone_box; PHP shares references. - // skipped_load population needs shared-ownership Rc. - + if !self.is_update_allowed(locked_package.as_rc().borrow().as_package_interface()) { // Path repo packages are never loaded from lock, to force them to always remain in sync // unless symlinking is disabled in which case we probably should rather treat them like // regular packages. We mark them specially so they can be reloaded fully including update propagation @@ -182,10 +181,7 @@ impl PoolBuilder { } } - // TODO(phase-b): lock_package wants Box; locked_package is a - // PackageInterface trait object from CanonicalPackagesTrait::get_packages. The - // PHP code passes the same object; needs Rc migration. - request.lock_package(todo!("convert PackageInterface → Box")); + request.lock_package(locked_package.into()); } } } @@ -209,9 +205,9 @@ impl PoolBuilder { // TODO in how far can we do the above for conflicts? It's more tricky cause conflicts can be limited to // specific versions while replace is a conflict with all versions of the name - let in_root_or_platform = package - .get_repository() - .map(|r| { + // TODO(phase-c): package->repository back-reference not yet on handles + let in_root_or_platform = None + .map(|r: &dyn RepositoryInterface| { r.as_any().is::() || r.as_any().is::() }) @@ -221,13 +217,12 @@ impl PoolBuilder { &self.acceptable_stabilities, &self.stability_flags, &package.get_names(true), - package.get_stability(), + &package.get_stability(), ) { - self.load_package(request, &repositories, &*package, false)?; + self.load_package(request, &repositories, &package, false)?; } else { - self.unacceptable_fixed_or_locked_packages - .push(package.clone_box()); + self.unacceptable_fixed_or_locked_packages.push(package); } } @@ -261,11 +256,11 @@ impl PoolBuilder { let indices: Vec = self.packages.keys().cloned().collect(); for i in indices { let package = match self.packages.get(&i) { - Some(p) => p.clone_box(), + Some(p) => p.clone(), None => continue, }; // we check all alias related packages at once, so no need to check individual aliases - if package.as_alias_package().is_some() { + if package.as_alias().is_some() { continue; } @@ -275,12 +270,11 @@ impl PoolBuilder { None => continue, }; - // TODO(phase-b): package_and_aliases originally held Box; - // AliasPackage is a PHP class so we collect (index, version) tuples instead of - // cloning the alias objects. + // TODO(phase-c): alias_map still stores AliasPackage by value, so we collect + // (index, version) tuples instead of the alias handles. 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)) { + if let Some(aliases) = self.alias_map.get(&package.ptr_id().to_string()) { for (idx, alias) in aliases { package_and_aliases.push((*idx, alias.get_version().to_string())); } @@ -314,10 +308,10 @@ impl PoolBuilder { self.stability_flags.clone(), self.root_aliases.clone(), self.root_references.clone(), - self.packages.values().map(|p| p.clone_box()).collect(), + self.packages.values().cloned().collect(), self.unacceptable_fixed_or_locked_packages .iter() - .map(|p| p.clone_box()) + .cloned() .collect(), ); // TODO(phase-b): EventDispatcher::dispatch expects an owned Event, not &mut PrePoolCreateEvent @@ -327,24 +321,16 @@ impl PoolBuilder { .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() - .iter() - .enumerate() - .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(); + // TODO(plugin)/TODO(phase-c): rebind self.packages from the (handle-based) event packages + // once EventDispatcher::dispatch returns the mutated event. + let _ = &pre_pool_create_event; } let mut pool = Pool::new( - self.packages.values().map(|p| p.clone_box()).collect(), + self.packages.values().cloned().collect(), self.unacceptable_fixed_or_locked_packages .iter() - .map(|p| p.clone_box()) + .cloned() .collect(), IndexMap::new(), IndexMap::new(), @@ -543,7 +529,7 @@ impl PoolBuilder { k.clone(), inner .iter() - .map(|(kk, vv)| (kk.clone(), vv.clone_package_box())) + .map(|(kk, vv)| (kk.clone(), vv.clone())) .collect(), ) }) @@ -561,8 +547,6 @@ impl PoolBuilder { } let packages_in_result = result.packages; for (_, package) in &packages_in_result { - // TODO(phase-b): proper upcast Box → Box; - // 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(); @@ -592,8 +576,8 @@ impl PoolBuilder { continue; } 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)?; + let propagate = !self.path_repo_unlocked.contains_key(&package.get_name()); + self.load_package(request, repositories, package, propagate)?; } } @@ -627,36 +611,33 @@ impl PoolBuilder { &mut self, request: &mut Request, repositories: &Vec>, - package: &dyn BasePackage, + package: &BasePackageHandle, propagate_update: bool, ) -> anyhow::Result<()> { let index = self.index_counter; self.index_counter += 1; - self.packages.insert(index, package.clone_box()); + self.packages.insert(index, package.clone()); - if let Some(alias) = package.as_alias_package() { + if let Some(alias) = package.as_alias() { // TODO(phase-b): alias_map should hold shared references (Rc); AliasPackage // is a PHP class and must not be cloned. - let _ = alias; + let _ = &alias; self.alias_map - .entry(spl_object_hash(alias.get_alias_of())) + .entry(alias.get_alias_of().ptr_id().to_string()) .or_insert_with(IndexMap::new) .insert(index, todo!("share AliasPackage via Rc")); } - let name = PackageInterface::get_name(package).to_string(); + let name = package.get_name(); // we're simply setting the root references on all versions for a name here and rely on the solver to pick the // right version. It'd be more work to figure out which versions and which aliases of those versions this may // apply to 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) { - // 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>. - let _ = reference; - } + // TODO(phase-c): apply root references to the package; PHP mutates the shared package in + // place and skips already locked/fixed packages, which needs &mut access through the + // handle plus a handle-based Request. + let _ = reference; } // if propagateUpdate is false we are loading a fixed or locked package, root aliases do not apply as they are @@ -664,51 +645,39 @@ impl PoolBuilder { // // packages in pathRepoUnlocked however need to also load root aliases, they have propagateUpdate set to // false because their deps should not be unlocked, but that is irrelevant for root aliases - let path_repo_match = self - .path_repo_unlocked - .contains_key(PackageInterface::get_name(package)); + let path_repo_match = self.path_repo_unlocked.contains_key(&package.get_name()); let alias_for_version = self .root_aliases .get(&name) - .and_then(|m| m.get(package.get_version())) + .and_then(|m| m.get(&package.get_version())) .cloned(); if (propagate_update || path_repo_match) && alias_for_version.is_some() { let alias = alias_for_version.unwrap(); - let base_package: Box = if let Some(ap) = package.as_alias_package() { - ap.get_alias_of().clone_box() + let base_package: BasePackageHandle = if let Some(ap) = package.as_alias() { + ap.get_alias_of().into() } else { - package.clone_box() + package.clone() + }; + let _ = (&base_package, &alias); + let alias_package: BasePackageHandle = if base_package.as_complete_package().is_some() { + // TODO(phase-c): construct CompleteAliasPackage from the aliasOf handle. + todo!("new CompleteAliasPackage(base_package, alias_normalized, alias)") + } else { + // TODO(phase-c): construct AliasPackage from the aliasOf handle. + todo!("new AliasPackage(base_package, alias_normalized, alias)") }; - let alias_package: Box = - if base_package.as_any().is::() { - // 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 migration + BasePackage impl. - let _ = CompleteAliasPackage::new( - todo!("downcast Box 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(), - alias.get("alias_normalized").cloned().unwrap_or_default(), - alias.get("alias").cloned().unwrap_or_default(), - )) - }; // PHP: $aliasPackage->setRootPackageAlias(true); // BasePackage doesn't expose this directly; the AliasPackage trait method handles it. let new_index = self.index_counter; self.index_counter += 1; - self.packages.insert(new_index, alias_package.clone_box()); - if let Some(ap) = alias_package.as_alias_package() { + self.packages.insert(new_index, alias_package.clone()); + if let Some(ap) = alias_package.as_alias() { // TODO(phase-b): alias_map should hold shared references (Rc); AliasPackage // is a PHP class and must not be cloned. - let _ = ap; + let _ = ≈ self.alias_map - .entry(spl_object_hash(ap.get_alias_of())) + .entry(ap.get_alias_of().ptr_id().to_string()) .or_insert_with(IndexMap::new) .insert(new_index, todo!("share AliasPackage via Rc")); } @@ -800,7 +769,7 @@ impl PoolBuilder { if root_requires.contains_key(name) { let name_owned = name.to_string(); return array_map( - |package: &Box| -> String { + |package: &PackageInterfaceHandle| -> String { if name_owned != package.get_name() { format!("{} (via replace of {})", package.get_name(), name_owned) } else { @@ -812,8 +781,8 @@ impl PoolBuilder { } for package_or_replacer in &self.skipped_load[name] { - if root_requires.contains_key(package_or_replacer.get_name()) { - matches.push(package_or_replacer.get_name().to_string()); + if root_requires.contains_key(&package_or_replacer.get_name()) { + matches.push(package_or_replacer.get_name()); } for (_k, link) in &package_or_replacer.get_replaces() { if root_requires.contains_key(link.get_target()) { @@ -863,7 +832,7 @@ impl PoolBuilder { for package in CanonicalPackagesTrait::get_packages(request.get_locked_repository().unwrap()) { - if Preg::is_match3(&pattern_regexp, package.get_name(), None).unwrap_or(false) { + if Preg::is_match3(&pattern_regexp, &package.get_name(), None).unwrap_or(false) { continue 'outer; } } @@ -905,10 +874,10 @@ impl PoolBuilder { repositories: &Vec>, name: &str, ) -> anyhow::Result<()> { - let skipped: Vec> = self + let skipped: Vec = self .skipped_load .get(name) - .map(|v| v.iter().map(|p| p.clone_package_box()).collect()) + .map(|v| v.iter().cloned().collect()) .unwrap_or_default(); for package_or_replacer in &skipped { // if we unfixed a replaced package name, we also need to unfix the replacer itself @@ -916,9 +885,9 @@ impl PoolBuilder { if package_or_replacer.get_name() != name && self .skipped_load - .contains_key(package_or_replacer.get_name()) + .contains_key(&package_or_replacer.get_name()) { - let replacer_name = package_or_replacer.get_name().to_string(); + let replacer_name = package_or_replacer.get_name(); if request.get_update_allow_transitive_root_dependencies() || (!self.is_root_require(request, name) && !self.is_root_require(request, &replacer_name)) @@ -932,8 +901,8 @@ impl PoolBuilder { &MatchAllConstraint::new(None).into(), ); } else { - let pkgs: Vec> = - self.packages.values().map(|p| p.clone_box()).collect(); + let pkgs: Vec = + self.packages.values().cloned().collect(); for loaded_package in &pkgs { let requires = loaded_package.get_requires(); if let Some(req_link) = requires.get(&replacer_name) { @@ -950,14 +919,14 @@ impl PoolBuilder { } if self.path_repo_unlocked.contains_key(name) { - let entries: Vec<(i64, Box)> = self + let entries: Vec<(i64, BasePackageHandle)> = self .packages .iter() - .filter(|(_, p)| PackageInterface::get_name(p.as_ref()) == name) - .map(|(i, p)| (*i, p.clone_box())) + .filter(|(_, p)| p.get_name() == name) + .map(|(i, p)| (*i, p.clone())) .collect(); for (index, package) in &entries { - self.remove_loaded_package(request, repositories, &**package, *index); + self.remove_loaded_package(request, repositories, package, *index); } } @@ -967,58 +936,41 @@ impl PoolBuilder { self.path_repo_unlocked.shift_remove(name); // remove locked package by this name which was already initialized - let locked_packages: Vec> = request - .get_locked_packages() - .values() - .map(|p| p.clone_box()) - .collect(); + let locked_packages: Vec = + request.get_locked_packages().values().cloned().collect(); for locked_package in &locked_packages { - if locked_package.as_alias_package().is_none() && locked_package.get_name() == name { - let pkgs: Vec> = - self.packages.values().map(|p| p.clone_box()).collect(); + if locked_package.as_alias().is_none() && locked_package.get_name() == name { + let pkgs: Vec = self.packages.values().cloned().collect(); // 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, - ) - }); + let index_opt = pkgs.iter().position(|p| p.ptr_eq(locked_package)); if let Some(index) = index_opt { - request.unlock_package(&**locked_package); - self.remove_loaded_package( - request, - repositories, - &**locked_package, - index as i64, - ); + request.unlock_package(locked_package); + 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 // satisfied their requirements // and if this package is replacing another that is required by a locked or fixed package, ensure // that we load that replaced package in case an update to this package removes the replacement - let fixed_or_locked: Vec> = request + let fixed_or_locked: Vec = request .get_fixed_or_locked_packages() .values() - .map(|p| p.clone_box()) + .cloned() .collect(); for fixed_or_locked_package in &fixed_or_locked { - if std::ptr::eq( - fixed_or_locked_package.as_ref() as *const _, - locked_package.as_ref() as *const _, - ) { + if fixed_or_locked_package.ptr_eq(locked_package) { continue; } if self .skipped_load - .contains_key(fixed_or_locked_package.get_name()) + .contains_key(&fixed_or_locked_package.get_name()) { let requires = fixed_or_locked_package.get_requires(); - if let Some(req_link) = requires.get(locked_package.get_name()) { + if let Some(req_link) = requires.get(&locked_package.get_name()) { self.mark_package_name_for_loading( request, - locked_package.get_name(), + &locked_package.get_name(), req_link.get_constraint(), ); } @@ -1054,8 +1006,7 @@ impl PoolBuilder { self.mark_package_name_for_loading(request, name, &cons); } - let pkgs: Vec> = - self.packages.values().map(|p| p.clone_box()).collect(); + let pkgs: Vec = self.packages.values().cloned().collect(); for package in &pkgs { for (_k, link) in &package.get_requires() { if name == link.get_target() { @@ -1073,35 +1024,24 @@ impl PoolBuilder { &mut self, _request: &Request, repositories: &Vec>, - package: &dyn BasePackage, + package: &BasePackageHandle, index: i64, ) { let repos_box: Vec> = repositories.iter().map(|r| r.clone_box()).collect(); - 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, - }; + let _ = &repos_box; + // TODO(phase-c): package->repository back-reference not yet on handles + let repo_index: i64 = -1; 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(package)) { - name_map.shift_remove(package.get_version()); + if let Some(name_map) = repo_map.get_mut(&package.get_name()) { + name_map.shift_remove(&package.get_version()); } } } self.packages.shift_remove(&index); - let object_hash = spl_object_hash(package); + let object_hash = package.ptr_id().to_string(); if let Some(aliases) = self.alias_map.shift_remove(&object_hash) { for (alias_index, alias_package) in &aliases { if repo_index >= 0 { diff --git a/crates/shirabe/src/dependency_resolver/pool_optimizer.rs b/crates/shirabe/src/dependency_resolver/pool_optimizer.rs index 27b3ffb..8aa8324 100644 --- a/crates/shirabe/src/dependency_resolver/pool_optimizer.rs +++ b/crates/shirabe/src/dependency_resolver/pool_optimizer.rs @@ -1,10 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/PoolOptimizer.php -use std::any::Any; - use anyhow::Result; use indexmap::IndexMap; -use shirabe_php_shim::{LogicException, PhpMixed, implode, ksort, spl_object_hash}; +use shirabe_php_shim::{LogicException, PhpMixed, implode, ksort}; use shirabe_semver::compiling_matcher::CompilingMatcher; use shirabe_semver::constraint::AnyConstraint; use shirabe_semver::constraint::MultiConstraint; @@ -14,8 +12,7 @@ use shirabe_semver::intervals::Intervals; use crate::dependency_resolver::PolicyInterface; use crate::dependency_resolver::Pool; use crate::dependency_resolver::Request; -use crate::package::AliasPackage; -use crate::package::BasePackage; +use crate::package::BasePackageHandle; use crate::package::PackageInterface; use crate::package::version::VersionParser; @@ -38,7 +35,7 @@ pub struct PoolOptimizer { packages_to_remove: IndexMap, /// @var array - aliases_per_package: IndexMap>>, + aliases_per_package: IndexMap>, /// @var array> removed_versions_by_package: IndexMap>, @@ -94,7 +91,7 @@ impl PoolOptimizer { // Mark fixed or locked packages as irremovable for (_, package) in request.get_fixed_or_locked_packages() { irremovable_package_constraint_groups - .entry(PackageInterface::get_name(package.as_ref()).to_string()) + .entry(package.get_name()) .or_insert_with(Vec::new) .push( SimpleConstraint::new( @@ -130,11 +127,11 @@ impl PoolOptimizer { // Keep track of alias packages for every package so if either the alias or aliased is kept // we keep the others as they are a unit of packages really - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + if let Some(alias_pkg) = package.as_alias() { self.aliases_per_package .entry(alias_pkg.get_alias_of().id()) .or_insert_with(Vec::new) - .push(package.clone_box()); + .push(package.clone()); } } @@ -153,31 +150,30 @@ impl PoolOptimizer { // Mark the packages as irremovable based on the constraints for package in pool.get_packages() { - if !irremovable_package_constraints - .contains_key(PackageInterface::get_name(package.as_ref())) - { + if !irremovable_package_constraints.contains_key(&package.get_name()) { continue; } let constraint = irremovable_package_constraints - .get(PackageInterface::get_name(package.as_ref())) + .get(&package.get_name()) .unwrap(); if CompilingMatcher::r#match( constraint, SimpleConstraint::OP_EQ, package.get_version().to_string(), ) { - self.mark_package_irremovable(package.as_ref()); + self.mark_package_irremovable(package); } } } - fn mark_package_irremovable(&mut self, package: &dyn BasePackage) { + fn mark_package_irremovable(&mut self, package: &BasePackageHandle) { self.irremovable_packages.insert(package.id(), true); - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + if let Some(alias_pkg) = package.as_alias() { // recursing here so aliasesPerPackage for the aliasOf can be checked // and all its aliases marked as irremovable as well - self.mark_package_irremovable(alias_pkg.get_alias_of()); + let aliased: BasePackageHandle = alias_pkg.get_alias_of().into(); + self.mark_package_irremovable(&aliased); } // PHP: foreach ($this->aliasesPerPackage[$package->id] as $aliasPackage) let alias_ids: Vec = self @@ -192,14 +188,14 @@ impl PoolOptimizer { /// @return Pool Optimized pool fn apply_removals_to_pool(&self, pool: &Pool) -> Pool { - let mut packages: Vec> = vec![]; + let mut packages: Vec = vec![]; let mut removed_versions: IndexMap> = IndexMap::new(); for package in pool.get_packages() { if !self.packages_to_remove.contains_key(&package.id()) { - packages.push(package.clone_box()); + packages.push(package.clone()); } else { removed_versions - .entry(PackageInterface::get_name(package.as_ref()).to_string()) + .entry(package.get_name()) .or_insert_with(IndexMap::new) .insert( package.get_version().to_string(), @@ -210,8 +206,7 @@ impl PoolOptimizer { Pool::new( packages, - // TODO(phase-b): clone Vec> from getter - todo!("pool.get_unacceptable_fixed_or_locked_packages().clone()"), + pool.get_unacceptable_fixed_or_locked_packages().clone(), removed_versions, self.removed_versions_by_package.clone(), // TODO(phase-b): PartialSecurityAdvisory is a PHP class (no Clone). Need shared ownership rework. @@ -227,7 +222,7 @@ impl PoolOptimizer { ) -> Result<()> { let mut identical_definitions_per_package: IndexMap< String, - IndexMap>>>, + IndexMap>>, > = IndexMap::new(); let mut package_identical_definition_lookup: IndexMap< i64, @@ -243,7 +238,7 @@ impl PoolOptimizer { self.mark_package_for_removal(package.id())?; - let dependency_hash = self.calculate_dependency_hash(package.as_ref()); + let dependency_hash = self.calculate_dependency_hash(package); for package_name in package.get_names(false) { if !self @@ -315,7 +310,7 @@ impl PoolOptimizer { .or_insert_with(IndexMap::new) .entry(dependency_hash.clone()) .or_insert_with(Vec::new) - .push(package.clone_box()); + .push(package.clone()); package_identical_definition_lookup .entry(package.id()) .or_insert_with(IndexMap::new) @@ -331,18 +326,14 @@ impl PoolOptimizer { } // PHP: foreach ($identicalDefinitionsPerPackage as $constraintGroups) - // TODO(phase-b): Box is not Clone; need restructuring to avoid borrow conflict. - let identical_clone: IndexMap< - String, - IndexMap>>>, - > = todo!("identical_definitions_per_package.clone()"); + let identical_clone = 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].as_ref(), + &packages[0], &identical_definitions_per_package, &package_identical_definition_lookup, ); @@ -362,7 +353,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, ); @@ -374,7 +365,7 @@ impl PoolOptimizer { Ok(()) } - fn calculate_dependency_hash(&self, package: &dyn BasePackage) -> String { + fn calculate_dependency_hash(&self, package: &BasePackageHandle) -> String { let mut hash = String::new(); let hash_relevant_links: Vec<(&str, Vec)> = vec![ @@ -447,10 +438,10 @@ impl PoolOptimizer { /// @param array> $packageIdenticalDefinitionLookup fn keep_package( &mut self, - package: &dyn BasePackage, + package: &BasePackageHandle, identical_definitions_per_package: &IndexMap< String, - IndexMap>>>, + IndexMap>>, >, package_identical_definition_lookup: &IndexMap< i64, @@ -464,11 +455,12 @@ impl PoolOptimizer { self.packages_to_remove.shift_remove(&package.id()); - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + if let Some(alias_pkg) = package.as_alias() { // recursing here so aliasesPerPackage for the aliasOf can be checked // and all its aliases marked to be kept as well + let aliased: BasePackageHandle = alias_pkg.get_alias_of().into(); self.keep_package( - alias_pkg.get_alias_of(), + &aliased, identical_definitions_per_package, package_identical_definition_lookup, ); @@ -484,21 +476,19 @@ impl PoolOptimizer { .and_then(|m| m.get(&package_group_pointers.dependency_hash)); if let Some(package_group) = package_group { for pkg in package_group { - let pkg = if let Some(alias_pkg) = - pkg.as_any().downcast_ref::() - { + let pkg: BasePackageHandle = if let Some(alias_pkg) = pkg.as_alias() { if alias_pkg.get_pretty_version() == VersionParser::DEFAULT_BRANCH_ALIAS { - alias_pkg.get_alias_of().clone_box() + alias_pkg.get_alias_of().into() } else { - pkg.clone_box() + pkg.clone() } } else { - pkg.clone_box() + pkg.clone() }; self.removed_versions_by_package - .entry(spl_object_hash(package)) + .entry(package.ptr_id().to_string()) .or_insert_with(IndexMap::new) .insert( pkg.get_version().to_string(), @@ -533,18 +523,17 @@ impl PoolOptimizer { .and_then(|m| m.get(&package_group_pointers.dependency_hash)); if let Some(package_group) = package_group { for pkg in package_group { - let pkg = if let Some(alias_pkg) = - pkg.as_any().downcast_ref::() + let pkg: BasePackageHandle = if let Some(alias_pkg) = pkg.as_alias() { if alias_pkg.get_pretty_version() == VersionParser::DEFAULT_BRANCH_ALIAS { - alias_pkg.get_alias_of().clone_box() + alias_pkg.get_alias_of().into() } else { - pkg.clone_box() + pkg.clone() } } else { - pkg.clone_box() + pkg.clone() }; self.removed_versions_by_package .entry(format!("alias-{}", alias_id)) @@ -569,8 +558,7 @@ impl PoolOptimizer { return; } - let mut package_index: IndexMap>> = - IndexMap::new(); + let mut package_index: IndexMap> = IndexMap::new(); for package in pool.get_packages() { let id = package.id(); @@ -580,22 +568,18 @@ impl PoolOptimizer { continue; } // Do not remove a package aliased by another package, nor aliases - if self.aliases_per_package.contains_key(&id) - || package.as_any().downcast_ref::().is_some() - { + if self.aliases_per_package.contains_key(&id) || package.as_alias().is_some() { continue; } // Do not remove locked packages - if request.is_fixed_package(package.as_ref()) - || request.is_locked_package(todo!("package as &dyn PackageInterface")) - { + if request.is_fixed_package(&package) || request.is_locked_package(&package) { continue; } package_index - .entry(PackageInterface::get_name(package.as_ref()).to_string()) + .entry(package.get_name()) .or_insert_with(IndexMap::new) - .insert(package.id(), package.clone_box()); + .insert(package.id(), package.clone()); } for (_, package) in request.get_locked_packages() { diff --git a/crates/shirabe/src/dependency_resolver/problem.rs b/crates/shirabe/src/dependency_resolver/problem.rs index 4436ec8..509a0dc 100644 --- a/crates/shirabe/src/dependency_resolver/problem.rs +++ b/crates/shirabe/src/dependency_resolver/problem.rs @@ -19,6 +19,7 @@ use crate::dependency_resolver::Request; use crate::dependency_resolver::rule::{self, Rule}; use crate::package::AliasPackage; use crate::package::BasePackage; +use crate::package::BasePackageHandle; use crate::package::CompletePackageInterface; use crate::package::Link; use crate::package::PackageInterface; @@ -67,7 +68,7 @@ impl Problem { request: &Request, pool: &mut Pool, is_verbose: bool, - installed_map: &IndexMap>, + installed_map: &IndexMap, learned_pool: &Vec>>>, ) -> anyhow::Result { // TODO doesn't this entirely defeat the purpose of the problem sections? what's the point of sections? @@ -155,7 +156,9 @@ impl Problem { // TODO(phase-b): reason_data is a Link. 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()), + rule::ReasonData::Link(link) => { + link.get_pretty_string(source.as_rc().borrow().as_package_interface()) + } _ => String::new(), }; format!("{}//{}", source.get_pretty_string(), link_pretty) @@ -205,7 +208,7 @@ impl Problem { request: &Request, pool: &mut Pool, is_verbose: bool, - installed_map: &IndexMap>, + installed_map: &IndexMap, learned_pool: &Vec>>>, ) -> String { let mut messages: Vec = Vec::new(); @@ -258,9 +261,9 @@ impl Problem { .entry(pkg_key.clone()) .or_insert_with(IndexMap::new) .insert(version_key, m2.clone()); - let source_package = rule_ref.get_source_package(pool); + let source_package = rule_ref.get_source_package(pool).unwrap(); for (version, pretty_version) in - pool.get_removed_versions_by_package(&spl_object_hash(&source_package)) + pool.get_removed_versions_by_package(&source_package.ptr_id().to_string()) { templates .get_mut(&template) @@ -551,11 +554,13 @@ impl Problem { } } - let mut locked_package: Option> = None; + let mut locked_package: Option = None; for (_key, package) in request.get_locked_packages() { - if package.get_name() == package_name { - locked_package = Some(package.clone_box()); - if pool.is_unacceptable_fixed_or_locked_package(package.as_ref()) { + if package.get_name().as_str() == package_name { + locked_package = Some(package.clone()); + // TODO(phase-c): wire Pool::is_unacceptable_fixed_or_locked_package(package) here; + // the locked package handle and the pool's identity check are now both handle-based. + if todo!("is_unacceptable_fixed_or_locked_package with a request package handle") { return ( "- ".to_string(), format!( @@ -629,7 +634,7 @@ impl Problem { if packages.len() > 0 { let root_reqs = repository_set.get_root_requires(); if root_reqs.contains_key(package_name) { - let filtered: Vec<&Box> = packages + let filtered: Vec<&BasePackageHandle> = packages .iter() .filter(|p| { root_reqs[package_name].matches( @@ -673,7 +678,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> = packages + let filtered: Vec<&BasePackageHandle> = packages .iter() .filter(|p| { temp_reqs[&name].matches( @@ -721,7 +726,7 @@ impl Problem { lp.get_version().to_string(), None, )); - let filtered: Vec<&Box> = packages + let filtered: Vec<&BasePackageHandle> = packages .iter() .filter(|p| { fixed_constraint.matches( @@ -756,14 +761,10 @@ impl Problem { } } - let non_locked_packages: Vec<&Box> = packages - .iter() - .filter(|p| { - p.get_repository() - .and_then(|r| r.as_any().downcast_ref::()) - .is_none() - }) - .collect(); + // TODO(phase-c): filtering out packages from a LockArrayRepository needs the handle's + // repository back-reference (phase-c handoff item #1), which is not yet available; keep + // all packages for now. + let non_locked_packages: Vec<&BasePackageHandle> = packages.iter().collect(); if non_locked_packages.len() == 0 { return ( @@ -961,7 +962,7 @@ impl Problem { let all_repos_packages = &packages; let top_package = all_repos_packages.first(); if let Some(tp) = top_package { - if tp.as_root_package_interface().is_some() { + if tp.as_root().is_some() { suffix = " See https://getcomposer.org/dep-on-root for details and assistance." .to_string(); } @@ -1023,7 +1024,7 @@ impl Problem { /// @internal pub fn get_package_list( - packages: &Vec>, + packages: &Vec, is_verbose: bool, pool: Option<&Pool>, constraint: Option<&AnyConstraint>, @@ -1044,7 +1045,7 @@ impl Problem { versions: IndexMap::new(), }); entry.name = package.get_pretty_name().to_string(); - let alias_suffix = if let Some(alias) = package.as_alias_package() { + let alias_suffix = if let Some(alias) = package.as_alias() { format!(" (alias of {})", alias.get_alias_of().get_pretty_version()) } else { String::new() @@ -1064,7 +1065,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.as_ref())) + .get_removed_versions_by_package(&package.ptr_id().to_string()) { entry.versions.insert(version, pretty_version); } @@ -1129,16 +1130,12 @@ impl Problem { let available = pool.what_provides(package_name, None); if available.len() > 0 { - let mut selected: Option<&Box> = None; + let mut selected: Option<&BasePackageHandle> = None; + // TODO(phase-c): the handle does not expose get_repository (a `RefCell`-borrowed + // back-reference); preferring the package from a PlatformRepository needs repository + // back-references on handles. Falling back to the first candidate for now. for pkg in &available { - if pkg - .get_repository() - .and_then(|r| r.as_any().downcast_ref::()) - .is_some() - { - selected = Some(pkg); - break; - } + let _ = pkg; } if selected.is_none() { selected = available.first(); @@ -1163,15 +1160,14 @@ impl Problem { let mut version: String = selected.get_pretty_version().to_string(); let extra = selected.get_extra(); - if selected.as_complete_package_interface().is_some() + if selected.as_complete().is_some() && extra.contains_key("config.platform") && extra["config.platform"].as_bool() == Some(true) { let description: String = selected - .as_complete_package_interface() + .as_complete() .and_then(|c| c.get_description()) - .unwrap_or("") - .to_string(); + .unwrap_or_default(); version = format!("{}; {}", version, str_replace("Package ", "", &description)); } return Some(version); @@ -1226,10 +1222,10 @@ impl Problem { filtered } - fn has_multiple_names(packages: &Vec>) -> bool { + fn has_multiple_names(packages: &Vec) -> bool { let mut name: Option = None; for package in packages { - if name.is_none() || name.as_deref() == Some(package.get_name()) { + if name.is_none() || name.as_deref() == Some(package.get_name().as_str()) { name = Some(package.get_name().to_string()); } else { return true; @@ -1243,30 +1239,22 @@ impl Problem { pool: &Pool, is_verbose: bool, package_name: &str, - higher_repo_packages: &Vec>, - all_repos_packages: &Vec>, + higher_repo_packages: &Vec, + all_repos_packages: &Vec, reason: &str, constraint: Option<&AnyConstraint>, ) -> (String, String) { - let mut next_repo_packages: Vec> = Vec::new(); - let mut next_repo: Option> = None; - - for package in all_repos_packages { - // 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; - } - } - - // assert(null !== $nextRepo); - let next_repo = next_repo.unwrap(); + // TODO(phase-c): selecting the next repository's packages relies on each package's + // repository back-reference, which the handle does not yet expose (phase-c handoff + // item #1). Both `next_repo_packages` and `next_repo` are blocked on that decision. + let _ = all_repos_packages; + let next_repo_packages: Vec = Vec::new(); + let next_repo: Box = + todo!("repository back-reference on handle pending (phase-c handoff item #1)"); if higher_repo_packages.len() > 0 { let top_package = higher_repo_packages.first().unwrap(); - if top_package.as_root_package_interface().is_some() { + if top_package.as_root().is_some() { return ( format!( "- Root composer.json requires {}{}, it is ", @@ -1309,7 +1297,7 @@ impl Problem { ) ); // symlinked path repos cannot be locked so do not suggest keeping it locked - if next_repo_packages[0].get_dist_type() == Some("path") { + if next_repo_packages[0].get_dist_type() == Some("path".to_string()) { let transport_options = next_repo_packages[0].get_transport_options(); if !transport_options.contains_key("symlink") || transport_options["symlink"].as_bool() != Some(false) @@ -1367,12 +1355,10 @@ impl Problem { constraint, false ), - higher_repo_packages - .first() - .unwrap() - .get_repository() - .map(|r| r.get_repo_name()) - .unwrap_or_default(), + // TODO(phase-c): the higher repo's name needs the handle's repository + // back-reference (phase-c handoff item #1); unreachable until `next_repo` above + // is resolved. + String::new(), reason ), ) diff --git a/crates/shirabe/src/dependency_resolver/request.rs b/crates/shirabe/src/dependency_resolver/request.rs index e2bfc3c..686373b 100644 --- a/crates/shirabe/src/dependency_resolver/request.rs +++ b/crates/shirabe/src/dependency_resolver/request.rs @@ -5,8 +5,7 @@ use shirabe_php_shim::{LogicException, spl_object_hash, strtolower}; use shirabe_semver::constraint::AnyConstraint; use shirabe_semver::constraint::MatchAllConstraint; -use crate::package::BasePackage; -use crate::package::PackageInterface; +use crate::package::BasePackageHandle; use crate::repository::CanonicalPackagesTrait; use crate::repository::LockArrayRepository; use crate::repository::RepositoryInterface; @@ -44,9 +43,9 @@ pub enum UpdateAllowTransitiveDeps { pub struct Request { pub(crate) locked_repository: Option, pub(crate) requires: IndexMap, - pub(crate) fixed_packages: IndexMap>, - pub(crate) locked_packages: IndexMap>, - pub(crate) fixed_locked_packages: IndexMap>, + pub(crate) fixed_packages: IndexMap, + pub(crate) locked_packages: IndexMap, + pub(crate) fixed_locked_packages: IndexMap, pub(crate) update_allow_list: Vec, pub(crate) update_allow_transitive_dependencies: UpdateAllowTransitiveDeps, restrict_packages: Option>, @@ -94,8 +93,8 @@ impl Request { /// This is used for platform packages which cannot be modified by Composer. A rule enforcing /// their installation is generated for dependency resolution. Partial updates with dependencies /// cannot in any way modify these packages. - pub fn fix_package(&mut self, package: Box) { - let hash = spl_object_hash(&package); + pub fn fix_package(&mut self, package: BasePackageHandle) { + let hash = package.ptr_id().to_string(); self.fixed_packages.insert(hash, package); } @@ -109,8 +108,8 @@ impl Request { /// for the solver, so if nothing requires these packages they will be removed. Additionally in /// a partial update these packages can be unlocked, meaning other versions can be installed if /// explicitly requested as part of the update. - pub fn lock_package(&mut self, package: Box) { - let hash = spl_object_hash(&package); + pub fn lock_package(&mut self, package: BasePackageHandle) { + let hash = package.ptr_id().to_string(); self.locked_packages.insert(hash, package); } @@ -120,15 +119,14 @@ impl Request { /// should not allow removal of any packages. At the same time lock packages there cannot simply /// be marked fixed, as error reporting would then report them as platform packages, so this /// still marks them as locked packages at the same time. - pub fn fix_locked_package(&mut self, package: Box) { - let hash = spl_object_hash(&package); - self.fixed_packages - .insert(hash.clone(), package.clone_box()); + pub fn fix_locked_package(&mut self, package: BasePackageHandle) { + let hash = package.ptr_id().to_string(); + self.fixed_packages.insert(hash.clone(), package.clone()); self.fixed_locked_packages.insert(hash, package); } - pub fn unlock_package(&mut self, package: &dyn BasePackage) { - self.locked_packages.remove(&spl_object_hash(package)); + pub fn unlock_package(&mut self, package: &BasePackageHandle) { + self.locked_packages.remove(&package.ptr_id().to_string()); } pub fn set_update_allow_list( @@ -159,33 +157,34 @@ impl Request { &self.requires } - pub fn get_fixed_packages(&self) -> &IndexMap> { + pub fn get_fixed_packages(&self) -> &IndexMap { &self.fixed_packages } - pub fn is_fixed_package(&self, package: &dyn BasePackage) -> bool { - self.fixed_packages.contains_key(&spl_object_hash(package)) + pub fn is_fixed_package(&self, package: &BasePackageHandle) -> bool { + self.fixed_packages + .contains_key(&package.ptr_id().to_string()) } - pub fn get_locked_packages(&self) -> &IndexMap> { + pub fn get_locked_packages(&self) -> &IndexMap { &self.locked_packages } - pub fn is_locked_package(&self, package: &dyn PackageInterface) -> bool { - let hash = spl_object_hash(package); + pub fn is_locked_package(&self, package: &BasePackageHandle) -> bool { + let hash = package.ptr_id().to_string(); self.locked_packages.contains_key(&hash) || self.fixed_locked_packages.contains_key(&hash) } - pub fn get_fixed_or_locked_packages(&self) -> IndexMap> { - let mut result: IndexMap> = self + pub fn get_fixed_or_locked_packages(&self) -> IndexMap { + let mut result: IndexMap = self .fixed_packages .iter() - .map(|(k, v)| (k.clone(), v.clone_box())) + .map(|(k, v)| (k.clone(), v.clone())) .collect(); result.extend( self.locked_packages .iter() - .map(|(k, v)| (k.clone(), v.clone_box())), + .map(|(k, v)| (k.clone(), v.clone())), ); result } @@ -194,15 +193,18 @@ impl Request { /// is for the installed map in the solver problems. /// Some locked packages may not be in the pool, /// so they have a package->id of -1 - pub fn get_present_map(&self, package_ids: bool) -> IndexMap> { - let mut present_map: IndexMap> = IndexMap::new(); + pub fn get_present_map( + &self, + package_ids: bool, + ) -> IndexMap { + let mut present_map: IndexMap = IndexMap::new(); if let Some(ref locked_repository) = self.locked_repository { for package in RepositoryInterface::get_packages(locked_repository) { let key = if package_ids { package.get_id().to_string() } else { - spl_object_hash(&package) + package.ptr_id().to_string() }; present_map.insert(key, package); } @@ -212,18 +214,18 @@ impl Request { let key = if package_ids { package.get_id().to_string() } else { - spl_object_hash(package) + package.ptr_id().to_string() }; - present_map.insert(key, package.clone_box()); + present_map.insert(key, package.clone()); } present_map } - pub fn get_fixed_packages_map(&self) -> IndexMap> { - let mut fixed_packages_map: IndexMap> = IndexMap::new(); + pub fn get_fixed_packages_map(&self) -> IndexMap { + let mut fixed_packages_map: IndexMap = IndexMap::new(); for (_, package) in &self.fixed_packages { - fixed_packages_map.insert(package.get_id(), package.clone_box()); + fixed_packages_map.insert(package.get_id(), package.clone()); } fixed_packages_map } diff --git a/crates/shirabe/src/dependency_resolver/rule.rs b/crates/shirabe/src/dependency_resolver/rule.rs index fbc8521..afb0318 100644 --- a/crates/shirabe/src/dependency_resolver/rule.rs +++ b/crates/shirabe/src/dependency_resolver/rule.rs @@ -22,6 +22,7 @@ use crate::dependency_resolver::Rule2Literals; use crate::dependency_resolver::RuleSet; use crate::package::AliasPackage; use crate::package::BasePackage; +use crate::package::BasePackageHandle; use crate::package::Link; use crate::package::PackageInterface; use crate::package::version::VersionParser; @@ -31,7 +32,7 @@ use crate::repository::RepositorySet; #[derive(Debug)] pub enum ReasonData { Link(Link), - BasePackage(Box), + BasePackage(BasePackageHandle), String(String), Int(i64), RootRequire { @@ -39,7 +40,7 @@ pub enum ReasonData { constraint: AnyConstraint, }, Fixed { - package: Box, + package: BasePackageHandle, }, /// Phase B placeholder for an arbitrary PHP-side value not yet mapped to a real variant. Mixed(PhpMixed), @@ -231,9 +232,9 @@ impl Rule { // TODO(phase-b): Request::get_locked_repository() signature let locked_repo: Option<()> = todo!("request.get_locked_repository()"); if let Some(_locked_repo) = locked_repo { - let packages: Vec> = todo!("locked_repo.get_packages()"); + let packages: Vec = todo!("locked_repo.get_packages()"); for package in packages { - let p: &dyn BasePackage = todo!("package as BasePackage reference"); + let p: &BasePackageHandle = &package; if p.get_name() == link.get_target() { if pool.is_unacceptable_fixed_or_locked_package(p) { return true; @@ -272,10 +273,10 @@ impl Rule { // TODO(phase-b): Request::get_locked_repository() signature let locked_repo: Option<()> = todo!("request.get_locked_repository()"); if let Some(_locked_repo) = locked_repo { - let packages: Vec> = todo!("locked_repo.get_packages()"); + let packages: Vec = todo!("locked_repo.get_packages()"); for package in packages { - let p: &dyn BasePackage = todo!("package as BasePackage reference"); - if p.get_name() == package_name { + let p: &BasePackageHandle = &package; + if p.get_name() == *package_name { if pool.is_unacceptable_fixed_or_locked_package(p) { return true; } @@ -300,17 +301,15 @@ impl Rule { } /// @internal - pub fn get_source_package(&self, pool: &Pool) -> Result> { + pub fn get_source_package(&self, pool: &Pool) -> Result { let literals = self.get_literals(); match self.get_reason() { r if r == RULE_PACKAGE_CONFLICT => { - let mut package1 = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[0]).clone_box(), - ); - let mut package2 = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[1]).clone_box(), - ); + let mut package1 = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[0])); + let mut package2 = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[1])); let reason_data = self.get_reason_data(); // swap literals if they are not in the right order with package2 being the conflicter @@ -325,9 +324,8 @@ impl Rule { 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(), - ); + let source_package = + self.deduplicate_default_branch_alias(pool.literal_to_package(source_literal)); Ok(source_package) } @@ -348,7 +346,7 @@ impl Rule { request: &Request, pool: &mut Pool, is_verbose: bool, - installed_map: &IndexMap>, + installed_map: &IndexMap, _learned_pool: &Vec>>>, ) -> String { let mut literals = self.get_literals(); @@ -373,10 +371,10 @@ impl Rule { ); } - let packages_non_alias: Vec> = packages + let packages_non_alias: Vec = packages .iter() - .filter(|p| p.as_any().downcast_ref::().is_none()) - .map(|p| p.clone_box()) + .filter(|p| p.as_alias().is_none()) + .map(|p| p.clone()) .collect(); if packages_non_alias.len() == 1 { let package = &packages_non_alias[0]; @@ -396,7 +394,7 @@ impl Rule { constraint.get_pretty_string(), self.format_packages_unique_from_packages( pool, - packages, + packages.iter().map(|p| p.clone()).collect(), is_verbose, Some(constraint), false @@ -406,7 +404,7 @@ impl Rule { r if r == RULE_FIXED => { let package_in = match self.get_reason_data() { - ReasonData::Fixed { package } => package.clone_box(), + ReasonData::Fixed { package } => package.clone(), _ => return String::new(), }; let package = self.deduplicate_default_branch_alias(package_in); @@ -427,12 +425,10 @@ impl Rule { } r if r == RULE_PACKAGE_CONFLICT => { - let mut package1 = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[0]).clone_box(), - ); - let mut package2 = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[1]).clone_box(), - ); + let mut package1 = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[0])); + let mut package2 = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[1])); let mut conflict_target = package1.get_pretty_string(); let reason_data = self.get_reason_data(); @@ -495,21 +491,21 @@ impl Rule { 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( - pool.literal_to_package(source_literal).clone_box(), - ); + let source_package = + self.deduplicate_default_branch_alias(pool.literal_to_package(source_literal)); let reason_data = self.get_reason_data(); let link = match reason_data { ReasonData::Link(l) => l, _ => return String::new(), }; - let mut requires: Vec> = vec![]; + let mut requires: Vec = vec![]; for literal in &literals { - requires.push(pool.literal_to_package(*literal).clone_box()); + requires.push(pool.literal_to_package(*literal)); } - let text = link.get_pretty_string(&*source_package); + let text = + link.get_pretty_string(source_package.as_rc().borrow().as_package_interface()); if requires.len() > 0 { format!( "{} -> satisfiable by {}.", @@ -573,13 +569,13 @@ impl Rule { reason_str }; - let mut installed_packages: Vec> = vec![]; - let mut removable_packages: Vec> = vec![]; + let mut installed_packages: Vec = vec![]; + let mut removable_packages: Vec = vec![]; for literal in &literals { if installed_map.contains_key(&abs(*literal).to_string()) { - installed_packages.push(pool.literal_to_package(*literal).clone_box()); + installed_packages.push(pool.literal_to_package(*literal)); } else { - removable_packages.push(pool.literal_to_package(*literal).clone_box()); + removable_packages.push(pool.literal_to_package(*literal)); } } @@ -628,7 +624,7 @@ impl Rule { let rule_text = if literals.len() == 1 { pool.literal_to_pretty_string(literals[0], &installed_map) } else { - let mut groups: IndexMap>> = IndexMap::new(); + let mut groups: IndexMap> = IndexMap::new(); for literal in &literals { let package = pool.literal_to_package(*literal); let group = if installed_map.contains_key(&package.id().to_string()) { @@ -644,7 +640,7 @@ impl Rule { groups .entry(group.to_string()) .or_insert_with(Vec::new) - .push(self.deduplicate_default_branch_alias(package.clone_box())); + .push(self.deduplicate_default_branch_alias(package.clone())); } let mut rule_texts: Vec = vec![]; for (group, packages) in &groups { @@ -654,7 +650,7 @@ impl Rule { if packages.len() > 1 { " one of" } else { "" }, self.format_packages_unique_from_packages( pool, - packages.iter().map(|p| p.clone_box()).collect(), + packages.iter().map(|p| p.clone()).collect(), is_verbose, None, false, @@ -674,9 +670,8 @@ impl Rule { if alias_package.get_version() == VersionParser::DEFAULT_BRANCH_ALIAS { return String::new(); } - let package = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[1]).clone_box(), - ); + let package = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[1])); format!( "{} is an alias of {} and thus requires it to be installed too.", @@ -692,9 +687,8 @@ impl Rule { if alias_package.get_version() == VersionParser::DEFAULT_BRANCH_ALIAS { return String::new(); } - let package = self.deduplicate_default_branch_alias( - pool.literal_to_package(literals[0]).clone_box(), - ); + let package = + self.deduplicate_default_branch_alias(pool.literal_to_package(literals[0])); format!( "{} is an alias of {} and must be installed with it.", @@ -720,7 +714,7 @@ impl Rule { fn format_packages_unique_from_packages( &self, pool: &Pool, - packages: Vec>, + packages: Vec, is_verbose: bool, constraint: Option<&AnyConstraint>, use_removed_version_group: bool, @@ -743,9 +737,9 @@ impl Rule { constraint: Option<&AnyConstraint>, use_removed_version_group: bool, ) -> String { - let mut packages: Vec> = vec![]; + let mut packages: Vec = vec![]; for literal in literals { - packages.push(pool.literal_to_package(*literal).clone_box()); + packages.push(pool.literal_to_package(*literal)); } Problem::get_package_list( &packages, @@ -756,13 +750,10 @@ impl Rule { ) } - fn deduplicate_default_branch_alias( - &self, - package: Box, - ) -> Box { - if let Some(alias_pkg) = package.as_any().downcast_ref::() { + fn deduplicate_default_branch_alias(&self, package: BasePackageHandle) -> BasePackageHandle { + if let Some(alias_pkg) = package.as_alias() { if alias_pkg.get_pretty_version() == VersionParser::DEFAULT_BRANCH_ALIAS { - return alias_pkg.get_alias_of().clone_box(); + return alias_pkg.get_alias_of().into(); } } diff --git a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs index 076ce42..69f5c4a 100644 --- a/crates/shirabe/src/dependency_resolver/rule_set_generator.rs +++ b/crates/shirabe/src/dependency_resolver/rule_set_generator.rs @@ -19,17 +19,15 @@ use crate::dependency_resolver::rule::{self, Rule}; use crate::filter::platform_requirement_filter::IgnoreListPlatformRequirementFilter; use crate::filter::platform_requirement_filter::PlatformRequirementFilterFactory; use crate::filter::platform_requirement_filter::PlatformRequirementFilterInterface; -use crate::package::AliasPackage; -use crate::package::BasePackage; -use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; #[derive(Debug)] pub struct RuleSetGenerator { pub(crate) policy: Box, pub(crate) pool: std::rc::Rc>, pub(crate) rules: RuleSet, - pub(crate) added_map: IndexMap>, - pub(crate) added_packages_by_names: IndexMap>>, + pub(crate) added_map: IndexMap, + pub(crate) added_packages_by_names: IndexMap>, } impl RuleSetGenerator { @@ -52,8 +50,8 @@ impl RuleSetGenerator { /// one requirement of the package A. fn create_require_rule( &self, - package: &dyn PackageInterface, - providers: &[Box], + package: &PackageInterfaceHandle, + providers: &[PackageInterfaceHandle], reason: i64, reason_data: PhpMixed, ) -> Option { @@ -61,10 +59,7 @@ impl RuleSetGenerator { for provider in providers { // self fulfilling rule? - if std::ptr::eq( - provider.as_ref() as *const dyn PackageInterface, - package as *const dyn PackageInterface, - ) { + if provider.ptr_eq(package) { return None; } literals.push(provider.get_id()); @@ -83,7 +78,7 @@ impl RuleSetGenerator { /// set of packages is empty an impossible rule is generated. fn create_install_one_of_rule( &self, - packages: &[Box], + packages: &[PackageInterfaceHandle], reason: i64, reason_data: PhpMixed, ) -> GenericRule { @@ -97,16 +92,13 @@ impl RuleSetGenerator { /// and B the provider. fn create_rule2_literals( &self, - issuer: &dyn PackageInterface, - provider: &dyn PackageInterface, + issuer: &PackageInterfaceHandle, + provider: &PackageInterfaceHandle, reason: i64, reason_data: PhpMixed, ) -> Option { // ignore self conflict - if std::ptr::eq( - issuer as *const dyn PackageInterface, - provider as *const dyn PackageInterface, - ) { + if issuer.ptr_eq(provider) { return None; } @@ -120,7 +112,7 @@ impl RuleSetGenerator { fn create_multi_conflict_rule( &self, - packages: &[Box], + packages: &[PackageInterfaceHandle], reason: i64, reason_data: PhpMixed, ) -> Rule { @@ -152,10 +144,10 @@ impl RuleSetGenerator { pub(crate) fn add_rules_for_package( &mut self, - package: Box, + package: PackageInterfaceHandle, platform_requirement_filter: &dyn PlatformRequirementFilterInterface, ) { - let mut work_queue: VecDeque> = VecDeque::new(); + let mut work_queue: VecDeque = VecDeque::new(); work_queue.push_back(package); while let Some(package) = work_queue.pop_front() { @@ -163,26 +155,25 @@ impl RuleSetGenerator { continue; } - self.added_map - .insert(package.get_id(), package.clone_package_box()); + self.added_map.insert(package.get_id(), package.clone()); - let is_alias = package.as_any().downcast_ref::().is_some(); + let is_alias = package.as_alias().is_some(); if !is_alias { for name in package.get_names(false) { self.added_packages_by_names .entry(name) .or_default() - .push(package.clone_package_box()); + .push(package.clone()); } } else { - let alias_pkg = package.as_any().downcast_ref::().unwrap(); + let alias_pkg = package.as_alias().unwrap(); - work_queue.push_back(alias_pkg.get_alias_of().clone_package_box()); - let alias_of = alias_pkg.get_alias_of(); + let alias_of: PackageInterfaceHandle = alias_pkg.get_alias_of().into(); + work_queue.push_back(alias_of.clone()); let rule = self.create_require_rule( - &*package, - &[alias_of.clone_package_box()], + &package, + &[alias_of.clone()], rule::RULE_PACKAGE_ALIAS, PhpMixed::Null, // reasonData: $package (BasePackage) ); @@ -190,8 +181,8 @@ impl RuleSetGenerator { // aliases must be installed with their main package, so create a rule the other way around as well let inverse_rule = self.create_require_rule( - alias_of, - &[package.clone_package_box()], + &alias_of, + &[package.clone()], rule::RULE_PACKAGE_INVERSE_ALIAS, PhpMixed::Null, // reasonData: $package->getAliasOf() (BasePackage) ); @@ -218,16 +209,16 @@ impl RuleSetGenerator { .unwrap_or(fallback); } - let possible_requires: Vec> = self + let possible_requires: Vec = self .pool .borrow_mut() .what_provides(link.get_target(), Some(&constraint)) .into_iter() - .map(|p| p.clone_package_box()) + .map(|p| p.into()) .collect(); let rule = self.create_require_rule( - &*package, + &package, &possible_requires, rule::RULE_PACKAGE_REQUIRES, PhpMixed::Null, // reasonData: $link (Link) @@ -245,11 +236,7 @@ impl RuleSetGenerator { &mut self, platform_requirement_filter: &dyn PlatformRequirementFilterInterface, ) { - let packages: Vec> = self - .added_map - .values() - .map(|p| p.clone_package_box()) - .collect(); + let packages: Vec = self.added_map.values().cloned().collect(); for package in &packages { for link in package.get_conflicts().values() { @@ -271,22 +258,24 @@ impl RuleSetGenerator { .unwrap_or(fallback); } - let conflicts = self + let conflicts: Vec = self .pool .borrow_mut() - .what_provides(link.get_target(), Some(&constraint)); + .what_provides(link.get_target(), Some(&constraint)) + .into_iter() + .map(|p| p.into()) + .collect(); for conflict in &conflicts { // define the conflict rule for regular packages, for alias packages it's only needed if the name // matches the conflict exactly, otherwise the name match is by provide/replace which means the // package which this is an alias of will conflict anyway, so no need to create additional rules - let conflict_is_alias = - conflict.as_any().downcast_ref::().is_some(); + let conflict_is_alias = conflict.as_alias().is_some(); let conflict_name_matches = conflict.get_name() == link.get_target(); if !conflict_is_alias || conflict_name_matches { let rule = self.create_rule2_literals( - &**package, - &**conflict, + package, + conflict, rule::RULE_PACKAGE_CONFLICT, PhpMixed::Null, // reasonData: $link (Link) ); @@ -296,10 +285,10 @@ impl RuleSetGenerator { } } - let names_packages: Vec<(String, Vec>)> = self + let names_packages: Vec<(String, Vec)> = self .added_packages_by_names .iter() - .map(|(k, v)| (k.clone(), v.iter().map(|p| p.clone_package_box()).collect())) + .map(|(k, v)| (k.clone(), v.iter().cloned().collect())) .collect(); for (name, packages) in names_packages { @@ -320,11 +309,9 @@ impl RuleSetGenerator { for package in request.get_fixed_packages().values() { if package.get_id() == -1 { // 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()) - { + // TODO(phase-c): wire Pool::is_unacceptable_fixed_or_locked_package(package) here; + // the package handle and the pool's identity check are now both handle-based. + if todo!("is_unacceptable_fixed_or_locked_package with a request package handle") { continue; } @@ -338,7 +325,7 @@ impl RuleSetGenerator { })); } - self.add_rules_for_package(package.clone_box(), platform_requirement_filter); + self.add_rules_for_package(package.clone().into(), platform_requirement_filter); let mut reason_data: IndexMap> = IndexMap::new(); reason_data.insert( @@ -346,7 +333,7 @@ impl RuleSetGenerator { Box::new(PhpMixed::Null), // reasonData: $package (BasePackage) ); let rule = self.create_install_one_of_rule( - &[package.clone_package_box()], + &[package.clone().into()], rule::RULE_FIXED, PhpMixed::Array(reason_data), ); @@ -367,19 +354,16 @@ impl RuleSetGenerator { .unwrap_or(fallback); } - let packages: Vec> = self + let packages: Vec = self .pool .borrow_mut() .what_provides(package_name, Some(&constraint)) .into_iter() - .map(|p| p.clone_package_box()) + .map(|p| p.into()) .collect(); if !packages.is_empty() { for package in &packages { - self.add_rules_for_package( - package.clone_package_box(), - platform_requirement_filter, - ); + self.add_rules_for_package(package.clone(), platform_requirement_filter); } let mut reason_data: IndexMap> = IndexMap::new(); @@ -407,19 +391,19 @@ impl RuleSetGenerator { &mut self, platform_requirement_filter: &dyn PlatformRequirementFilterInterface, ) { - let packages: Vec> = self + let packages: Vec = self .pool .borrow() .get_packages() .iter() - .map(|p| p.clone_box()) + .map(|p| p.clone().into()) .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 let is_not_added = !self.added_map.contains_key(&package.get_id()); - let as_alias = package.as_any().downcast_ref::(); + let as_alias = package.as_alias(); if is_not_added { if let Some(alias_pkg) = as_alias { if alias_pkg.is_root_package_alias() @@ -427,10 +411,7 @@ impl RuleSetGenerator { .added_map .contains_key(&alias_pkg.get_alias_of().get_id()) { - self.add_rules_for_package( - package.clone_package_box(), - platform_requirement_filter, - ); + self.add_rules_for_package(package.clone(), 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 4daf641..ba2cf09 100644 --- a/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs +++ b/crates/shirabe/src/dependency_resolver/security_advisory_pool_filter.rs @@ -31,12 +31,11 @@ impl SecurityAdvisoryPoolFilter { repositories: Vec>, request: &Request, ) -> Pool { - // TODO(phase-b): port the filter() body. Blockers: + // TODO(phase-c): port the filter() body. Blockers: // * RepositorySet::new takes 6 args; ConfigSourceInterface refactor pending - // * pool.get_packages() yields Box, but the audit/repo APIs - // expect Box; needs trait-object coercion / cloning story - // * Pool::new requires owned Vecs, but existing pool's getters return refs and - // Box is not Clone (only clone_box). + // * pool.get_packages() yields BasePackageHandle; widen to PackageInterfaceHandle + // (via .into()) where the audit/repo APIs expect PackageInterface. + // * Pool::new requires owned Vecs; clone the handles out of the existing pool. // * advisory map element type mismatch (PhpMixed vs PartialSecurityAdvisory). let _ = ( pool, diff --git a/crates/shirabe/src/dependency_resolver/solver.rs b/crates/shirabe/src/dependency_resolver/solver.rs index 820f792..2610f07 100644 --- a/crates/shirabe/src/dependency_resolver/solver.rs +++ b/crates/shirabe/src/dependency_resolver/solver.rs @@ -29,7 +29,7 @@ use crate::filter::platform_requirement_filter::IgnoreListPlatformRequirementFil use crate::filter::platform_requirement_filter::PlatformRequirementFilterFactory; use crate::filter::platform_requirement_filter::PlatformRequirementFilterInterface; use crate::io::IOInterface; -use crate::package::BasePackage; +use crate::package::BasePackageHandle; #[derive(Debug)] pub struct Solver { @@ -40,7 +40,7 @@ pub struct Solver { pub(crate) watch_graph: RuleWatchGraph, pub(crate) decisions: Decisions, - pub(crate) fixed_map: IndexMap>, + pub(crate) fixed_map: IndexMap, pub(crate) propagate_index: i64, /// Pairs of `(literals, level)` — PHP indexes into these with the BRANCH_* constants. @@ -176,7 +176,7 @@ 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.get_id(), package.clone_box()); + self.fixed_map.insert(package.get_id(), package.clone()); } } @@ -296,13 +296,22 @@ impl Solver { return Err(anyhow::anyhow!("solver problems")); } - // TODO(phase-b): LockTransaction expects IndexMap<_, Box> - // and borrows Pool/Decisions. The present/fixed maps from Request are keyed - // by BasePackage; converting requires reworking Request. + // LockTransaction stores PackageInterfaceHandle maps; widen the request's BasePackageHandle + // maps into them. + let present_map = request + .get_present_map(false) + .into_iter() + .map(|(k, v)| (k, v.into())) + .collect(); + let unlockable_map = request + .get_fixed_packages_map() + .into_iter() + .map(|(k, v)| (k, v.into())) + .collect(); Ok(LockTransaction::new( &*self.pool.borrow(), - todo!("convert request.get_present_map(false) to PackageInterface map"), - todo!("convert request.get_fixed_packages_map() to PackageInterface map"), + present_map, + unlockable_map, &self.decisions, )) } diff --git a/crates/shirabe/src/dependency_resolver/transaction.rs b/crates/shirabe/src/dependency_resolver/transaction.rs index a704c67..8cf5d5b 100644 --- a/crates/shirabe/src/dependency_resolver/transaction.rs +++ b/crates/shirabe/src/dependency_resolver/transaction.rs @@ -1,11 +1,8 @@ //! ref: composer/src/Composer/DependencyResolver/Transaction.php -use std::any::Any; - use indexmap::IndexMap; use shirabe_php_shim::{ - PhpMixed, array_filter, array_intersect, array_keys, array_pop, array_unshift, spl_object_hash, - strcmp, uasort, + PhpMixed, array_filter, array_intersect, array_keys, array_pop, array_unshift, strcmp, uasort, }; use crate::dependency_resolver::operation::InstallOperation; @@ -14,9 +11,8 @@ use crate::dependency_resolver::operation::MarkAliasUninstalledOperation; use crate::dependency_resolver::operation::OperationInterface; use crate::dependency_resolver::operation::UninstallOperation; use crate::dependency_resolver::operation::UpdateOperation; -use crate::package::AliasPackage; use crate::package::Link; -use crate::package::PackageInterface; +use crate::package::PackageInterfaceHandle; use crate::repository::PlatformRepository; /// @internal @@ -27,14 +23,14 @@ pub struct Transaction { /// Packages present at the beginning of the transaction /// @var PackageInterface[] - pub(crate) present_packages: Vec>, + pub(crate) present_packages: Vec, /// Package set resulting from this transaction /// @var array - pub(crate) result_package_map: IndexMap>, + pub(crate) result_package_map: IndexMap, /// @var array - pub(crate) result_packages_by_name: IndexMap>>, + pub(crate) result_packages_by_name: IndexMap>, } impl Default for Transaction { @@ -52,8 +48,8 @@ impl Transaction { /// @param PackageInterface[] $presentPackages /// @param PackageInterface[] $resultPackages pub fn new( - present_packages: Vec>, - result_packages: Vec>, + present_packages: Vec, + result_packages: Vec, ) -> Self { let mut this = Self { operations: vec![], @@ -72,23 +68,23 @@ impl Transaction { } /// @param PackageInterface[] $resultPackages - fn set_result_package_maps(&mut self, result_packages: Vec>) { + fn set_result_package_maps(&mut self, result_packages: Vec) { // PHP: static function (PackageInterface $a, PackageInterface $b): int { ... }; // TODO(phase-b): bridge the closure to uasort's argument type - let _package_sort = |a: &dyn PackageInterface, b: &dyn PackageInterface| -> i64 { + let _package_sort = |a: &PackageInterfaceHandle, b: &PackageInterfaceHandle| -> i64 { // sort alias packages by the same name behind their non alias version if a.get_name() == b.get_name() { - let a_is_alias = a.as_any().downcast_ref::().is_some(); - let b_is_alias = b.as_any().downcast_ref::().is_some(); + let a_is_alias = a.as_alias().is_some(); + let b_is_alias = b.as_alias().is_some(); if a_is_alias != b_is_alias { return if a_is_alias { -1 } else { 1 }; } // if names are the same, compare version, e.g. to sort aliases reliably, actual order does not matter - return strcmp(b.get_version(), a.get_version()); + return strcmp(&b.get_version(), &a.get_version()); } - strcmp(b.get_name(), a.get_name()) + strcmp(&b.get_name(), &a.get_name()) }; self.result_package_map = IndexMap::new(); @@ -97,10 +93,10 @@ impl Transaction { self.result_packages_by_name .entry(name) .or_insert_with(Vec::new) - .push(package.clone_package_box()); + .push(package.clone()); } self.result_package_map - .insert(spl_object_hash(package.as_ref()), package); + .insert(package.ptr_id().to_string(), package); } // TODO(phase-b): uasort signature mismatch — needs to operate on the IndexMap with a PackageInterface comparator @@ -121,24 +117,23 @@ impl Transaction { pub(crate) fn calculate_operations(&mut self) -> Vec> { let mut operations: Vec> = vec![]; - let mut present_package_map: IndexMap> = IndexMap::new(); - let mut remove_map: IndexMap> = IndexMap::new(); - let mut present_alias_map: IndexMap> = IndexMap::new(); - let mut remove_alias_map: IndexMap> = IndexMap::new(); + let mut present_package_map: IndexMap = IndexMap::new(); + let mut remove_map: IndexMap = IndexMap::new(); + let mut present_alias_map: IndexMap = IndexMap::new(); + let mut remove_alias_map: IndexMap = IndexMap::new(); for package in &self.present_packages { - if package.as_any().downcast_ref::().is_some() { + if package.as_alias().is_some() { let key = format!("{}::{}", package.get_name(), package.get_version()); - present_alias_map.insert(key.clone(), package.clone_package_box()); - remove_alias_map.insert(key, package.clone_package_box()); + present_alias_map.insert(key.clone(), package.clone()); + remove_alias_map.insert(key, package.clone()); } else { - present_package_map - .insert(package.get_name().to_string(), package.clone_package_box()); - remove_map.insert(package.get_name().to_string(), package.clone_package_box()); + present_package_map.insert(package.get_name().to_string(), package.clone()); + remove_map.insert(package.get_name().to_string(), package.clone()); } } // PHP: $stack = $this->getRootPackages(); - let mut stack: Vec> = + let mut stack: Vec = self.get_root_packages().into_values().collect(); let mut visited: IndexMap = IndexMap::new(); @@ -147,16 +142,16 @@ impl Transaction { while !stack.is_empty() { let package = array_pop(&mut stack).unwrap(); - if processed.contains_key(&spl_object_hash(package.as_ref())) { + if processed.contains_key(&package.ptr_id().to_string()) { continue; } - if !visited.contains_key(&spl_object_hash(package.as_ref())) { - visited.insert(spl_object_hash(package.as_ref()), true); + if !visited.contains_key(&package.ptr_id().to_string()) { + visited.insert(package.ptr_id().to_string(), true); - stack.push(package.clone_package_box()); - if let Some(alias) = package.as_any().downcast_ref::() { - stack.push(alias.get_alias_of().clone_package_box()); + stack.push(package.clone()); + if let Some(alias) = package.as_alias() { + stack.push(alias.get_alias_of().into()); } else { for link in package.get_requires().values() { let possible_requires = self.get_providers_in_result(link); @@ -166,10 +161,10 @@ impl Transaction { } } } - } else if !processed.contains_key(&spl_object_hash(package.as_ref())) { - processed.insert(spl_object_hash(package.as_ref()), true); + } else if !processed.contains_key(&package.ptr_id().to_string()) { + processed.insert(package.ptr_id().to_string(), true); - if package.as_any().downcast_ref::().is_some() { + if package.as_alias().is_some() { let alias_key = format!("{}::{}", package.get_name(), package.get_version()); if present_alias_map.contains_key(&alias_key) { remove_alias_map.shift_remove(&alias_key); @@ -179,18 +174,22 @@ impl Transaction { "package as AliasPackage by value" )))); } - } else if let Some(source) = present_package_map.get(package.get_name()) { + } else if let Some(source) = present_package_map.get(&package.get_name()).cloned() { // do we need to update? // TODO different for lock? - let present = present_package_map.get(package.get_name()).unwrap(); - // TODO(phase-b): downcast to CompletePackageInterface trait object - let package_is_complete = false; - let present_is_complete = false; + let present = present_package_map.get(&package.get_name()).unwrap(); + // PHP: $package instanceof CompletePackageInterface + // && $presentPackageMap[$package->getName()] instanceof CompletePackageInterface + // && ($package->isAbandoned() !== $presentPackageMap[...]->isAbandoned() + // || $package->getReplacementPackage() !== $presentPackageMap[...]->getReplacementPackage()) let abandoned_or_replacement_changed = - package_is_complete && present_is_complete && { - // PHP: $package->isAbandoned() !== $presentPackageMap[$package->getName()]->isAbandoned() - // || $package->getReplacementPackage() !== $presentPackageMap[$package->getName()]->getReplacementPackage() - todo!("compare abandoned/replacement across CompletePackageInterface") + match (package.as_complete(), present.as_complete()) { + (Some(package), Some(present)) => { + package.is_abandoned() != present.is_abandoned() + || package.get_replacement_package() + != present.get_replacement_package() + } + _ => false, }; if package.get_version() != present.get_version() || package.get_dist_reference() != present.get_dist_reference() @@ -198,14 +197,14 @@ impl Transaction { || abandoned_or_replacement_changed { operations.push(Box::new(UpdateOperation::new( - source.clone_package_box(), - package.clone_package_box(), + source.clone(), + package.clone(), ))); } - remove_map.shift_remove(package.get_name()); + remove_map.shift_remove(&package.get_name()); } else { - operations.push(Box::new(InstallOperation::new(package.clone_package_box()))); - remove_map.shift_remove(package.get_name()); + operations.push(Box::new(InstallOperation::new(package.clone()))); + remove_map.shift_remove(&package.get_name()); } } } @@ -244,11 +243,11 @@ impl Transaction { /// If there are packages with a cycle on the top level the package with the lowest name gets picked /// /// @return array - pub(crate) fn get_root_packages(&self) -> IndexMap> { - let mut roots: IndexMap> = self + pub(crate) fn get_root_packages(&self) -> IndexMap { + let mut roots: IndexMap = self .result_package_map .iter() - .map(|(k, v)| (k.clone(), v.clone_package_box())) + .map(|(k, v)| (k.clone(), v.clone())) .collect(); for (package_hash, package) in &self.result_package_map { @@ -261,8 +260,8 @@ impl Transaction { for require in possible_requires { // PHP: if ($require !== $package) — strict reference inequality - if spl_object_hash(require.as_ref()) != spl_object_hash(package.as_ref()) { - roots.shift_remove(&spl_object_hash(require.as_ref())); + if require.ptr_id().to_string() != package.ptr_id().to_string() { + roots.shift_remove(&require.ptr_id().to_string()); } } } @@ -272,12 +271,12 @@ impl Transaction { } /// @return PackageInterface[] - pub(crate) fn get_providers_in_result(&self, link: &Link) -> Vec> { + pub(crate) fn get_providers_in_result(&self, link: &Link) -> Vec { let Some(packages) = self.result_packages_by_name.get(link.get_target()) else { return vec![]; }; - packages.iter().map(|p| p.clone_package_box()).collect() + packages.iter().cloned().collect() } /// Workaround: if your packages depend on plugins, we must be sure @@ -308,12 +307,12 @@ impl Transaction { for idx in (0..operations.len()).rev() { let op = &operations[idx]; - let package: Box = if let Some(install_op) = + let package: PackageInterfaceHandle = if let Some(install_op) = op.as_ref().as_any().downcast_ref::() { - install_op.get_package().clone_package_box() + install_op.get_package().clone() } else if let Some(update_op) = op.as_ref().as_any().downcast_ref::() { - update_op.get_target_package().clone_package_box() + update_op.get_target_package().clone() } else { continue; }; -- cgit v1.3.1