From a02fc7d728a9973a3275a0f47604081c4439b424 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 4 Aug 2026 02:25:28 +0900 Subject: feat(plugin): activate plugins through the PHP RPC worker Implement the remainder of PluginManager::registerPackage: the plugin autoload map is built by the ported createLoader/parseAutoloads and served to the worker over the existing reverse-RPC autoloader, files entries go through a composerRequire-equivalent glue call, and already-defined classes take the upstream _composer_tmp rename/eval path. Instantiation uses the new NewObject/CallPhpMethod lanes backed by a P table in the worker; PhpPluginProxy adapts the resulting handle to PluginInterface, with $composer/$io exposed to plugin callbacks via an R table (unsupported methods stay explicit errors). Hand-written proxy stubs cover Composer, PartialComposer and the IO hierarchy, and the stub autoloader is re-prepended after loading the Composer PHP runtime so its vendor autoloader cannot shadow proxied FQCNs. FilesystemRepository::write now mirrors InstalledVersions::reload into a running worker (class_exists-guarded, so an unloaded class keeps its upstream lazy-load behavior), removing the previously undefined observation window. The installer pipeline passes the installed repository as a shared handle instead of a long-lived `&mut dyn`: plugin registration runs inside InstallationManager::execute and re-enters the same local repository through the RepositoryManager, which would panic on the RefCell re-borrow under the old shape. PluginInterface lifecycle methods now take an owned ComposerHandle (plugins retain $composer past the call) and return anyhow::Result (PHP plugin code may throw); the plugin list uses shared ownership so the identity comparison of removePlugin survives the dual storage in registeredPlugins, matching PHP reference semantics. Ports the activate/upgrade/uninstall tests of PluginInstallerTest, serialized across the shared worker process whose persistent class table is exactly what exercises the rename path. Co-Authored-By: Claude Fable 5 --- .../shirabe/src/installer/installation_manager.rs | 42 +++++++++---------- .../shirabe/src/installer/installer_interface.rs | 17 ++++---- crates/shirabe/src/installer/library_installer.rs | 12 +++--- .../shirabe/src/installer/metapackage_installer.rs | 12 +++--- crates/shirabe/src/installer/noop_installer.rs | 12 +++--- crates/shirabe/src/installer/plugin_installer.rs | 47 ++++++++++++++-------- crates/shirabe/src/installer/project_installer.rs | 10 ++--- 7 files changed, 82 insertions(+), 70 deletions(-) (limited to 'crates/shirabe/src/installer') diff --git a/crates/shirabe/src/installer/installation_manager.rs b/crates/shirabe/src/installer/installation_manager.rs index cf75469c..8d248a76 100644 --- a/crates/shirabe/src/installer/installation_manager.rs +++ b/crates/shirabe/src/installer/installation_manager.rs @@ -16,6 +16,7 @@ use crate::io::IOInterfaceImmutable; use crate::io::io_interface; use crate::package::PackageInterfaceHandle; use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use crate::util::Platform; use crate::util::r#loop::Loop; use crate::util::sync_executor; @@ -198,17 +199,18 @@ impl InstallationManager { /// Checks whether provided package is installed in one of the registered installers. pub fn is_package_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { // For testing only (ref InstallationManagerMock::isPackageInstalled). if self.mock.is_some() { - return repo.has_package(package); + return repo.borrow_mut().has_package(package); } if let Some(alias) = package.as_alias() { let alias_of: PackageInterfaceHandle = alias.get_alias_of().into(); - return Ok(repo.has_package(package)? && self.is_package_installed(repo, alias_of)?); + return Ok(repo.borrow_mut().has_package(package)? + && self.is_package_installed(repo, alias_of)?); } self.get_installer(&package.get_type())? @@ -236,7 +238,7 @@ impl InstallationManager { /// Executes solver operation. pub fn execute( &mut self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, operations: Vec, dev_mode: bool, run_scripts: bool, @@ -248,6 +250,7 @@ impl InstallationManager { // borrowed across the loop without also borrowing `&self`. if let Some(mock) = self.mock.as_mut() { let _ = (dev_mode, run_scripts, download_only); + let mut repo = repo.borrow_mut(); for operation in operations { let trace = shirabe_php_shim::strip_tags(&operation.to_string()); match operation { @@ -318,11 +321,6 @@ impl InstallationManager { let all_operations: Vec = operations.clone(); - // The concurrent operation chains share the repository; each chain borrows it only in - // synchronous sections, never across an await. - let repo_cell: std::cell::RefCell<&mut dyn InstalledRepositoryInterface> = - std::cell::RefCell::new(repo); - let result: anyhow::Result<()> = (|| -> anyhow::Result<()> { // execute operations in batches to make sure download-modifying-plugins are installed // before the other packages get downloaded @@ -363,7 +361,7 @@ impl InstallationManager { for batch_to_execute in batches { sync_executor::block_on(self.download_and_execute_batch( - &repo_cell, + repo, batch_to_execute, &mut cleanup_promises, dev_mode, @@ -394,7 +392,7 @@ impl InstallationManager { // do a last write so that we write the repository even if nothing changed // as that can trigger an update of some files like InstalledVersions.php if // running a new composer version - repo_cell.into_inner().write(dev_mode, self); + repo.borrow_mut().write(dev_mode, self); Ok(()) } @@ -402,7 +400,7 @@ impl InstallationManager { #[allow(clippy::too_many_arguments, reason = "to keep PHP signature")] async fn download_and_execute_batch( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, operations: IndexMap, cleanup_promises: &mut IndexMap< i64, @@ -544,7 +542,7 @@ impl InstallationManager { async fn execute_batch( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, operations: IndexMap, cleanup_promises: &IndexMap< i64, @@ -577,10 +575,10 @@ impl InstallationManager { } match &operation { AnyOperation::MarkAliasInstalled(op) => { - self.mark_alias_installed(&mut **repo.borrow_mut(), op)?; + self.mark_alias_installed(&mut *repo.borrow_mut(), op)?; } AnyOperation::MarkAliasUninstalled(op) => { - self.mark_alias_uninstalled(&mut **repo.borrow_mut(), op); + self.mark_alias_uninstalled(&mut *repo.borrow_mut(), op); } _ => {} } @@ -701,7 +699,7 @@ impl InstallationManager { /// Executes install operation. pub async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, operation: &InstallOperation, ) -> anyhow::Result> { let package = operation.get_package(); @@ -716,7 +714,7 @@ impl InstallationManager { /// Executes update operation. pub async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, operation: &UpdateOperation, ) -> anyhow::Result> { let initial = operation.get_initial_package().clone(); @@ -744,7 +742,7 @@ impl InstallationManager { /// Uninstalls package. pub async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, operation: &UninstallOperation, ) -> anyhow::Result> { let package = operation.get_package(); @@ -1017,13 +1015,13 @@ pub trait InstallationManagerInterface: std::fmt::Debug { fn disable_plugins(&mut self); fn is_package_installed( &mut self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result; fn ensure_binaries_presence(&mut self, package: PackageInterfaceHandle); fn execute( &mut self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, operations: Vec, dev_mode: bool, run_scripts: bool, @@ -1053,7 +1051,7 @@ impl InstallationManagerInterface for InstallationManager { fn is_package_installed( &mut self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { InstallationManager::is_package_installed(self, repo, package) @@ -1065,7 +1063,7 @@ impl InstallationManagerInterface for InstallationManager { fn execute( &mut self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, operations: Vec, dev_mode: bool, run_scripts: bool, diff --git a/crates/shirabe/src/installer/installer_interface.rs b/crates/shirabe/src/installer/installer_interface.rs index f2c0cd7c..bc464735 100644 --- a/crates/shirabe/src/installer/installer_interface.rs +++ b/crates/shirabe/src/installer/installer_interface.rs @@ -3,7 +3,7 @@ use crate::installer::BinaryPresenceInterface; use crate::installer::PluginInstaller; use crate::package::PackageInterfaceHandle; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use shirabe_php_shim::PhpMixed; #[async_trait::async_trait(?Send)] @@ -12,7 +12,7 @@ pub trait InstallerInterface: std::fmt::Debug { fn is_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result; @@ -29,25 +29,26 @@ pub trait InstallerInterface: std::fmt::Debug { prev_package: Option, ) -> anyhow::Result>; - // install/update/uninstall take the repository behind a RefCell: the concurrent operation - // chains share it, and implementations must borrow it only in synchronous sections (never - // across an await). + // install/update/uninstall take the repository as a shared handle: the concurrent operation + // chains share it (and re-entrant flows like plugin registration reach the same repository + // through RepositoryManager), so implementations must borrow it only in synchronous + // sections (never across an await). async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result>; async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result>; async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result>; diff --git a/crates/shirabe/src/installer/library_installer.rs b/crates/shirabe/src/installer/library_installer.rs index 976c5a1f..c5a9608e 100644 --- a/crates/shirabe/src/installer/library_installer.rs +++ b/crates/shirabe/src/installer/library_installer.rs @@ -8,7 +8,7 @@ use crate::installer::BinaryPresenceInterface; use crate::installer::InstallerInterface; use crate::io::IOInterface; use crate::package::PackageInterfaceHandle; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use crate::util::Filesystem; use crate::util::Platform; use crate::util::Silencer; @@ -241,10 +241,10 @@ impl InstallerInterface for LibraryInstaller { fn is_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { - if !repo.has_package(package.clone())? { + if !repo.borrow_mut().has_package(package.clone())? { return Ok(false); } @@ -315,7 +315,7 @@ impl InstallerInterface for LibraryInstaller { async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { self.initialize_vendor_dir(); @@ -346,7 +346,7 @@ impl InstallerInterface for LibraryInstaller { async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result> { @@ -380,7 +380,7 @@ impl InstallerInterface for LibraryInstaller { async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { if !repo.borrow_mut().has_package(package.clone())? { diff --git a/crates/shirabe/src/installer/metapackage_installer.rs b/crates/shirabe/src/installer/metapackage_installer.rs index 26032a6d..e2821fd3 100644 --- a/crates/shirabe/src/installer/metapackage_installer.rs +++ b/crates/shirabe/src/installer/metapackage_installer.rs @@ -8,7 +8,7 @@ use crate::io::IOInterface; use crate::io::IOInterfaceImmutable; use crate::io::io_interface; use crate::package::PackageInterfaceHandle; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use shirabe_php_shim::{InvalidArgumentException, PhpMixed}; #[derive(Debug)] @@ -30,10 +30,10 @@ impl InstallerInterface for MetapackageInstaller { fn is_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { - repo.has_package(package) + repo.borrow_mut().has_package(package) } async fn download( @@ -64,7 +64,7 @@ impl InstallerInterface for MetapackageInstaller { async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { self.io.write_error3( @@ -81,7 +81,7 @@ impl InstallerInterface for MetapackageInstaller { async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result> { @@ -111,7 +111,7 @@ impl InstallerInterface for MetapackageInstaller { async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { if !repo.borrow_mut().has_package(package.clone())? { diff --git a/crates/shirabe/src/installer/noop_installer.rs b/crates/shirabe/src/installer/noop_installer.rs index d76ad4ad..68a2b981 100644 --- a/crates/shirabe/src/installer/noop_installer.rs +++ b/crates/shirabe/src/installer/noop_installer.rs @@ -2,7 +2,7 @@ use crate::installer::InstallerInterface; use crate::package::PackageInterfaceHandle; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use shirabe_php_shim::{InvalidArgumentException, PhpMixed}; #[derive(Debug)] @@ -16,10 +16,10 @@ impl InstallerInterface for NoopInstaller { fn is_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { - repo.has_package(package) + repo.borrow_mut().has_package(package) } async fn download( @@ -50,7 +50,7 @@ impl InstallerInterface for NoopInstaller { async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { let mut repo = repo.borrow_mut(); @@ -63,7 +63,7 @@ impl InstallerInterface for NoopInstaller { async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result> { @@ -86,7 +86,7 @@ impl InstallerInterface for NoopInstaller { async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { let mut repo = repo.borrow_mut(); diff --git a/crates/shirabe/src/installer/plugin_installer.rs b/crates/shirabe/src/installer/plugin_installer.rs index c0473fd0..1e7a2636 100644 --- a/crates/shirabe/src/installer/plugin_installer.rs +++ b/crates/shirabe/src/installer/plugin_installer.rs @@ -8,7 +8,7 @@ use crate::io::IOInterface; use crate::io::IOInterfaceImmutable; use crate::package::PackageInterfaceHandle; use crate::plugin::PluginManager; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use crate::util::Filesystem; use crate::util::Platform; use shirabe_php_shim::{PhpMixed, UnexpectedValueException, empty}; @@ -46,7 +46,7 @@ impl PluginInstaller { async fn rollback_install( &self, e: anyhow::Error, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result<()> { self.inner.io.write_error(&format!( @@ -79,7 +79,7 @@ impl InstallerInterface for PluginInstaller { fn is_installed( &self, - repo: &mut dyn InstalledRepositoryInterface, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result { self.inner.is_installed(repo, package) @@ -135,43 +135,56 @@ impl InstallerInterface for PluginInstaller { async fn install( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { - self.inner.install(repo, package).await?; + self.inner.install(repo, package.clone()).await?; - // TODO(plugin): register package in plugin manager after install, rollback on failure Platform::workaround_filesystem_issues(); - // self.get_plugin_manager().register_package(package, true)?; - // On error: self.rollback_install(e, repo, package)?; + let result = + self.get_plugin_manager() + .borrow_mut() + .register_package(package.clone(), true, false); + if let Err(e) = result { + self.rollback_install(e, repo, package).await?; + } Ok(None) } async fn update( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, ) -> anyhow::Result> { - self.inner.update(repo, initial, target).await?; + self.inner + .update(repo, initial.clone(), target.clone()) + .await?; - // TODO(plugin): deactivate initial and register target in plugin manager after update, rollback on failure Platform::workaround_filesystem_issues(); - // self.get_plugin_manager().deactivate_package(initial); - // self.get_plugin_manager().register_package(target, true)?; - // On error: self.rollback_install(e, repo, target)?; + let result = (|| -> anyhow::Result<()> { + self.get_plugin_manager() + .borrow_mut() + .deactivate_package(initial)?; + self.get_plugin_manager() + .borrow_mut() + .register_package(target.clone(), true, false)?; + Ok(()) + })(); + if let Err(e) = result { + self.rollback_install(e, repo, target).await?; + } Ok(None) } async fn uninstall( &self, - repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { - // TODO(plugin): uninstall package from plugin manager self.get_plugin_manager() .borrow_mut() - .uninstall_package(package.clone()); + .uninstall_package(package.clone())?; self.inner.uninstall(repo, package).await } diff --git a/crates/shirabe/src/installer/project_installer.rs b/crates/shirabe/src/installer/project_installer.rs index 628e4252..6fee682f 100644 --- a/crates/shirabe/src/installer/project_installer.rs +++ b/crates/shirabe/src/installer/project_installer.rs @@ -3,7 +3,7 @@ use crate::downloader::DownloadManagerInterface; use crate::installer::InstallerInterface; use crate::package::PackageInterfaceHandle; -use crate::repository::InstalledRepositoryInterface; +use crate::repository::InstalledRepositoryInterfaceHandle; use crate::util::Filesystem; use shirabe_php_shim::{InvalidArgumentException, PhpMixed}; @@ -37,7 +37,7 @@ impl InstallerInterface for ProjectInstaller { fn is_installed( &self, - _repo: &mut dyn InstalledRepositoryInterface, + _repo: &InstalledRepositoryInterfaceHandle, _package: PackageInterfaceHandle, ) -> anyhow::Result { Ok(false) @@ -94,7 +94,7 @@ impl InstallerInterface for ProjectInstaller { async fn install( &self, - _repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + _repo: &InstalledRepositoryInterfaceHandle, package: PackageInterfaceHandle, ) -> anyhow::Result> { self.download_manager @@ -105,7 +105,7 @@ impl InstallerInterface for ProjectInstaller { async fn update( &self, - _repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + _repo: &InstalledRepositoryInterfaceHandle, _initial: PackageInterfaceHandle, _target: PackageInterfaceHandle, ) -> anyhow::Result> { @@ -118,7 +118,7 @@ impl InstallerInterface for ProjectInstaller { async fn uninstall( &self, - _repo: &std::cell::RefCell<&mut dyn InstalledRepositoryInterface>, + _repo: &InstalledRepositoryInterfaceHandle, _package: PackageInterfaceHandle, ) -> anyhow::Result> { Err(InvalidArgumentException { -- cgit v1.3.1