diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-06-07 14:15:55 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-06-07 14:15:55 +0900 |
| commit | da6f05c12d08ac96b4286664cd8205d3fee042d8 (patch) | |
| tree | 7fb50447d73d04544edb224346604f294b450f95 /crates/shirabe/src | |
| parent | 86961b16b2f5c9c26a776193934d13ff87ab7fea (diff) | |
| download | php-shirabe-da6f05c12d08ac96b4286664cd8205d3fee042d8.tar.gz php-shirabe-da6f05c12d08ac96b4286664cd8205d3fee042d8.tar.zst php-shirabe-da6f05c12d08ac96b4286664cd8205d3fee042d8.zip | |
feat(phase-c): resolve owned-argument phase-b TODOs
Replace value-by-value signature workarounds with proper ownership:
- validate_json_schema: borrow JsonFile via ValidateJsonInput<&JsonFile>,
restoring the dropped local auth file validation call
- Auditor::audit / Solver::new / create_pool: pass cloned Rc handles and
share Pool via Rc<RefCell<Pool>>; create_pool now takes &mut Request
- MarkAlias{Installed,Uninstalled}Operation: hand over AliasPackageHandle
from the alias-confirmed branch
- dispatch_installer_event: clone the base Transaction (Rc-backed
contents) and enable the PRE_OPERATIONS_EXEC dispatch
- SuggestedPackagesReporter: share between command and installer via
Rc<RefCell<>> to mirror PHP reference semantics
PrePoolCreateEvent remains a TODO: it is plugin-only and would require a
speculative Rc migration of Request whose payload is never read today.
Diffstat (limited to 'crates/shirabe/src')
| -rw-r--r-- | crates/shirabe/src/command/create_project_command.rs | 13 | ||||
| -rw-r--r-- | crates/shirabe/src/dependency_resolver/local_repo_transaction.rs | 4 | ||||
| -rw-r--r-- | crates/shirabe/src/dependency_resolver/transaction.rs | 32 | ||||
| -rw-r--r-- | crates/shirabe/src/factory.rs | 27 | ||||
| -rw-r--r-- | crates/shirabe/src/installer.rs | 111 | ||||
| -rw-r--r-- | crates/shirabe/src/repository/repository_set.rs | 6 |
6 files changed, 103 insertions, 90 deletions
diff --git a/crates/shirabe/src/command/create_project_command.rs b/crates/shirabe/src/command/create_project_command.rs index f59e1eb..bbde603 100644 --- a/crates/shirabe/src/command/create_project_command.rs +++ b/crates/shirabe/src/command/create_project_command.rs @@ -52,7 +52,8 @@ pub struct CreateProjectCommand { base_command_data: BaseCommandData, /// @var SuggestedPackagesReporter - pub(crate) suggested_packages_reporter: Option<SuggestedPackagesReporter>, + pub(crate) suggested_packages_reporter: + Option<std::rc::Rc<std::cell::RefCell<SuggestedPackagesReporter>>>, } impl CreateProjectCommand { @@ -282,7 +283,9 @@ impl CreateProjectCommand { io.borrow_mut() .load_configuration(&mut *config.borrow_mut())?; - self.suggested_packages_reporter = Some(SuggestedPackagesReporter::new(io.clone())); + self.suggested_packages_reporter = Some(std::rc::Rc::new(std::cell::RefCell::new( + SuggestedPackagesReporter::new(io.clone()), + ))); let installed_from_vcs = if let Some(package_name) = package_name.as_ref() { self.install_root_package( @@ -420,14 +423,14 @@ impl CreateProjectCommand { .set_output_progress(!no_progress); let mut installer = Installer::create(io.clone(), &composer_handle); - // TODO(phase-b): set_suggested_packages_reporter takes by value but PHP class - // means shared ownership; needs Rc<SuggestedPackagesReporter> for proper sharing. installer .set_prefer_source(prefer_source) .set_prefer_dist(prefer_dist) .set_dev_mode(install_dev_packages) .set_platform_requirement_filter(platform_requirement_filter.clone()) - .set_suggested_packages_reporter(SuggestedPackagesReporter::new(io.clone())) + .set_suggested_packages_reporter( + self.suggested_packages_reporter.as_ref().unwrap().clone(), + ) .set_optimize_autoloader( config .borrow_mut() diff --git a/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs b/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs index eb77add..e592741 100644 --- a/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs +++ b/crates/shirabe/src/dependency_resolver/local_repo_transaction.rs @@ -26,4 +26,8 @@ impl LocalRepoTransaction { pub fn get_operations(&self) -> &Vec<std::rc::Rc<dyn OperationInterface>> { self.inner.get_operations() } + + pub(crate) fn to_transaction(&self) -> Transaction { + self.inner.clone() + } } diff --git a/crates/shirabe/src/dependency_resolver/transaction.rs b/crates/shirabe/src/dependency_resolver/transaction.rs index f9ee5f0..3450815 100644 --- a/crates/shirabe/src/dependency_resolver/transaction.rs +++ b/crates/shirabe/src/dependency_resolver/transaction.rs @@ -1,6 +1,7 @@ //! ref: composer/src/Composer/DependencyResolver/Transaction.php use indexmap::IndexMap; +use indexmap::IndexSet; use shirabe_php_shim::{ PhpMixed, array_filter, array_intersect, array_keys, array_pop, array_unshift, strcmp, uasort, }; @@ -11,12 +12,13 @@ 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::AliasPackageHandle; use crate::package::Link; use crate::package::PackageInterfaceHandle; use crate::repository::PlatformRepository; /// @internal -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Transaction { /// @var OperationInterface[] pub(crate) operations: Vec<std::rc::Rc<dyn OperationInterface>>, @@ -118,13 +120,13 @@ impl Transaction { let mut present_package_map: IndexMap<String, PackageInterfaceHandle> = IndexMap::new(); let mut remove_map: IndexMap<String, PackageInterfaceHandle> = IndexMap::new(); - let mut present_alias_map: IndexMap<String, PackageInterfaceHandle> = IndexMap::new(); - let mut remove_alias_map: IndexMap<String, PackageInterfaceHandle> = IndexMap::new(); + let mut present_alias_map: IndexSet<String> = IndexSet::new(); + let mut remove_alias_map: IndexMap<String, AliasPackageHandle> = IndexMap::new(); for package in &self.present_packages { - if package.as_alias().is_some() { + if let Some(alias) = package.as_alias() { let key = format!("{}::{}", package.get_name(), package.get_version()); - present_alias_map.insert(key.clone(), package.clone()); - remove_alias_map.insert(key, package.clone()); + present_alias_map.insert(key.clone()); + remove_alias_map.insert(key, alias); } else { present_package_map.insert(package.get_name().to_string(), package.clone()); remove_map.insert(package.get_name().to_string(), package.clone()); @@ -163,15 +165,12 @@ impl Transaction { } else if !processed.contains_key(&package.ptr_id().to_string()) { processed.insert(package.ptr_id().to_string(), true); - if package.as_alias().is_some() { + if let Some(alias) = package.as_alias() { let alias_key = format!("{}::{}", package.get_name(), package.get_version()); - if present_alias_map.contains_key(&alias_key) { + if present_alias_map.contains(&alias_key) { remove_alias_map.shift_remove(&alias_key); } else { - // TODO(phase-b): MarkAliasInstalledOperation::new expects AliasPackage by value - operations.push(std::rc::Rc::new(MarkAliasInstalledOperation::new(todo!( - "package as AliasPackage by value" - )))); + operations.push(std::rc::Rc::new(MarkAliasInstalledOperation::new(alias))); } } else if let Some(source) = present_package_map.get(&package.get_name()).cloned() { // do we need to update? @@ -216,11 +215,10 @@ impl Transaction { as std::rc::Rc<dyn OperationInterface>, ); } - for (_name_version, _package) in remove_alias_map { - // TODO(phase-b): MarkAliasUninstalledOperation::new expects AliasPackage by value - operations.push(std::rc::Rc::new(MarkAliasUninstalledOperation::new(todo!( - "package as AliasPackage by value" - )))); + for (_name_version, package) in remove_alias_map { + operations.push(std::rc::Rc::new(MarkAliasUninstalledOperation::new( + package, + ))); } let operations = self.move_plugins_to_front(operations); diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs index 95511b0..c099777 100644 --- a/crates/shirabe/src/factory.rs +++ b/crates/shirabe/src/factory.rs @@ -273,14 +273,9 @@ impl Factory { crate::io::DEBUG, ); } - // TODO(phase-b): validate_json_schema takes ownership of JsonFile; recreate it Self::validate_json_schema( io.clone(), - ValidateJsonInput::File(JsonFile::new( - global_config_path.clone(), - None, - io.clone(), - )?), + ValidateJsonInput::File(&file), JsonFile::LAX_SCHEMA, None, )?; @@ -337,10 +332,9 @@ impl Factory { crate::io::DEBUG, ); } - // TODO(phase-b): validate_json_schema takes ownership; recreate JsonFile Self::validate_json_schema( io.clone(), - ValidateJsonInput::File(JsonFile::new(auth_file_path.clone(), None, io.clone())?), + ValidateJsonInput::File(&auth_file), JsonFile::AUTH_SCHEMA, None, )?; @@ -554,9 +548,12 @@ impl Factory { true, crate::io::DEBUG, ); - // TODO(phase-b): validate_json_schema/ValidateJsonInput::File expects an owned - // JsonFile (PHP class semantics share refs); needs Rc<RefCell<JsonFile>> refactor. - let _ = &local_auth_file; + Self::validate_json_schema( + Some(io.clone()), + ValidateJsonInput::File(&local_auth_file), + JsonFile::AUTH_SCHEMA, + None, + )?; let auth_read = local_auth_file.read()?; let mut wrapped: IndexMap<String, PhpMixed> = IndexMap::new(); wrapped.insert("config".to_string(), auth_read); @@ -1475,7 +1472,7 @@ impl Factory { fn validate_json_schema( io: Option<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>, - file_or_data: ValidateJsonInput, + file_or_data: ValidateJsonInput<'_>, schema: i64, source: Option<&str>, ) -> anyhow::Result<()> { @@ -1484,7 +1481,7 @@ impl Factory { } let result = match file_or_data { - ValidateJsonInput::File(mut file) => file.validate_schema(schema, None), + ValidateJsonInput::File(file) => file.validate_schema(schema, None), ValidateJsonInput::Data(data) => { let source = source.ok_or_else(|| { anyhow::anyhow!(InvalidArgumentException { @@ -1522,7 +1519,7 @@ impl Factory { } } -enum ValidateJsonInput { - File(JsonFile), +enum ValidateJsonInput<'a> { + File(&'a JsonFile), Data(PhpMixed), } diff --git a/crates/shirabe/src/installer.rs b/crates/shirabe/src/installer.rs index 1ae8e11..b770b48 100644 --- a/crates/shirabe/src/installer.rs +++ b/crates/shirabe/src/installer.rs @@ -144,7 +144,8 @@ pub struct Installer { pub(crate) update_mirrors: bool, pub(crate) update_allow_list: Option<Vec<String>>, pub(crate) update_allow_transitive_dependencies: i64, - pub(crate) suggested_packages_reporter: SuggestedPackagesReporter, + pub(crate) suggested_packages_reporter: + std::rc::Rc<std::cell::RefCell<SuggestedPackagesReporter>>, pub(crate) platform_requirement_filter: std::rc::Rc<dyn PlatformRequirementFilterInterface>, pub(crate) additional_fixed_repository: Option<crate::repository::RepositoryInterfaceHandle>, pub(crate) temporary_constraints: IndexMap<String, AnyConstraint>, @@ -172,7 +173,9 @@ impl Installer { event_dispatcher: std::rc::Rc<std::cell::RefCell<EventDispatcher>>, autoload_generator: std::rc::Rc<std::cell::RefCell<AutoloadGenerator>>, ) -> Self { - let suggested_packages_reporter = SuggestedPackagesReporter::new(io.clone()); + let suggested_packages_reporter = std::rc::Rc::new(std::cell::RefCell::new( + SuggestedPackagesReporter::new(io.clone()), + )); let platform_requirement_filter = PlatformRequirementFilterFactory::ignore_nothing(); let write_lock = config.borrow_mut().get("lock").as_bool().unwrap_or(false); @@ -357,9 +360,11 @@ impl Installer { ]); if is_fresh_install { self.suggested_packages_reporter + .borrow_mut() .add_suggestions_from_package(self.package.clone().into()); } self.suggested_packages_reporter + .borrow() .output_minimalistic(Some(&mut installed_repo), None)?; } @@ -488,7 +493,7 @@ impl Installer { gc_enable(); } - let audit_config = self.get_audit_config()?; + let audit_config = self.get_audit_config()?.clone(); if audit_config.audit { let (packages, target) = if self.update && !self.install { @@ -521,12 +526,18 @@ impl Installer { repo_set.add_repository(repo.clone())?; } - // TODO(phase-b): Auditor::audit takes owned packages/ignore lists; need cloning - // strategy. PHP shares these (copy semantics for arrays). Cloning for now is - // safe because arrays use copy semantics, but trait objects (packages) cannot - // be cloned trivially. - let audit_result: anyhow::Result<i64> = todo!(); - let _ = (&auditor, &repo_set, &packages, &audit_config); + let audit_result = auditor.audit( + &mut *self.io.borrow_mut(), + &repo_set, + packages, + &audit_config.audit_format, + true, + audit_config.ignore_list_for_audit.clone(), + &audit_config.audit_abandoned, + audit_config.ignore_severity_for_audit.clone(), + audit_config.ignore_unreachable, + audit_config.ignore_abandoned_for_audit.clone(), + ); match audit_result { Ok(n) => { return Ok(if n > 0 && self.error_on_audit { @@ -646,21 +657,21 @@ impl Installer { let _ = allow_list; } - // TODO(phase-b): create_pool takes owned Request, std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, Option<Rc<...>> - // but locally we only have refs. PHP classes (IO, dispatcher) shouldn't Clone. - let mut pool: Option<Pool> = { - let _ = (&request, &self.event_dispatcher, &policy, &repository_set); - todo!() - }; + let pool = std::rc::Rc::new(std::cell::RefCell::new(repository_set.create_pool( + &mut request, + self.io.clone(), + Some(self.event_dispatcher.clone()), + self.create_pool_optimizer(policy.clone()), + self.ignored_types.clone(), + self.allowed_types.clone(), + self.create_security_audit_pool_filter()?, + )?)); self.io.write_error("<info>Updating dependencies</info>"); // solve dependencies - // TODO(phase-b): Solver::new takes owned policy/pool/io; refactor needed - let mut solver: Option<Solver> = { - let _ = (&policy, pool.as_ref(), &self.io); - todo!() - }; + let mut solver: Option<Solver> = + Some(Solver::new(policy.clone(), pool.clone(), self.io.clone())); let mut lock_transaction: LockTransaction; let rule_set_size; match solver @@ -676,7 +687,7 @@ impl Installer { Err(e) => { // TODO(phase-b): SolverProblemsException contains dyn Rule which isn't Send+Sync // so anyhow::Error::downcast_ref can't extract it. Skipping detection. - let _ = (&repository_set, &request, pool.as_ref()); + let _ = (&repository_set, &request, &pool); return Err(e); } } @@ -685,7 +696,7 @@ impl Installer { self.io.write_error3( &format!( "Analyzed {} packages to resolve dependencies", - pool.as_ref().unwrap().get_packages().len() + pool.borrow().get_packages().len() ), true, io_interface::VERBOSE, @@ -696,8 +707,7 @@ impl Installer { io_interface::VERBOSE, ); - pool = None; - let _ = pool; + drop(pool); if lock_transaction.get_operations().is_empty() { self.io.write_error("Nothing to modify in lock file"); @@ -714,7 +724,7 @@ impl Installer { &mut lock_transaction, &platform_repo, &aliases, - &*policy, + policy.clone(), locked_repository.as_ref(), )?; if exit_code != 0 { @@ -837,6 +847,7 @@ impl Installer { // collect suggestions if let Some(io) = operation.as_install_operation() { self.suggested_packages_reporter + .borrow_mut() .add_suggestions_from_package(io.get_package()); } @@ -919,7 +930,7 @@ impl Installer { lock_transaction: &mut LockTransaction, platform_repo: &PlatformRepositoryHandle, aliases: &Vec<IndexMap<String, String>>, - policy: &dyn PolicyInterface, + policy: std::rc::Rc<dyn PolicyInterface>, locked_repository: Option<&crate::repository::LockArrayRepositoryHandle>, ) -> anyhow::Result<i64> { if self.package.get_dev_requires().is_empty() { @@ -946,13 +957,11 @@ impl Installer { self.create_request(self.fixed_root_package.clone(), platform_repo, None)?; self.require_packages_for_update(&mut request, locked_repository, false)?; - let pool = repository_set.create_pool_with_all_packages()?; + let pool = std::rc::Rc::new(std::cell::RefCell::new( + repository_set.create_pool_with_all_packages()?, + )); - // TODO(phase-b): Solver::new takes owned policy/pool/io; refactor needed - let mut solver: Option<Solver> = { - let _ = (policy, &pool, &self.io); - todo!() - }; + let mut solver: Option<Solver> = Some(Solver::new(policy, pool.clone(), self.io.clone())); let non_dev_lock_transaction: LockTransaction; match solver .as_mut() @@ -1085,18 +1094,19 @@ impl Installer { } drop(root_requires); - // TODO(phase-b): create_pool takes owned Request, std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, Option<Rc<...>> - let pool: Pool = { - let _ = (&request, &self.io, &self.event_dispatcher, &repository_set); - todo!() - }; + let pool = std::rc::Rc::new(std::cell::RefCell::new(repository_set.create_pool( + &mut request, + self.io.clone(), + Some(self.event_dispatcher.clone()), + None, + self.ignored_types.clone(), + self.allowed_types.clone(), + None, + )?)); // solve dependencies - // TODO(phase-b): Solver::new takes owned policy/pool/io - let mut solver: Option<Solver> = { - let _ = (&policy, &pool, &self.io); - todo!() - }; + let mut solver: Option<Solver> = + Some(Solver::new(policy, pool.clone(), self.io.clone())); match solver .as_mut() .unwrap() @@ -1136,13 +1146,14 @@ impl Installer { .unwrap(), )? }; - // TODO(phase-b): dispatch_installer_event takes owned Transaction, not &LocalRepoTransaction - // self.event_dispatcher.borrow_mut().dispatch_installer_event( - // InstallerEvents::PRE_OPERATIONS_EXEC, - // self.dev_mode, - // self.execute_operations, - // &local_repo_transaction, - // ); + self.event_dispatcher + .borrow_mut() + .dispatch_installer_event( + InstallerEvents::PRE_OPERATIONS_EXEC, + self.dev_mode, + self.execute_operations, + local_repo_transaction.to_transaction(), + )?; let mut installs: Vec<String> = vec![]; let mut updates: Vec<String> = vec![]; @@ -1968,7 +1979,7 @@ impl Installer { pub fn set_suggested_packages_reporter( &mut self, - suggested_packages_reporter: SuggestedPackagesReporter, + suggested_packages_reporter: std::rc::Rc<std::cell::RefCell<SuggestedPackagesReporter>>, ) -> &mut Self { self.suggested_packages_reporter = suggested_packages_reporter; diff --git a/crates/shirabe/src/repository/repository_set.rs b/crates/shirabe/src/repository/repository_set.rs index 62b25cb..aa5f128 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<string>|null $allowedTypes Only packages of those types are allowed if set to non-null pub fn create_pool( &mut self, - mut request: Request, + request: &mut Request, io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, event_dispatcher: Option<std::rc::Rc<std::cell::RefCell<EventDispatcher>>>, pool_optimizer: Option<PoolOptimizer>, @@ -518,7 +518,7 @@ impl RepositorySet { self.locked = true; - pool_builder.build_pool(self.repositories.clone(), &mut request) + pool_builder.build_pool(self.repositories.clone(), request) } /// Create a pool for dependency resolution from the packages in this repository set. @@ -625,7 +625,7 @@ impl RepositorySet { } self.create_pool( - request, + &mut request, std::rc::Rc::new(std::cell::RefCell::new(NullIO::new())), None, None, |
