From e1053c6881da1bba409a16783e01a89248507a66 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 7 Jun 2026 10:32:26 +0900 Subject: refactor(phase-c): share PlatformRepository and RepositorySet via handles PHP shares a single PlatformRepository by reference across the RepositorySet, createRequest, VersionSelector, and (in show) the installed repository. The port worked with owned values / &mut, so it could not share: create_repository_set silently dropped the platform repo from the pool (PlatformRepository is not Clone), show rebuilt a fresh PlatformRepository per use, and the package discovery / show version selectors were stubbed because VersionSelector wanted an owned RepositorySet. Thread the existing PlatformRepositoryHandle (Rc>) through installer.rs and show, restoring the RootPackageRepository + platform repo registration and implementing same_repository via RepositoryInterfaceHandle ptr_eq. Hold package-discovery repos as a shared RepositoryInterfaceHandle, and share RepositorySet as Rc> in the set caches and VersionSelector (which only reads it), unblocking both stubbed selector sites and dropping show's placeholder set. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shirabe/src/command/archive_command.rs | 3 +- .../shirabe/src/command/create_project_command.rs | 5 +- crates/shirabe/src/command/init_command.rs | 11 +-- .../shirabe/src/command/package_discovery_trait.rs | 51 ++++++++------ crates/shirabe/src/command/require_command.rs | 14 ++-- crates/shirabe/src/command/show_command.rs | 79 ++++++++++------------ crates/shirabe/src/command/update_command.rs | 5 +- 7 files changed, 91 insertions(+), 77 deletions(-) (limited to 'crates/shirabe/src/command') diff --git a/crates/shirabe/src/command/archive_command.rs b/crates/shirabe/src/command/archive_command.rs index c950eaa..8ac5b9f 100644 --- a/crates/shirabe/src/command/archive_command.rs +++ b/crates/shirabe/src/command/archive_command.rs @@ -319,7 +319,8 @@ impl ArchiveCommand { let packages = repo_set.find_packages(&package_name.to_lowercase(), constraint, 0)?; let package = if packages.len() > 1 { - let mut version_selector = VersionSelector::new(repo_set, None)?; + let mut version_selector = + VersionSelector::new(std::rc::Rc::new(std::cell::RefCell::new(repo_set)), None)?; let best = version_selector.find_best_candidate( &package_name.to_lowercase(), version.as_deref(), diff --git a/crates/shirabe/src/command/create_project_command.rs b/crates/shirabe/src/command/create_project_command.rs index 0d5708d..f59e1eb 100644 --- a/crates/shirabe/src/command/create_project_command.rs +++ b/crates/shirabe/src/command/create_project_command.rs @@ -848,7 +848,10 @@ impl CreateProjectCommand { )?; // find the latest version if there are multiple - let mut version_selector = VersionSelector::new(repository_set, Some(&mut platform_repo))?; + let mut version_selector = VersionSelector::new( + std::rc::Rc::new(std::cell::RefCell::new(repository_set)), + Some(&mut platform_repo), + )?; // TODO(phase-b): platform_requirement_filter is &dyn here but VersionSelector expects // Option>; pass None as placeholder. let _ = platform_requirement_filter; diff --git a/crates/shirabe/src/command/init_command.rs b/crates/shirabe/src/command/init_command.rs index eabe6e2..5a3c71b 100644 --- a/crates/shirabe/src/command/init_command.rs +++ b/crates/shirabe/src/command/init_command.rs @@ -44,13 +44,14 @@ pub struct InitCommand { } impl PackageDiscoveryTrait for InitCommand { - fn get_repos_mut(&mut self) -> &mut Option { + fn get_repos_mut(&mut self) -> &mut Option { todo!() } fn get_repository_sets_mut( &mut self, - ) -> &mut IndexMap { + ) -> &mut IndexMap>> + { todo!() } @@ -512,7 +513,9 @@ impl InitCommand { )?); } - *self.get_repos_mut() = Some(CompositeRepository::new(repos)); + *self.get_repos_mut() = Some(crate::repository::RepositoryInterfaceHandle::new( + CompositeRepository::new(repos), + )); // unset($repos, $config, $repositories); } @@ -746,7 +749,7 @@ impl InitCommand { io.write_error3("\nDefine your dependencies.\n", true, io_interface::NORMAL); // prepare to resolve dependencies - let repos = self.get_repos(); + let _repos = self.get_repos(); let preferred_stability = if let Some(s) = minimum_stability_default.clone().filter(|s| !s.is_empty()) { s diff --git a/crates/shirabe/src/command/package_discovery_trait.rs b/crates/shirabe/src/command/package_discovery_trait.rs index 4162676..cd6c5a0 100644 --- a/crates/shirabe/src/command/package_discovery_trait.rs +++ b/crates/shirabe/src/command/package_discovery_trait.rs @@ -36,8 +36,10 @@ use crate::util::Filesystem; pub trait PackageDiscoveryTrait { // PHP: private $repos; private $repositorySets; // TODO(phase-b): trait fields require an associated state struct in Rust; expose via accessors - fn get_repos_mut(&mut self) -> &mut Option; - fn get_repository_sets_mut(&mut self) -> &mut IndexMap; + fn get_repos_mut(&mut self) -> &mut Option; + fn get_repository_sets_mut( + &mut self, + ) -> &mut IndexMap>>; // PHP: trait dependencies (provided by BaseCommand) fn get_io(&self) -> std::rc::Rc>; @@ -56,15 +58,14 @@ pub trait PackageDiscoveryTrait { fn normalize_requirements(&self, requires: Vec) -> Vec>; - fn get_repos(&mut self) -> &mut CompositeRepository { + fn get_repos(&mut self) -> crate::repository::RepositoryInterfaceHandle { if self.get_repos_mut().is_none() { // PHP: array_merge([new PlatformRepository], RepositoryFactory::defaultReposWithDefaultManager($this->getIO())) - let mut repos: Vec = vec![ - // TODO(phase-b): PlatformRepository::new() signature - crate::repository::RepositoryInterfaceHandle::new::(todo!( - "PlatformRepository::new()" - )), - ]; + let mut repos: Vec = + vec![crate::repository::RepositoryInterfaceHandle::new( + PlatformRepository::new(vec![], IndexMap::new()) + .expect("PlatformRepository::new should not fail"), + )]; let io_owned: std::rc::Rc> = self.get_io(); for (_, repo) in RepositoryFactory::default_repos_with_default_manager(io_owned) .unwrap() @@ -72,10 +73,12 @@ pub trait PackageDiscoveryTrait { { repos.push(repo); } - *self.get_repos_mut() = Some(CompositeRepository::new(repos)); + *self.get_repos_mut() = Some(crate::repository::RepositoryInterfaceHandle::new( + CompositeRepository::new(repos), + )); } - self.get_repos_mut().as_mut().unwrap() + self.get_repos_mut().as_ref().unwrap().clone() } /// @param key-of|null $minimumStability @@ -83,7 +86,7 @@ pub trait PackageDiscoveryTrait { &mut self, input: std::rc::Rc>, minimum_stability: Option<&str>, - ) -> &RepositorySet { + ) -> std::rc::Rc> { let key = minimum_stability.unwrap_or("default").to_string(); if !self.get_repository_sets_mut().contains_key(&key) { @@ -98,14 +101,15 @@ pub trait PackageDiscoveryTrait { IndexMap::new(), IndexMap::new(), ); - // TODO(phase-b): self.get_repos() returns reference; add_repository takes ownership - let repos = todo!("self.get_repos() owned/cloned for add_repository"); + let repos = self.get_repos(); let _ = repository_set.add_repository(repos); - self.get_repository_sets_mut() - .insert(key.clone(), repository_set); + self.get_repository_sets_mut().insert( + key.clone(), + std::rc::Rc::new(std::cell::RefCell::new(repository_set)), + ); } - self.get_repository_sets_mut().get(&key).unwrap() + self.get_repository_sets_mut().get(&key).unwrap().clone() } /// @return key-of @@ -515,9 +519,12 @@ pub trait PackageDiscoveryTrait { // find the latest version allowed in this repo set let repo_set = self.get_repository_set(input.clone(), None); - // TODO(phase-b): VersionSelector::new takes owned RepositorySet; we have a shared reference - let mut version_selector: VersionSelector = - todo!("VersionSelector::new with owned repo_set"); + let mut version_selector = match platform_repo { + Some(handle) => { + VersionSelector::new(repo_set.clone(), Some(&mut *handle.borrow_mut()))? + } + None => VersionSelector::new(repo_set.clone(), None)?, + }; let effective_minimum_stability = self.get_minimum_stability(input.clone()); let package = version_selector.find_best_candidate( @@ -539,7 +546,7 @@ pub trait PackageDiscoveryTrait { } // Check if it is a virtual package provided by others - let providers = repo_set.get_providers(name)?; + let providers = repo_set.borrow().get_providers(name)?; if count(&PhpMixed::List( providers.iter().map(|_| Box::new(PhpMixed::Null)).collect(), )) > 0 @@ -823,7 +830,7 @@ pub trait PackageDiscoveryTrait { .into()); } self.get_repos_mut() - .as_mut() + .as_ref() .unwrap() .search(package.to_string(), 0, None) })() { diff --git a/crates/shirabe/src/command/require_command.rs b/crates/shirabe/src/command/require_command.rs index 871b07a..b3ad6cf 100644 --- a/crates/shirabe/src/command/require_command.rs +++ b/crates/shirabe/src/command/require_command.rs @@ -61,11 +61,13 @@ pub struct RequireCommand { } impl PackageDiscoveryTrait for RequireCommand { - fn get_repos_mut(&mut self) -> &mut Option { + fn get_repos_mut(&mut self) -> &mut Option { todo!() } - fn get_repository_sets_mut(&mut self) -> &mut IndexMap { + fn get_repository_sets_mut( + &mut self, + ) -> &mut IndexMap>> { todo!() } @@ -280,7 +282,9 @@ impl RequireCommand { for repo in repos { combined.push(repo.clone()); } - *self.get_repos_mut() = Some(CompositeRepository::new(combined)); + *self.get_repos_mut() = Some(crate::repository::RepositoryInterfaceHandle::new( + CompositeRepository::new(combined), + )); let preferred_stability = if composer.get_package().get_prefer_stable() { "stable".to_string() @@ -1033,14 +1037,14 @@ impl RequireCommand { let locker_is_locked = composer.get_locker().borrow_mut().is_locked(); let mut requirements: IndexMap = IndexMap::new(); let mut version_selector = VersionSelector::new( - RepositorySet::new( + std::rc::Rc::new(std::cell::RefCell::new(RepositorySet::new( "stable", IndexMap::new(), vec![], IndexMap::new(), IndexMap::new(), IndexMap::new(), - ), + ))), None, )?; let repo: crate::repository::RepositoryInterfaceHandle = if locker_is_locked { diff --git a/crates/shirabe/src/command/show_command.rs b/crates/shirabe/src/command/show_command.rs index 4f771a8..53f3ac4 100644 --- a/crates/shirabe/src/command/show_command.rs +++ b/crates/shirabe/src/command/show_command.rs @@ -38,6 +38,7 @@ use crate::repository::FilterRepository; use crate::repository::InstalledArrayRepository; use crate::repository::InstalledRepository; use crate::repository::PlatformRepository; +use crate::repository::PlatformRepositoryHandle; use crate::repository::RepositoryFactory; use crate::repository::RepositoryInterface; use crate::repository::RepositoryInterfaceHandle; @@ -56,7 +57,7 @@ pub struct ShowCommand { pub(crate) version_parser: VersionParser, pub(crate) colors: Vec, - repository_set: Option, + repository_set: Option>>, } impl ShowCommand { @@ -201,13 +202,8 @@ impl ShowCommand { platform_overrides = p.into_iter().map(|(k, v)| (k, *v)).collect(); } } - // TODO(phase-b): PHP shares a single $platformRepo instance by reference. - // We clone the overrides and re-construct as needed because PlatformRepository - // is not Clone (PHP class semantics; Phase D will introduce Rc sharing). - let mut platform_repo = PlatformRepository::new(vec![], platform_overrides.clone())?; - let make_platform_repo = || -> anyhow::Result { - PlatformRepository::new(vec![], platform_overrides.clone()) - }; + let platform_repo = + PlatformRepositoryHandle::new(PlatformRepository::new(vec![], platform_overrides)?); let mut locked_repo: Option = None; // The single-package $package binding from PHP gets surfaced here. @@ -245,15 +241,13 @@ impl ShowCommand { single_package = Some(package.clone().into()); } else if input.borrow().get_option("platform").as_bool() == Some(true) { installed_repo = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ - RepositoryInterfaceHandle::new(make_platform_repo()?), + platform_repo.clone().into(), ])); repos = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ - RepositoryInterfaceHandle::new(make_platform_repo()?), + platform_repo.clone().into(), ])); } else if input.borrow().get_option("available").as_bool() == Some(true) { - let mut ir = InstalledRepository::new(vec![RepositoryInterfaceHandle::new( - make_platform_repo()?, - )]); + let mut ir = InstalledRepository::new(vec![platform_repo.clone().into()]); if let Some(ref composer) = composer { let composer = crate::command::composer_full(composer); repos = RepositoryInterfaceHandle::new(CompositeRepository::new( @@ -299,13 +293,13 @@ impl ShowCommand { installed_repo = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ lr_handle.clone(), local_repo, - RepositoryInterfaceHandle::new(make_platform_repo()?), + platform_repo.clone().into(), ])); locked_repo = Some(lr_handle); } else { installed_repo = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ local_repo, - RepositoryInterfaceHandle::new(make_platform_repo()?), + platform_repo.clone().into(), ])); } let mut composite_input: Vec = @@ -334,7 +328,7 @@ impl ShowCommand { names.join(", ") )); installed_repo = RepositoryInterfaceHandle::new(InstalledRepository::new(vec![ - RepositoryInterfaceHandle::new(make_platform_repo()?), + platform_repo.clone().into(), ])); let mut composite_input: Vec = vec![installed_repo.clone()]; for (_k, v) in default_repos.into_iter() { @@ -600,7 +594,7 @@ impl ShowCommand { latest_package = self.find_latest_package( package.clone().into(), composer.as_ref().unwrap(), - &mut platform_repo, + &platform_repo, input .borrow() .get_option("major-only") @@ -752,10 +746,10 @@ impl ShowCommand { for repo in RepositoryUtils::flatten_repositories(repos.clone(), false) { // TODO(phase-b): InstalledRepository needs as_repository_interface / get_repositories // wired through; placeholder classification until then. - let r#type = if Self::same_repository(&*repo.borrow(), &platform_repo) { + let r#type = if Self::same_repository(&repo, &platform_repo) { "platform" } else if let Some(ref lr) = locked_repo { - if Self::same_repository_dyn(&*repo.borrow(), &*lr.borrow()) { + if Self::same_repository(&repo, lr) { "locked" } else { "available" @@ -810,8 +804,8 @@ impl ShowCommand { } } } - if Self::same_repository(&*repo.borrow(), &platform_repo) { - for (name, p) in platform_repo.get_disabled_packages() { + if Self::same_repository(&repo, &platform_repo) { + for (name, p) in platform_repo.borrow().get_disabled_packages() { packages .entry(type_owned.clone()) .or_insert_with(IndexMap::new) @@ -872,7 +866,7 @@ impl ShowCommand { let latest = self.find_latest_package( package.clone(), composer.as_ref().unwrap(), - &mut platform_repo, + &platform_repo, show_major_only, show_minor_only, show_patch_only, @@ -2600,7 +2594,7 @@ impl ShowCommand { &mut self, package: PackageInterfaceHandle, composer: &PartialComposerHandle, - platform_repo: &mut PlatformRepository, + platform_repo: &PlatformRepositoryHandle, major_only: bool, minor_only: bool, patch_only: bool, @@ -2608,19 +2602,10 @@ impl ShowCommand { ) -> anyhow::Result> { // find the latest version allowed in this repo set let name = package.get_name(); - // TODO(phase-b): VersionSelector::new wants RepositorySet by value, but get_repository_set - // returns &mut RepositorySet. Constructing a placeholder set keeps compile clean. - let _ = self.get_repository_set(composer)?; + let repo_set = self.get_repository_set(composer)?; let composer_ref = crate::command::composer_full(composer); - let placeholder_rs = RepositorySet::new( - &composer_ref.get_package().get_minimum_stability(), - composer_ref.get_package().get_stability_flags().clone(), - Vec::new(), - IndexMap::new(), - IndexMap::new(), - IndexMap::new(), - ); - let mut version_selector = VersionSelector::new(placeholder_rs, Some(platform_repo))?; + let mut version_selector = + VersionSelector::new(repo_set, Some(&mut *platform_repo.borrow_mut()))?; let mut stability = composer_ref .get_package() .get_minimum_stability() @@ -2738,7 +2723,7 @@ impl ShowCommand { fn get_repository_set( &mut self, composer: &PartialComposerHandle, - ) -> anyhow::Result<&mut RepositorySet> { + ) -> anyhow::Result>> { let composer = crate::command::composer_full(composer); if self.repository_set.is_none() { // TODO(phase-b): RepositorySet::with_stability_and_flags — using new() placeholder. @@ -2759,10 +2744,10 @@ impl ShowCommand { .map(|r| r.clone()) .collect(), )))?; - self.repository_set = Some(rs); + self.repository_set = Some(std::rc::Rc::new(std::cell::RefCell::new(rs))); } - Ok(self.repository_set.as_mut().unwrap()) + Ok(self.repository_set.as_ref().unwrap().clone()) } fn get_relative_time(&self, release_date: &chrono::DateTime) -> String { @@ -2794,13 +2779,21 @@ impl ShowCommand { format!("{} year{} ago", years, if years > 1 { "s" } else { "" }) } - fn same_repository(_a: &dyn RepositoryInterface, _b: &PlatformRepository) -> bool { - // PHP uses object identity (===); approximation here uses pointer equality. - false + fn same_repository(a: &T, b: &U) -> bool + where + T: Into + Clone, + U: Into + Clone, + { + let a = a.clone().into(); + let b = b.clone().into(); + Self::same_repository_handle(&a, &b) } - fn same_repository_dyn(_a: &dyn RepositoryInterface, _b: &dyn RepositoryInterface) -> bool { - false + fn same_repository_handle( + a: &RepositoryInterfaceHandle, + b: &RepositoryInterfaceHandle, + ) -> bool { + a.ptr_eq(b) } } diff --git a/crates/shirabe/src/command/update_command.rs b/crates/shirabe/src/command/update_command.rs index 4064e57..b2dab1c 100644 --- a/crates/shirabe/src/command/update_command.rs +++ b/crates/shirabe/src/command/update_command.rs @@ -732,7 +732,10 @@ impl UpdateCommand { ))?; let _ = array_filter:: bool>; - VersionSelector::new(repository_set, None) + VersionSelector::new( + std::rc::Rc::new(std::cell::RefCell::new(repository_set)), + None, + ) } } -- cgit v1.3.1