From 20dbcf11b86cb03c451ba1d5cd9efe17b68fa66d Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 31 May 2026 21:34:47 +0900 Subject: fix(package): port every PHP clone operator to handle dup() --- .../shirabe/src/command/base_dependency_command.rs | 8 +++-- .../src/command/check_platform_reqs_command.rs | 4 ++- crates/shirabe/src/command/home_command.rs | 4 ++- crates/shirabe/src/command/show_command.rs | 28 ++++++++------- crates/shirabe/src/command/suggests_command.rs | 4 +-- .../src/dependency_resolver/pool_builder.rs | 13 +++---- crates/shirabe/src/installer.rs | 9 ++--- .../shirabe/src/installer/installation_manager.rs | 2 +- crates/shirabe/src/installer/library_installer.rs | 4 +-- .../shirabe/src/installer/metapackage_installer.rs | 4 +-- crates/shirabe/src/installer/noop_installer.rs | 4 +-- crates/shirabe/src/package/alias_package.rs | 2 +- .../shirabe/src/package/complete_alias_package.rs | 4 +-- crates/shirabe/src/package/complete_package.rs | 2 +- crates/shirabe/src/package/handle.rs | 41 +++++++++++++++++++++- .../src/package/loader/root_package_loader.rs | 19 ++++------ crates/shirabe/src/package/locker.rs | 3 +- crates/shirabe/src/package/package.rs | 2 +- crates/shirabe/src/package/root_alias_package.rs | 14 +++++--- crates/shirabe/src/package/root_package.rs | 2 +- crates/shirabe/src/plugin/plugin_manager.rs | 6 ++-- 21 files changed, 113 insertions(+), 66 deletions(-) diff --git a/crates/shirabe/src/command/base_dependency_command.rs b/crates/shirabe/src/command/base_dependency_command.rs index 0b89387..177b0e6 100644 --- a/crates/shirabe/src/command/base_dependency_command.rs +++ b/crates/shirabe/src/command/base_dependency_command.rs @@ -55,7 +55,9 @@ pub trait BaseDependencyCommand: BaseCommand { let mut repos: Vec = vec![crate::repository::RepositoryInterfaceHandle::new( - RootPackageRepository::new(composer.get_package().clone()), + RootPackageRepository::new(crate::package::RootPackageInterfaceHandle::dup( + composer.get_package(), + )), )]; if input.get_option("locked").as_bool().unwrap_or(false) { @@ -159,7 +161,9 @@ pub trait BaseDependencyCommand: BaseCommand { ) { installed_repo.add_repository( crate::repository::RepositoryInterfaceHandle::new( - InstalledArrayRepository::new_with_packages(vec![r#match.into()])?, + InstalledArrayRepository::new_with_packages(vec![ + crate::package::PackageInterfaceHandle::dup(&r#match), + ])?, ), )?; } else if PlatformRepository::is_platform_package(&needle) { diff --git a/crates/shirabe/src/command/check_platform_reqs_command.rs b/crates/shirabe/src/command/check_platform_reqs_command.rs index e53727d..bfef6a5 100644 --- a/crates/shirabe/src/command/check_platform_reqs_command.rs +++ b/crates/shirabe/src/command/check_platform_reqs_command.rs @@ -112,7 +112,9 @@ impl CheckPlatformReqsCommand { } } - let root_pkg_repo = RootPackageRepository::new(composer.get_package().clone()); + let root_pkg_repo = RootPackageRepository::new( + crate::package::RootPackageInterfaceHandle::dup(composer.get_package()), + ); let installed_repo = InstalledRepository::new(vec![ installed_repo_base, crate::repository::RepositoryInterfaceHandle::new(root_pkg_repo), diff --git a/crates/shirabe/src/command/home_command.rs b/crates/shirabe/src/command/home_command.rs index 067af8f..e037487 100644 --- a/crates/shirabe/src/command/home_command.rs +++ b/crates/shirabe/src/command/home_command.rs @@ -207,7 +207,9 @@ impl HomeCommand { let composer = crate::command::composer_full(&composer); let mut repos: Vec = vec![]; repos.push(crate::repository::RepositoryInterfaceHandle::new( - RootPackageRepository::new(composer.get_package().clone()), + RootPackageRepository::new(crate::package::RootPackageInterfaceHandle::dup( + composer.get_package(), + )), )); // TODO(phase-b): get_local_repository / get_repositories return shared refs; needs Rc migration return Ok(repos); diff --git a/crates/shirabe/src/command/show_command.rs b/crates/shirabe/src/command/show_command.rs index 781ee20..e7fcfe1 100644 --- a/crates/shirabe/src/command/show_command.rs +++ b/crates/shirabe/src/command/show_command.rs @@ -212,8 +212,9 @@ impl ShowCommand { && input.get_option("locked").as_bool() != Some(true) { let composer = self.require_composer(None, None)?; - let package: crate::package::RootPackageInterfaceHandle = - composer.borrow_partial().get_package().clone(); + let package = crate::package::RootPackageInterfaceHandle::dup( + composer.borrow_partial().get_package(), + ); if input.get_option("name-only").as_bool() == Some(true) { self.get_io().write(&package.get_name()); @@ -381,14 +382,15 @@ impl ShowCommand { }; let root_pkg = composer_local.get_package(); - let root_repo: RepositoryInterfaceHandle = - if input.get_option("self").as_bool() == Some(true) { - RepositoryInterfaceHandle::new(RootPackageRepository::new( - composer_local.get_package().clone(), - )) - } else { - RepositoryInterfaceHandle::new(InstalledArrayRepository::new()?) - }; + let root_repo: RepositoryInterfaceHandle = if input.get_option("self").as_bool() + == Some(true) + { + RepositoryInterfaceHandle::new(RootPackageRepository::new( + crate::package::RootPackageInterfaceHandle::dup(composer_local.get_package()), + )) + } else { + RepositoryInterfaceHandle::new(InstalledArrayRepository::new()?) + }; if input.get_option("no-dev").as_bool() == Some(true) { let local_packages = composer_local .get_repository_manager() @@ -401,8 +403,10 @@ impl ShowCommand { false, Vec::new(), ); - let cloned: Vec = - packages.into_iter().map(|p| p.into()).collect(); + let cloned: Vec = packages + .iter() + .map(crate::package::PackageInterfaceHandle::dup) + .collect(); installed_repo = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ root_repo.clone(), RepositoryInterfaceHandle::new(InstalledArrayRepository::new_with_packages( diff --git a/crates/shirabe/src/command/suggests_command.rs b/crates/shirabe/src/command/suggests_command.rs index 0aead77..2091608 100644 --- a/crates/shirabe/src/command/suggests_command.rs +++ b/crates/shirabe/src/command/suggests_command.rs @@ -48,11 +48,9 @@ impl SuggestsCommand { let composer = self.require_composer(None, None)?; let mut composer = crate::command::composer_full_mut(&composer); - let root_package_handle: crate::package::RootPackageInterfaceHandle = - composer.get_package().clone(); let mut installed_repos: Vec = vec![RepositoryInterfaceHandle::new(RootPackageRepository::new( - root_package_handle, + crate::package::RootPackageInterfaceHandle::dup(composer.get_package()), ))]; if composer.get_locker().borrow_mut().is_locked() { diff --git a/crates/shirabe/src/dependency_resolver/pool_builder.rs b/crates/shirabe/src/dependency_resolver/pool_builder.rs index f85eb31..18d2db4 100644 --- a/crates/shirabe/src/dependency_resolver/pool_builder.rs +++ b/crates/shirabe/src/dependency_resolver/pool_builder.rs @@ -625,10 +625,12 @@ impl PoolBuilder { // 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) { - // 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; + // do not modify the references on already locked or fixed packages + if !request.is_locked_package(package.clone()) + && !request.is_fixed_package(package.clone()) + { + package.set_source_dist_references(reference); + } } // if propagateUpdate is false we are loading a fixed or locked package, root aliases do not apply as they are @@ -664,8 +666,7 @@ impl PoolBuilder { let new_index = self.index_counter; self.index_counter += 1; - self.packages - .insert(new_index, alias_handle.clone().into()); + self.packages.insert(new_index, alias_handle.clone().into()); self.alias_map .entry(alias_handle.get_alias_of().ptr_id().to_string()) .or_insert_with(IndexMap::new) diff --git a/crates/shirabe/src/installer.rs b/crates/shirabe/src/installer.rs index 434996d..ec374ac 100644 --- a/crates/shirabe/src/installer.rs +++ b/crates/shirabe/src/installer.rs @@ -352,7 +352,7 @@ impl Installer { locked_repository_handle, crate::repository::RepositoryInterfaceHandle::new(self.create_platform_repo(false)), crate::repository::RepositoryInterfaceHandle::new(RootPackageRepository::new( - self.package.clone(), + RootPackageInterfaceHandle::dup(&self.package), )), ]); if is_fresh_install { @@ -1335,10 +1335,7 @@ impl Installer { root_requires.insert(req, constraint); } - // TODO(phase-c): PHP does `$this->fixedRootPackage = clone($this->package)` (a deep, - // fresh-identity copy) and then strips its requires below. A handle clone only shares the - // same Rc, so a real deep clone of the package is needed to avoid mutating self.package. - // self.fixed_root_package = deep_clone(&self.package); + self.fixed_root_package = RootPackageInterfaceHandle::dup(&self.package); self.fixed_root_package.set_requires(vec![]); self.fixed_root_package.set_dev_requires(vec![]); @@ -1578,7 +1575,7 @@ impl Installer { fn mock_local_repositories(&self, rm: &mut RepositoryManager) { let mut packages: IndexMap = IndexMap::new(); for package in rm.get_local_repository().get_packages() { - packages.insert(package.to_string(), package.clone().into()); + packages.insert(package.to_string(), PackageInterfaceHandle::dup(&package)); } let keys: Vec = packages.keys().cloned().collect(); for key in keys { diff --git a/crates/shirabe/src/installer/installation_manager.rs b/crates/shirabe/src/installer/installation_manager.rs index 31cc23c..578a61e 100644 --- a/crates/shirabe/src/installer/installation_manager.rs +++ b/crates/shirabe/src/installer/installation_manager.rs @@ -600,7 +600,7 @@ impl InstallationManager { let package = operation.get_package(); if !repo.has_package(package.clone().into()) { - repo.add_package(package.into()); + repo.add_package(crate::package::PackageInterfaceHandle::dup(&package.into())); } } diff --git a/crates/shirabe/src/installer/library_installer.rs b/crates/shirabe/src/installer/library_installer.rs index c1fedef..b4b2d20 100644 --- a/crates/shirabe/src/installer/library_installer.rs +++ b/crates/shirabe/src/installer/library_installer.rs @@ -322,7 +322,7 @@ impl InstallerInterface for LibraryInstaller { self.binary_installer .install_binaries(package.clone(), &install_path, true); if !repo.has_package(package.clone()) { - repo.add_package(package.clone()); + repo.add_package(PackageInterfaceHandle::dup(&package)); } Ok(None) @@ -353,7 +353,7 @@ impl InstallerInterface for LibraryInstaller { .install_binaries(target.clone(), &install_path, true); repo.remove_package(initial.clone()); if !repo.has_package(target.clone()) { - repo.add_package(target.clone()); + repo.add_package(PackageInterfaceHandle::dup(&target)); } Ok(None) diff --git a/crates/shirabe/src/installer/metapackage_installer.rs b/crates/shirabe/src/installer/metapackage_installer.rs index 820023b..0c67e9a 100644 --- a/crates/shirabe/src/installer/metapackage_installer.rs +++ b/crates/shirabe/src/installer/metapackage_installer.rs @@ -74,7 +74,7 @@ impl InstallerInterface for MetapackageInstaller { io_interface::NORMAL, ); - repo.add_package(package); + repo.add_package(PackageInterfaceHandle::dup(&package)); Ok(None) } @@ -103,7 +103,7 @@ impl InstallerInterface for MetapackageInstaller { ); repo.remove_package(initial.clone()); - repo.add_package(target); + repo.add_package(PackageInterfaceHandle::dup(&target)); Ok(None) } diff --git a/crates/shirabe/src/installer/noop_installer.rs b/crates/shirabe/src/installer/noop_installer.rs index dad5231..3041e14 100644 --- a/crates/shirabe/src/installer/noop_installer.rs +++ b/crates/shirabe/src/installer/noop_installer.rs @@ -54,7 +54,7 @@ impl InstallerInterface for NoopInstaller { package: PackageInterfaceHandle, ) -> anyhow::Result> { if !repo.has_package(package.clone()) { - repo.add_package(package.clone()); + repo.add_package(PackageInterfaceHandle::dup(&package)); } Ok(None) @@ -76,7 +76,7 @@ impl InstallerInterface for NoopInstaller { repo.remove_package(initial.clone()); if !repo.has_package(target.clone()) { - repo.add_package(target.clone()); + repo.add_package(PackageInterfaceHandle::dup(&target)); } Ok(None) diff --git a/crates/shirabe/src/package/alias_package.rs b/crates/shirabe/src/package/alias_package.rs index ff846a6..b4ddc70 100644 --- a/crates/shirabe/src/package/alias_package.rs +++ b/crates/shirabe/src/package/alias_package.rs @@ -13,7 +13,7 @@ use crate::package::PackageInterface; use crate::package::version::VersionParser; use crate::repository::RepositoryInterfaceHandle; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct AliasPackage { id: i64, name: String, diff --git a/crates/shirabe/src/package/complete_alias_package.rs b/crates/shirabe/src/package/complete_alias_package.rs index 6a23bee..7720258 100644 --- a/crates/shirabe/src/package/complete_alias_package.rs +++ b/crates/shirabe/src/package/complete_alias_package.rs @@ -9,9 +9,9 @@ use crate::package::CompletePackageInterface; use crate::package::PackageHandle; use crate::package::handle::delegate_package_interface_to_inner; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct CompleteAliasPackage { - inner: AliasPackage, + pub(crate) inner: AliasPackage, // overrides AliasPackage::alias_of with the more specific CompletePackage type pub(crate) alias_of: CompletePackageHandle, } diff --git a/crates/shirabe/src/package/complete_package.rs b/crates/shirabe/src/package/complete_package.rs index 55c7654..97669d8 100644 --- a/crates/shirabe/src/package/complete_package.rs +++ b/crates/shirabe/src/package/complete_package.rs @@ -6,7 +6,7 @@ use crate::package::PackageInterface; use indexmap::IndexMap; use shirabe_php_shim::PhpMixed; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct CompletePackage { pub(crate) inner: Package, pub(crate) repositories: Vec>, diff --git a/crates/shirabe/src/package/handle.rs b/crates/shirabe/src/package/handle.rs index 746e2ce..aecc219 100644 --- a/crates/shirabe/src/package/handle.rs +++ b/crates/shirabe/src/package/handle.rs @@ -12,7 +12,7 @@ use crate::package::{ }; /// Any package type. -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum AnyPackage { Package(Package), CompletePackage(CompletePackage), @@ -137,6 +137,36 @@ impl AnyPackage { pub fn is_root_alias(&self) -> bool { matches!(self, Self::RootAliasPackage(_)) } + + /// PHP `clone $package`: fresh object identity. Matches PHP's shallow + /// clone for most types (scalars/arrays are copied, nested object + /// references — including `aliasOf` on alias variants — are shared), + /// except for RootAliasPackage where PHP's `__clone` hook explicitly + /// reseats `aliasOf` to a fresh clone. + pub fn dup(&self) -> Self { + match self { + Self::Package(p) => Self::Package(p.clone()), + Self::CompletePackage(p) => Self::CompletePackage(p.clone()), + Self::RootPackage(p) => Self::RootPackage(p.clone()), + Self::AliasPackage(p) => Self::AliasPackage(p.clone()), + Self::CompleteAliasPackage(p) => Self::CompleteAliasPackage(p.clone()), + Self::RootAliasPackage(p) => { + // PHP's RootAliasPackage overrides `__clone()`: + // $this->aliasOf = clone $this->aliasOf; + let new_alias_of_inner = p.alias_of.0.borrow().dup(); + let new_alias_of_rc = Rc::new(RefCell::new(new_alias_of_inner)); + let new_root = RootPackageHandle(new_alias_of_rc.clone()); + let new_complete = CompletePackageHandle(new_alias_of_rc.clone()); + let new_pkg = PackageHandle(new_alias_of_rc); + + let mut cloned = p.clone(); + cloned.alias_of = new_root; + cloned.inner.alias_of = new_complete; + cloned.inner.inner.alias_of = new_pkg; + Self::RootAliasPackage(cloned) + } + } + } } macro_rules! delegate_package_interface_to_inner { @@ -1119,6 +1149,15 @@ macro_rules! impl_handle_common { pub fn ptr_eq(&self, other: &Self) -> bool { std::rc::Rc::ptr_eq(&self.0, &other.0) } + + /// PHP `clone $x`: fresh object identity. See [`AnyPackage::dup`] + /// for the per-variant semantics (including the RootAliasPackage + /// `__clone` hook). + pub fn dup(other: &Self) -> Self { + Self(std::rc::Rc::new(std::cell::RefCell::new( + other.0.borrow().dup(), + ))) + } } impl PartialEq for $Handle { diff --git a/crates/shirabe/src/package/loader/root_package_loader.rs b/crates/shirabe/src/package/loader/root_package_loader.rs index 1b4d155..b31525f 100644 --- a/crates/shirabe/src/package/loader/root_package_loader.rs +++ b/crates/shirabe/src/package/loader/root_package_loader.rs @@ -11,8 +11,6 @@ use crate::io::IOInterface; use crate::io::IOInterfaceImmutable; use crate::package::CompletePackageInterface; use crate::package::PackageInterface; -use crate::package::RootAliasPackage; -use crate::package::RootPackage; use crate::package::RootPackageInterface; use crate::package::loader::ArrayLoader; use crate::package::loader::LoaderInterface; @@ -194,19 +192,16 @@ impl RootPackageLoader { Some("Composer\\Package\\RootPackage".to_string()), )?; - // TODO(phase-c): mutating the loaded RootPackage through a PackageInterfaceHandle - // requires going through as_root_package() + a RefCell borrow; the inherent - // RootPackage mutators used below are not yet reachable that way. - let real_package: &mut RootPackage = { - let _ = &mut package; - todo!( - "mutate RootPackage through PackageInterfaceHandle (as_root_package + borrow_mut)" - ) + let real_package = if let Some(alias) = package.as_root_alias_package() { + alias.get_alias_of() + } else { + package + .as_root_package() + .expect("Expecting a Composer\\Package\\RootPackage at this point") }; if auto_versioned { // TODO(phase-b): replace_version is an inherent method on Package, not exposed via trait. - let _ = real_package; todo!("replace_version is not accessible through RootPackage's embedded Package"); } @@ -230,7 +225,7 @@ impl RootPackageLoader { aliases = self.extract_aliases(&links, aliases); stability_flags = Self::extract_stability_flags( &links, - real_package.get_minimum_stability(), + &real_package.get_minimum_stability(), stability_flags, ); references = Self::extract_references(&links, references); diff --git a/crates/shirabe/src/package/locker.rs b/crates/shirabe/src/package/locker.rs index 327f662..9e36255 100644 --- a/crates/shirabe/src/package/locker.rs +++ b/crates/shirabe/src/package/locker.rs @@ -951,8 +951,7 @@ impl Locker { description: "Required (in require-dev)".to_string(), }); } - // TODO(phase-b): clone $package to a RootPackageRepository - let root_repo = RootPackageRepository::new(todo!("phase-b: clone root package")); + let root_repo = RootPackageRepository::new(RootPackageInterfaceHandle::dup(&package)); for set in &sets { let installed_repo = InstalledRepository::new(vec![/* set.repo, root_repo */]); diff --git a/crates/shirabe/src/package/package.rs b/crates/shirabe/src/package/package.rs index ba9e3f9..f573326 100644 --- a/crates/shirabe/src/package/package.rs +++ b/crates/shirabe/src/package/package.rs @@ -24,7 +24,7 @@ pub struct Mirror { } /// Core package definitions that are needed to resolve dependencies and install packages -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Package { id: i64, name: String, diff --git a/crates/shirabe/src/package/root_alias_package.rs b/crates/shirabe/src/package/root_alias_package.rs index 7298c1f..67e123b 100644 --- a/crates/shirabe/src/package/root_alias_package.rs +++ b/crates/shirabe/src/package/root_alias_package.rs @@ -11,9 +11,9 @@ use crate::package::RootPackageHandle; use crate::package::RootPackageInterface; use crate::package::handle::delegate_package_interface_to_inner; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct RootAliasPackage { - inner: CompleteAliasPackage, + pub(crate) inner: CompleteAliasPackage, // overrides CompleteAliasPackage::alias_of with the more specific RootPackage type pub(crate) alias_of: RootPackageHandle, } @@ -83,8 +83,14 @@ impl RootPackageInterface for RootAliasPackage { } fn set_requires(&mut self, requires: Vec) { - // TODO(phase-c): PHP re-derives the local links via - // replaceSelfVersionDependencies before forwarding to aliasOf. + let replaced = self + .inner + .inner + .replace_self_version_dependencies(requires.clone(), Link::TYPE_REQUIRE); + self.inner.inner.requires = replaced + .into_iter() + .map(|l| (l.get_target().to_string(), l)) + .collect(); self.alias_of.set_requires(requires); } diff --git a/crates/shirabe/src/package/root_package.rs b/crates/shirabe/src/package/root_package.rs index 5a719de..9cedc1a 100644 --- a/crates/shirabe/src/package/root_package.rs +++ b/crates/shirabe/src/package/root_package.rs @@ -11,7 +11,7 @@ use crate::package::PackageInterface; use crate::package::RootPackageInterface; use crate::repository::RepositoryInterfaceHandle; -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct RootPackage { inner: CompletePackage, pub(crate) minimum_stability: String, diff --git a/crates/shirabe/src/plugin/plugin_manager.rs b/crates/shirabe/src/plugin/plugin_manager.rs index c917e99..fb68918 100644 --- a/crates/shirabe/src/plugin/plugin_manager.rs +++ b/crates/shirabe/src/plugin/plugin_manager.rs @@ -135,9 +135,9 @@ impl PluginManager { .get_repository_manager() .borrow() .get_local_repository(); - // The root package borrow is also tied to `self.composer`; clone the package handle - // (shared Rc) for the same reason as above. - let root_package = self.composer_full().borrow().get_package().clone(); + let root_package = crate::package::RootPackageInterfaceHandle::dup( + self.composer_full().borrow().get_package(), + ); self.load_repository(&*repo.borrow(), false, Some(root_package))?; } -- cgit v1.3.1