diff options
Diffstat (limited to 'crates/shirabe/src/plugin')
| -rw-r--r-- | crates/shirabe/src/plugin/php_plugin_proxy.rs | 42 | ||||
| -rw-r--r-- | crates/shirabe/src/plugin/plugin_interface.rs | 2 | ||||
| -rw-r--r-- | crates/shirabe/src/plugin/plugin_manager.rs | 125 |
3 files changed, 106 insertions, 63 deletions
diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs index 87d67ea4..09867079 100644 --- a/crates/shirabe/src/plugin/php_plugin_proxy.rs +++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs @@ -15,7 +15,7 @@ use shirabe_php_rpc::{ }; /// A Rust-side entity a PHP proxy stub points back to. -#[derive(Debug)] +#[derive(Debug, Clone)] enum RustEntity { Composer(ComposerHandle), Io(std::rc::Rc<std::cell::RefCell<dyn IOInterface>>), @@ -25,6 +25,10 @@ thread_local! { /// The R table. Entries are strong references kept for the worker's lifetime. /// TODO(plugin): GC (dropping entries on ReleaseRustHandle) is not implemented yet; /// until then entities registered here are intentionally never released. + /// TODO(plugin): thread-local while the worker and its stub intern table are + /// process-global; the session lock serializes calls, and every dispatch currently runs on + /// the thread that registered the handle, but a handle minted on one thread is invisible + /// to another. static R_TABLE: std::cell::RefCell<IndexMap<u64, RustEntity>> = std::cell::RefCell::new(IndexMap::new()); } @@ -96,6 +100,12 @@ pub(crate) fn io_stub_class( /// Looks a class up in every registered Rust-side `ClassLoader`, in registration order — the /// Rust mirror of what the PHP `spl_autoload_register` stack would do in-process. +/// +/// TODO(plugin): `ClassLoader::register` keeps one loader per vendor-dir (matching upstream's +/// `$registeredLoaders`), but the real spl stack keeps every registered loader; because the +/// `spl_autoload_register` shim is a no-op, registering a second plugin loader under the same +/// vendor-dir evicts the first one here, and a class of the earlier plugin that was never +/// loaded can become unresolvable (PHP would still find it). pub(crate) fn find_file_in_registered_loaders(class: &str) -> Option<String> { for (_vendor_dir, mut loader) in ClassLoader::get_registered_loaders() { if let Some(file) = loader.find_file(class) { @@ -122,6 +132,7 @@ impl RustMethodDispatcher for PluginRpcDispatcher { if rhandle == 0 { if method_name == "__shirabe_find_file" { let class = match args.first() { + // TODO(phase-e): lossy UTF-8; class names are bytes in PHP. Some(PluginValue::String(bytes)) => String::from_utf8_lossy(bytes).into_owned(), other => { return Err(runtime_throw(format!( @@ -139,20 +150,20 @@ impl RustMethodDispatcher for PluginRpcDispatcher { ))); } - R_TABLE.with(|table| { - let table = table.borrow(); - match table.get(&rhandle) { - Some(RustEntity::Io(io)) => dispatch_io_method(io, method_name, &args), - Some(RustEntity::Composer(_)) => { - // TODO(plugin): the Composer object graph (getConfig, getRepositoryManager, - // ...) becomes reachable over RPC later. - Err(runtime_throw(format!( - "the Composer method `{method_name}` is not available over RPC yet" - ))) - } - None => Err(runtime_throw(format!("unknown Rust handle {rhandle}"))), + // The entity is cloned out so no table borrow is held while the handler runs (a + // handler that re-enters register_*_entity would otherwise panic on the RefCell). + let entity = R_TABLE.with(|table| table.borrow().get(&rhandle).cloned()); + match entity { + Some(RustEntity::Io(io)) => dispatch_io_method(&io, method_name, &args), + Some(RustEntity::Composer(_)) => { + // TODO(plugin): the Composer object graph (getConfig, getRepositoryManager, + // ...) becomes reachable over RPC later. + Err(runtime_throw(format!( + "the Composer method `{method_name}` is not available over RPC yet" + ))) } - }) + None => Err(runtime_throw(format!("unknown Rust handle {rhandle}"))), + } } } @@ -193,6 +204,7 @@ fn decode_write_args( method_name: &str, args: &[PluginValue], ) -> Result<(Vec<String>, bool, i64), PhpThrow> { + // TODO(phase-e): lossy UTF-8; IO messages are bytes in PHP. let messages = match args.first() { Some(PluginValue::String(bytes)) => vec![String::from_utf8_lossy(bytes).into_owned()], Some(PluginValue::List(items)) => { @@ -347,7 +359,7 @@ impl PluginInterface for PhpPluginProxy { self.class.clone() } - fn as_php_plugin_proxy(&self) -> Option<&PhpPluginProxy> { + fn __as_php_plugin_proxy(&self) -> Option<&PhpPluginProxy> { Some(self) } } diff --git a/crates/shirabe/src/plugin/plugin_interface.rs b/crates/shirabe/src/plugin/plugin_interface.rs index 54fa6009..989329ea 100644 --- a/crates/shirabe/src/plugin/plugin_interface.rs +++ b/crates/shirabe/src/plugin/plugin_interface.rs @@ -44,7 +44,7 @@ pub trait PluginInterface: std::fmt::Debug { /// For testing only: recovers the PHP-backed proxy so tests can read plugin properties the /// way PHPUnit asserts `$plugins[0]->version`. - fn as_php_plugin_proxy(&self) -> Option<&crate::plugin::PhpPluginProxy> { + fn __as_php_plugin_proxy(&self) -> Option<&crate::plugin::PhpPluginProxy> { None } } diff --git a/crates/shirabe/src/plugin/plugin_manager.rs b/crates/shirabe/src/plugin/plugin_manager.rs index 6760bf20..65c4b03f 100644 --- a/crates/shirabe/src/plugin/plugin_manager.rs +++ b/crates/shirabe/src/plugin/plugin_manager.rs @@ -272,15 +272,19 @@ impl PluginManager { .map(|v| v.as_bool() == Some(true)) .unwrap_or(false); if !self.is_plugin_allowed(&package.get_name(), is_global_plugin, plugin_optional, true)? { - self.io.write_error(&format!( - "Skipped loading \"{}\" {}as it is not in config.allow-plugins", - package.get_name(), - if is_global_plugin || self.running_in_global_dir { - "(installed globally) " - } else { - "" - } - )); + self.io.write_error3( + &format!( + "Skipped loading \"{}\" {}as it is not in config.allow-plugins", + package.get_name(), + if is_global_plugin || self.running_in_global_dir { + "(installed globally) " + } else { + "" + } + ), + true, + crate::io::DEBUG, + ); return Ok(()); } @@ -298,6 +302,7 @@ impl PluginManager { Some(PhpMixed::Null) => true, Some(PhpMixed::Bool(false)) => true, Some(PhpMixed::Int(0)) => true, + Some(PhpMixed::Float(f)) if *f == 0.0 => true, Some(PhpMixed::String(s)) if s.is_empty() || s == "0" => true, Some(PhpMixed::Array(a)) => a.is_empty(), Some(PhpMixed::List(l)) => l.is_empty(), @@ -309,17 +314,19 @@ impl PluginManager { code: 0, }.into()); } - let classes: Vec<String> = if let Some(arr) = class_value.and_then(|v| v.as_list()) { - arr.iter() - .filter_map(|v| v.as_string().map(|s| s.to_string())) - .collect() - } else { - vec![ - class_value - .and_then(|v| v.as_string()) - .unwrap_or("") - .to_string(), - ] + // PHP: is_array($extra['class']) ? $extra['class'] : [$extra['class']] — an associative + // array iterates its values too, and a non-string entry reaches class_exists() where it + // raises a TypeError (an Error, not caught by the plugin installer's rollback). + let expect_class_name = |value: &PhpMixed| -> String { + value.as_string().map(|s| s.to_string()).unwrap_or_else(|| { + panic!("extra.class entries must be strings (PHP raises a TypeError): {value:?}") + }) + }; + let classes: Vec<String> = match class_value { + Some(PhpMixed::List(items)) => items.iter().map(expect_class_name).collect(), + Some(PhpMixed::Array(map)) => map.values().map(expect_class_name).collect(), + Some(other) => vec![expect_class_name(other)], + None => unreachable!("empty() above rejected a missing extra.class"), }; let composer = self.composer_full(); @@ -421,6 +428,8 @@ impl PluginManager { let path = class_loader.find_file(&class).unwrap_or_else(|| { panic!("plugin class `{class}` is already defined but has no autoloadable file") }); + // TODO(phase-e): file_get_contents is lossy UTF-8; the eval'd plugin source + // should be carried as bytes. let code = file_get_contents(&path) .unwrap_or_else(|| panic!("unable to read the plugin class file `{path}`")); let class_counter = CLASS_COUNTER.load(std::sync::atomic::Ordering::Relaxed); @@ -578,24 +587,34 @@ impl PluginManager { return Ok(()); } - let plugins = self + // PHP unsets registeredPlugins only after the loop; a deactivate() throw must leave the + // entry observable, so removal happens last here too. Plugins are cloned out per index + // (shared handles); installer entries are only referenced while calling removeInstaller. + let name = package.get_name(); + let count = self .registered_plugins - .shift_remove(&package.get_name()) - .unwrap_or_default(); - for plugin in plugins { + .get(&name) + .map(|entries| entries.len()) + .unwrap_or(0); + for index in 0..count { + let plugin = match &self.registered_plugins.get(&name).unwrap()[index] { + PluginOrInstaller::Plugin(p) => Some(p.clone()), + PluginOrInstaller::Installer(_) => None, + }; match plugin { - PluginOrInstaller::Installer(inst) => { - self.composer_full() - .borrow() - .get_installation_manager() - .borrow_mut() - .remove_installer(&*inst); - } - PluginOrInstaller::Plugin(p) => { - self.remove_plugin(&p)?; + Some(p) => self.remove_plugin(&p)?, + None => { + let composer = self.composer_full(); + let installation_manager = composer.borrow().get_installation_manager(); + if let PluginOrInstaller::Installer(inst) = + &self.registered_plugins.get(&name).unwrap()[index] + { + installation_manager.borrow_mut().remove_installer(&**inst); + } } } } + self.registered_plugins.shift_remove(&name); Ok(()) } @@ -605,25 +624,35 @@ impl PluginManager { return Ok(()); } - let plugins = self + // PHP unsets registeredPlugins only after the loop, as in deactivate_package. + let name = package.get_name(); + let count = self .registered_plugins - .shift_remove(&package.get_name()) - .unwrap_or_default(); - for plugin in plugins { + .get(&name) + .map(|entries| entries.len()) + .unwrap_or(0); + for index in 0..count { + let plugin = match &self.registered_plugins.get(&name).unwrap()[index] { + PluginOrInstaller::Plugin(p) => Some(p.clone()), + PluginOrInstaller::Installer(_) => None, + }; match plugin { - PluginOrInstaller::Installer(inst) => { - self.composer_full() - .borrow() - .get_installation_manager() - .borrow_mut() - .remove_installer(&*inst); - } - PluginOrInstaller::Plugin(p) => { + Some(p) => { self.remove_plugin(&p)?; self.uninstall_plugin(&p)?; } + None => { + let composer = self.composer_full(); + let installation_manager = composer.borrow().get_installation_manager(); + if let PluginOrInstaller::Installer(inst) = + &self.registered_plugins.get(&name).unwrap()[index] + { + installation_manager.borrow_mut().remove_installer(&**inst); + } + } } } + self.registered_plugins.shift_remove(&name); Ok(()) } @@ -895,11 +924,13 @@ impl PluginManager { global: bool, ) -> Option<String> { if !global { + // Shared borrow: this runs re-entrantly while InstallationManager::execute holds a + // shared borrow of the same manager handle. return self .composer_full() .borrow() .get_installation_manager() - .borrow_mut() + .borrow() .get_install_path(package); } @@ -909,7 +940,7 @@ impl PluginManager { .unwrap() .borrow_partial() .get_installation_manager() - .borrow_mut() + .borrow() .get_install_path(package) } |