diff options
| -rw-r--r-- | crates/shirabe/src/factory.rs | 118 | ||||
| -rw-r--r-- | crates/shirabe/src/installer/library_installer.rs | 71 | ||||
| -rw-r--r-- | crates/shirabe/src/installer/plugin_installer.rs | 5 | ||||
| -rw-r--r-- | crates/shirabe/tests/command/install_command_test.rs | 6 | ||||
| -rw-r--r-- | crates/shirabe/tests/command/remove_command_test.rs | 14 | ||||
| -rw-r--r-- | crates/shirabe/tests/installer/library_installer_test.rs | 8 |
6 files changed, 131 insertions, 91 deletions
diff --git a/crates/shirabe/src/factory.rs b/crates/shirabe/src/factory.rs index 2c27015..6d1729b 100644 --- a/crates/shirabe/src/factory.rs +++ b/crates/shirabe/src/factory.rs @@ -10,7 +10,7 @@ use shirabe_php_shim::{ InvalidArgumentException, PATHINFO_EXTENSION, PHP_EOL, PHP_OS, PhpMixed, RuntimeException, UnexpectedValueException, array_replace_recursive, class_exists, dirname, extension_loaded, file_exists, file_get_contents, file_put_contents, implode, is_dir, is_file, json_decode, - mkdir, pathinfo, realpath, rename, strpos, strtr, substr, trim, + mkdir, pathinfo, realpath, rename, rtrim, strpos, strtr, substr, trim, }; use crate::autoload::AutoloadGenerator; @@ -725,8 +725,10 @@ impl Factory { .set_archive_manager(std::rc::Rc::new(std::cell::RefCell::new(am))); } - // add installers to the manager (must happen after download manager is created since they read it out of $composer) - self.create_default_installers(&im, &composer, io.clone(), Some(&process)); + // PHP adds the installers here, but LibraryInstaller/PluginInstaller upgrade the + // Composer back-reference in their constructors, which is not yet upgradeable + // inside Rc::new_cyclic. Registration is deferred until after the Rc is built + // (see create_default_installers call below). // init locker if possible if let PartialOrFullComposer::Full(ref mut composer_full) = composer { @@ -799,6 +801,26 @@ impl Factory { return Err(e); } + // add installers to the manager (must happen after download manager is created since they + // read it out of $composer). Deferred to here because LibraryInstaller/PluginInstaller + // upgrade the Composer back-reference in their constructors, which only becomes upgradeable + // once Rc::new_cyclic has returned. + { + let im = composer.borrow().get_installation_manager(); + let process = composer + .borrow() + .get_loop() + .borrow() + .get_process_executor() + .cloned(); + self.create_default_installers( + &im, + PartialComposerWeakHandle::from_weak(std::rc::Rc::downgrade(&composer)), + io.clone(), + process.as_ref(), + ); + } + // initialize plugin manager // // PluginManager::new upgrades the Composer back-reference to read its config and locker, so @@ -1195,58 +1217,66 @@ impl Factory { fn create_default_installers( &self, - im: &std::rc::Rc<std::cell::RefCell<InstallationManager>>, - composer: &PartialOrFullComposer, + im: &std::rc::Rc<std::cell::RefCell<dyn crate::installer::InstallationManagerInterface>>, + composer: PartialComposerWeakHandle, io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, process: Option<&std::rc::Rc<std::cell::RefCell<ProcessExecutor>>>, ) { let fs = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new( process.map(std::rc::Rc::clone), ))); - let bin_dir = trim( - &composer - .get_config() - .borrow_mut() - .get_str("bin-dir") - .unwrap_or_default(), - Some("/"), - ); - let bin_compat = composer - .get_config() - .borrow_mut() - .get_str("bin-compat") - .unwrap_or_default(); - let vendor_dir = trim( - &composer - .get_config() + let (bin_dir, bin_compat, vendor_dir) = { + let composer_rc = composer + .upgrade() + .expect("Composer must outlive create_default_installers"); + let composer_ref = composer_rc.borrow_partial(); + let config = composer_ref.get_config(); + let bin_dir = rtrim( + &config.borrow_mut().get_str("bin-dir").unwrap_or_default(), + Some("/"), + ); + let bin_compat = config .borrow_mut() - .get_str("vendor-dir") - .unwrap_or_default(), - Some("/"), - ); - // PHP: $binaryInstaller = new BinaryInstaller(...); - // $im->addInstaller(new LibraryInstaller($io, $composer, null, $fs, $binaryInstaller)); - // $im->addInstaller(new PluginInstaller($io, $composer, $fs, $binaryInstaller)); - // $im->addInstaller(new MetapackageInstaller($io)); + .get_str("bin-compat") + .unwrap_or_default(); + let vendor_dir = rtrim( + &config + .borrow_mut() + .get_str("vendor-dir") + .unwrap_or_default(), + Some("/"), + ); + (bin_dir, bin_compat, vendor_dir) + }; + // The same BinaryInstaller object is shared by the Library and Plugin installers. - // TODO(phase-c): two coupled blockers. (1) Sharing one BinaryInstaller requires - // Rc<RefCell<BinaryInstaller>>; LibraryInstaller/PluginInstaller currently own a - // `BinaryInstaller` by value. (2) Both installers' constructors take a - // PartialComposerWeakHandle, but create_default_installers runs (line 737) before the - // composer is wrapped in its shared Rc, so no weak handle is obtainable here yet — - // installer registration must move after the composer Rc is established. Both are part of - // the composer construction-ordering / shared-ownership rework, so no installers are - // registered for now. - let _binary_installer = BinaryInstaller::new( + let binary_installer: std::rc::Rc< + std::cell::RefCell<dyn crate::installer::BinaryInstallerInterface>, + > = std::rc::Rc::new(std::cell::RefCell::new(BinaryInstaller::new( io.clone(), - bin_dir.clone(), - bin_compat.clone(), + bin_dir, + bin_compat, Some(fs.clone()), - Some(vendor_dir.clone()), - ); + Some(vendor_dir), + ))); - // TODO(phase-b): InstallationManager not clone-able; need shared Rc<RefCell<>> - let _ = im; + im.borrow_mut() + .add_installer(Box::new(crate::installer::LibraryInstaller::new( + io.clone(), + composer.clone(), + None, + Some(fs.clone()), + Some(binary_installer.clone()), + ))); + im.borrow_mut() + .add_installer(Box::new(crate::installer::PluginInstaller::new( + io.clone(), + composer, + Some(fs), + Some(binary_installer), + ))); + im.borrow_mut() + .add_installer(Box::new(crate::installer::MetapackageInstaller::new(io))); } fn purge_packages( diff --git a/crates/shirabe/src/installer/library_installer.rs b/crates/shirabe/src/installer/library_installer.rs index 96f38c7..26fbf0e 100644 --- a/crates/shirabe/src/installer/library_installer.rs +++ b/crates/shirabe/src/installer/library_installer.rs @@ -30,7 +30,7 @@ pub struct LibraryInstaller { pub(crate) io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, pub(crate) r#type: Option<String>, pub(crate) filesystem: std::rc::Rc<std::cell::RefCell<Filesystem>>, - pub(crate) binary_installer: Box<dyn BinaryInstallerInterface>, + pub(crate) binary_installer: std::rc::Rc<std::cell::RefCell<dyn BinaryInstallerInterface>>, } impl LibraryInstaller { @@ -40,7 +40,7 @@ impl LibraryInstaller { composer: PartialComposerWeakHandle, r#type: Option<String>, filesystem: Option<std::rc::Rc<std::cell::RefCell<Filesystem>>>, - binary_installer: Option<BinaryInstaller>, + binary_installer: Option<std::rc::Rc<std::cell::RefCell<dyn BinaryInstallerInterface>>>, ) -> Self { let composer_rc = composer .upgrade() @@ -62,31 +62,32 @@ impl LibraryInstaller { .unwrap_or_default(), Some("/"), ); - let binary_installer: Box<dyn BinaryInstallerInterface> = match binary_installer { - Some(binary_installer) => Box::new(binary_installer), - None => { - let bin_dir = rtrim( - &composer_ref + let binary_installer: std::rc::Rc<std::cell::RefCell<dyn BinaryInstallerInterface>> = + match binary_installer { + Some(binary_installer) => binary_installer, + None => { + let bin_dir = rtrim( + &composer_ref + .get_config() + .borrow_mut() + .get_str("bin-dir") + .unwrap_or_default(), + Some("/"), + ); + let bin_compat = composer_ref .get_config() .borrow_mut() - .get_str("bin-dir") - .unwrap_or_default(), - Some("/"), - ); - let bin_compat = composer_ref - .get_config() - .borrow_mut() - .get_str("bin-compat") - .unwrap_or_default(); - Box::new(BinaryInstaller::new( - io.clone(), - bin_dir, - bin_compat, - Some(filesystem.clone()), - Some(vendor_dir.clone()), - )) - } - }; + .get_str("bin-compat") + .unwrap_or_default(); + std::rc::Rc::new(std::cell::RefCell::new(BinaryInstaller::new( + io.clone(), + bin_dir, + bin_compat, + Some(filesystem.clone()), + Some(vendor_dir.clone()), + ))) + } + }; Self { composer, @@ -101,7 +102,10 @@ impl LibraryInstaller { /// For testing only: swap the binary installer for a recording double, mirroring the /// constructor injection PHPUnit performs with a mocked BinaryInstaller. - pub fn __set_binary_installer(&mut self, binary_installer: Box<dyn BinaryInstallerInterface>) { + pub fn __set_binary_installer( + &mut self, + binary_installer: std::rc::Rc<std::cell::RefCell<dyn BinaryInstallerInterface>>, + ) { self.binary_installer = binary_installer; } @@ -109,6 +113,7 @@ impl LibraryInstaller { pub fn ensure_binaries_presence(&mut self, package: PackageInterfaceHandle) { let install_path = self.get_install_path(package.clone()).unwrap(); self.binary_installer + .borrow_mut() .install_binaries(package, &install_path, false); } @@ -317,13 +322,16 @@ impl InstallerInterface for LibraryInstaller { // remove the binaries if it appears the package files are missing if !Filesystem::is_readable(&download_path) && repo.has_package(package.clone()) { - self.binary_installer.remove_binaries(package.clone()); + self.binary_installer + .borrow_mut() + .remove_binaries(package.clone()); } let _ = self.install_code(package.clone()).await?; let install_path = self.get_install_path(package.clone()).unwrap(); self.binary_installer + .borrow_mut() .install_binaries(package.clone(), &install_path, true); if !repo.has_package(package.clone()) { repo.add_package(PackageInterfaceHandle::dup(&package)); @@ -348,11 +356,14 @@ impl InstallerInterface for LibraryInstaller { self.initialize_vendor_dir(); - self.binary_installer.remove_binaries(initial.clone()); + self.binary_installer + .borrow_mut() + .remove_binaries(initial.clone()); let _ = self.update_code(initial.clone(), target.clone()).await?; let install_path = self.get_install_path(target.clone()).unwrap(); self.binary_installer + .borrow_mut() .install_binaries(target.clone(), &install_path, true); repo.remove_package(initial.clone()); if !repo.has_package(target.clone()) { @@ -378,7 +389,9 @@ impl InstallerInterface for LibraryInstaller { let _ = self.remove_code(package.clone()).await?; let download_path = self.get_package_base_path(package.clone()); - self.binary_installer.remove_binaries(package.clone()); + self.binary_installer + .borrow_mut() + .remove_binaries(package.clone()); repo.remove_package(package.clone()); if strpos(&package.get_name(), "/").is_some_and(|pos| pos != 0) { diff --git a/crates/shirabe/src/installer/plugin_installer.rs b/crates/shirabe/src/installer/plugin_installer.rs index cd4de0a..28e4e0d 100644 --- a/crates/shirabe/src/installer/plugin_installer.rs +++ b/crates/shirabe/src/installer/plugin_installer.rs @@ -1,7 +1,6 @@ //! ref: composer/src/Composer/Installer/PluginInstaller.php use crate::composer::PartialComposerWeakHandle; -use crate::installer::BinaryInstaller; use crate::installer::BinaryPresenceInterface; use crate::installer::InstallerInterface; use crate::installer::LibraryInstaller; @@ -25,7 +24,9 @@ impl PluginInstaller { io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, composer: PartialComposerWeakHandle, fs: Option<std::rc::Rc<std::cell::RefCell<Filesystem>>>, - binary_installer: Option<BinaryInstaller>, + binary_installer: Option< + std::rc::Rc<std::cell::RefCell<dyn crate::installer::BinaryInstallerInterface>>, + >, ) -> Self { Self { inner: LibraryInstaller::new( diff --git a/crates/shirabe/tests/command/install_command_test.rs b/crates/shirabe/tests/command/install_command_test.rs index c495b12..0fababa 100644 --- a/crates/shirabe/tests/command/install_command_test.rs +++ b/crates/shirabe/tests/command/install_command_test.rs @@ -87,7 +87,7 @@ fn test_install_command_errors() { #[test] #[serial] -#[ignore = "Factory::create_default_installers is a Phase-C stub that registers no installers (pending the composer construction-ordering / shared-ownership rework), so the install step fails with \"Unknown installer type: metapackage\""] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] fn test_install_from_empty_vendor() { let composer_json = serde_json::json!({ "require": { "root/req": "1.*" }, @@ -128,7 +128,7 @@ Generating autoload files", #[test] #[serial] -#[ignore = "Factory::create_default_installers is a Phase-C stub that registers no installers (pending the composer construction-ordering / shared-ownership rework), so the install step fails with \"Unknown installer type: metapackage\""] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] fn test_install_from_empty_vendor_no_dev() { let composer_json = serde_json::json!({ "require": { "root/req": "1.*" }, @@ -169,7 +169,7 @@ Generating autoload files", #[test] #[serial] -#[ignore = "Factory::create_default_installers is a Phase-C stub that registers no installers (pending the composer construction-ordering / shared-ownership rework), so the install step fails with \"Unknown installer type: metapackage\""] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] fn test_install_new_packages_with_existing_partial_vendor() { let composer_json = serde_json::json!({ "require": { diff --git a/crates/shirabe/tests/command/remove_command_test.rs b/crates/shirabe/tests/command/remove_command_test.rs index a27976a..8d76b64 100644 --- a/crates/shirabe/tests/command/remove_command_test.rs +++ b/crates/shirabe/tests/command/remove_command_test.rs @@ -84,7 +84,6 @@ fn test_exception_when_running_unused_without_lock_file() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_warning_when_removing_non_existent_package() { @@ -257,7 +256,6 @@ fn test_message_output_when_no_unused_packages_to_remove() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_remove_unused_package() { @@ -323,7 +321,7 @@ fn test_remove_unused_package() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] #[test] #[serial] fn test_remove_package_by_name() { @@ -408,7 +406,6 @@ fn test_remove_package_by_name() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_remove_package_by_name_with_dry_run() { @@ -495,7 +492,7 @@ fn test_remove_package_by_name_with_dry_run() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] #[test] #[serial] fn test_remove_allowed_plugin_package_with_no_other_allowed_plugins() { @@ -553,7 +550,6 @@ fn test_remove_allowed_plugin_package_with_no_other_allowed_plugins() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_remove_allowed_plugin_package_with_other_allowed_plugins() { @@ -605,7 +601,6 @@ fn test_remove_allowed_plugin_package_with_other_allowed_plugins() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_remove_packages_by_vendor() { @@ -689,7 +684,6 @@ fn test_remove_packages_by_vendor() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] #[test] #[serial] fn test_remove_packages_by_vendor_with_dry_run() { @@ -811,7 +805,7 @@ fn test_warning_when_removing_packages_by_vendor_from_wrong_type() { drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] #[test] #[serial] fn test_package_still_present_error_when_no_install_flag_used() { @@ -968,7 +962,7 @@ fn run_update_inherited_dependencies_flag_case( drop(tear_down); } -#[ignore = "remove runs the post-remove Installer, which fails with 'Unknown installer type: metapackage': Factory::create_default_installers (factory.rs:1234-1256) registers no installers yet, blocked on the composer construction-ordering / shared-ownership rework (installers need the composer weak handle, available only after the Rc is established)"] +#[ignore = "InstallationManager::execute_batch only awaits prepare(); the install/update/uninstall + cleanup + repo.write promise chain is still a todo!() stub, so package operations do not actually execute"] #[test] #[serial] fn test_update_inherited_dependencies_flag_is_passed_to_post_remove_installer() { diff --git a/crates/shirabe/tests/installer/library_installer_test.rs b/crates/shirabe/tests/installer/library_installer_test.rs index 42730fe..f519764 100644 --- a/crates/shirabe/tests/installer/library_installer_test.rs +++ b/crates/shirabe/tests/installer/library_installer_test.rs @@ -366,9 +366,11 @@ fn test_ensure_binaries_installed() { None, None, ); - library.__set_binary_installer(Box::new(RecordingBinaryInstaller { - calls: calls.clone(), - })); + library.__set_binary_installer(std::rc::Rc::new(std::cell::RefCell::new( + RecordingBinaryInstaller { + calls: calls.clone(), + }, + ))); let package = get_package("foo/bar", "1.0.0"); let expected_path = library.get_install_path(package.clone()).unwrap(); |