From 971824aa15334fd12d08ae0f441f6bf6079344c3 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sun, 7 Jun 2026 10:33:53 +0900 Subject: feat(shirabe): resolve phase-b TODOs with shared ownership Replace TODO(phase-b) placeholders (todo!() and commented-out code) with real implementations: - Share JsonFile via Rc> so JsonConfigSource and the owning command can hold the same instance (base_config_command, config_command, repository_command, require_command, create_project, remove_command, factory) - Change InstallerInterface methods (is_installed, download, prepare, cleanup, get_install_path) to &mut self so initialize_vendor_dir can run, propagated to all installer implementations - Pass io/config/filesystem/process by clone instead of moving or stubbing (auth_helper, svn_driver, curl_downloader, library_installer) - Make TransportException Clone and store it by value in VcsRepository - Clone operations in Transaction sort, root_aliases/temporary_constraints in RepositorySet::create_pool, and share CompletePackage via handle in PlatformRepository - Wire up set_option, set_requires/set_dev_requires, installation manager setters, BumpCommand::set_composer, and clean_backups/set_local_phar --- .../shirabe/src/repository/platform_repository.rs | 15 ++++------ crates/shirabe/src/repository/repository_set.rs | 33 +++++++++++++++------- crates/shirabe/src/repository/vcs/svn_driver.rs | 10 +++---- crates/shirabe/src/repository/vcs_repository.rs | 19 +++---------- 4 files changed, 37 insertions(+), 40 deletions(-) (limited to 'crates/shirabe/src/repository') diff --git a/crates/shirabe/src/repository/platform_repository.rs b/crates/shirabe/src/repository/platform_repository.rs index f194e69..a35874d 100644 --- a/crates/shirabe/src/repository/platform_repository.rs +++ b/crates/shirabe/src/repository/platform_repository.rs @@ -1641,14 +1641,14 @@ impl PlatformRepository { name: self.overrides["php"].name.clone(), version: self.overrides["php"].version.clone(), }; - let mut overrider = + let overrider = self.add_overridden_package(&php_override, Some(package.get_pretty_name()))?; let actual_text = if package.get_version() == overrider.get_version() { "same as actual".to_string() } else { format!("actual: {}", package.get_pretty_version()) }; - let current_description = overrider.get_description().unwrap_or("").to_string(); + let current_description = overrider.get_description().unwrap_or_default(); overrider.set_description(format!("{}, {}", current_description, actual_text)); return Ok(()); @@ -1662,7 +1662,7 @@ impl PlatformRepository { &mut self, r#override: &PlatformOverride, name: Option, - ) -> anyhow::Result { + ) -> anyhow::Result { let version_str = match &r#override.version { PhpMixed::String(s) => s.clone(), _ => "".to_string(), @@ -1681,14 +1681,11 @@ impl PlatformRepository { let mut extra: IndexMap = IndexMap::new(); extra.insert("config.platform".to_string(), PhpMixed::Bool(true)); package.inner.set_extra(extra); - // TODO(phase-b): CompletePackage is a PHP class (shared by ref); cannot Clone. - // The container should likely store Rc> so both the inner - // ArrayRepository and the function caller can share ownership. - let _: () = todo!("share CompletePackage via Rc between add_package and return"); + let package = CompletePackageHandle::from_complete_package(package); + self.add_package(package.clone().into())?; - #[allow(unreachable_code)] if package.get_name() == "php" { - let parts = explode(".", package.get_version()); + let parts = explode(".", &package.get_version()); let head = array_slice_strs(&parts, 0, Some(3)); *LAST_SEEN_PLATFORM_PHP.lock().unwrap() = Some(implode(".", &head)); } diff --git a/crates/shirabe/src/repository/repository_set.rs b/crates/shirabe/src/repository/repository_set.rs index 3cc4b9f..367a46a 100644 --- a/crates/shirabe/src/repository/repository_set.rs +++ b/crates/shirabe/src/repository/repository_set.rs @@ -459,7 +459,7 @@ impl RepositorySet { /// @param list|null $allowedTypes Only packages of those types are allowed if set to non-null pub fn create_pool( &mut self, - request: Request, + mut request: Request, io: std::rc::Rc>, event_dispatcher: Option>>, pool_optimizer: Option, @@ -467,17 +467,34 @@ impl RepositorySet { allowed_types: Option>, security_advisory_pool_filter: Option, ) -> Result { + let root_aliases = self + .root_aliases + .iter() + .map(|(name, versions)| { + let versions = versions + .iter() + .map(|(version, entry)| { + let mut fields = IndexMap::new(); + fields.insert("alias".to_string(), entry.alias.clone()); + fields.insert( + "alias_normalized".to_string(), + entry.alias_normalized.clone(), + ); + (version.clone(), fields) + }) + .collect(); + (name.clone(), versions) + }) + .collect(); let mut pool_builder = PoolBuilder::new( self.acceptable_stabilities.clone(), self.stability_flags.clone(), - // TODO(phase-b): clone root_aliases into PoolBuilder's expected type - todo!("self.root_aliases.clone()"), + root_aliases, self.root_references.clone(), io, event_dispatcher, pool_optimizer, - // TODO(phase-b): clone temporary_constraints - todo!("self.temporary_constraints.clone()"), + self.temporary_constraints.clone(), security_advisory_pool_filter, ); pool_builder.set_ignored_types(ignored_types); @@ -501,11 +518,7 @@ impl RepositorySet { self.locked = true; - // TODO(phase-b): pool_builder.build_pool takes owned Vec and &mut Request; revisit sharing model - pool_builder.build_pool( - todo!("share self.repositories"), - todo!("share request as &mut"), - ) + pool_builder.build_pool(self.repositories.clone(), &mut request) } /// Create a pool for dependency resolution from the packages in this repository set. diff --git a/crates/shirabe/src/repository/vcs/svn_driver.rs b/crates/shirabe/src/repository/vcs/svn_driver.rs index 3d344ec..9b2ea47 100644 --- a/crates/shirabe/src/repository/vcs/svn_driver.rs +++ b/crates/shirabe/src/repository/vcs/svn_driver.rs @@ -85,8 +85,7 @@ impl SvnDriver { } self.inner.cache = Some(Cache::new( - // TODO(phase-b): pass io by reference/clone - todo!("self.inner.io clone"), + self.inner.io.clone(), &format!( "{}/{}", self.inner @@ -551,10 +550,9 @@ impl SvnDriver { if self.util.is_none() { self.util = Some(SvnUtil::new( self.base_url.clone(), - // TODO(phase-b): clone or borrow io/config - todo!("self.inner.io clone"), - todo!("self.inner.config clone"), - Some(todo!("self.inner.process clone")), + self.inner.io.clone(), + self.inner.config.clone(), + Some(self.inner.process.clone()), )); self.util .as_mut() diff --git a/crates/shirabe/src/repository/vcs_repository.rs b/crates/shirabe/src/repository/vcs_repository.rs index e90ac62..4c2492a 100644 --- a/crates/shirabe/src/repository/vcs_repository.rs +++ b/crates/shirabe/src/repository/vcs_repository.rs @@ -70,9 +70,7 @@ pub struct VcsRepository { /// @var list empty_references: Vec, /// @var array<'tags'|'branches', array> - // TODO(phase-b): TransportException is a PHP class; uses Rc for shared ownership. - version_transport_exceptions: - IndexMap>>, + version_transport_exceptions: IndexMap>, /// @var ?EventDispatcher (preserved for plugin events) _dispatcher: Option>>, } @@ -277,7 +275,7 @@ impl VcsRepository { /// @return array<'tags'|'branches', array> pub fn get_version_transport_exceptions( &self, - ) -> &IndexMap>> { + ) -> &IndexMap> { &self.version_transport_exceptions } @@ -543,15 +541,10 @@ impl VcsRepository { })(); if let Err(e) = result { if let Some(te) = e.downcast_ref::() { - // TODO(phase-b): TransportException is a PHP class (shared by ref). We only - // have &TransportException from downcast_ref; obtaining the Rc requires the - // anyhow::Error chain to carry an Rc. For now we insert a todo!() placeholder. - let shared_te: std::rc::Rc = - todo!("share TransportException via Rc through anyhow::Error chain"); self.version_transport_exceptions .entry("tags".to_string()) .or_insert_with(IndexMap::new) - .insert(tag.clone(), shared_te); + .insert(tag.clone(), te.clone()); if te.get_code() == 404 { self.empty_references.push(identifier.clone()); } @@ -728,14 +721,10 @@ impl VcsRepository { })(); if let Err(e) = result { if let Some(te) = e.downcast_ref::() { - // TODO(phase-b): TransportException is a PHP class (shared by ref). - // See the matching tags block above; same Rc story applies. - let shared_te: std::rc::Rc = - todo!("share TransportException via Rc through anyhow::Error chain"); self.version_transport_exceptions .entry("branches".to_string()) .or_insert_with(IndexMap::new) - .insert(branch.clone(), shared_te); + .insert(branch.clone(), te.clone()); if te.get_code() == 404 { self.empty_references.push(identifier.clone()); } -- cgit v1.3.1