From 9b291d454b059977639db26c847af4e835cda0c3 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 18 Jul 2026 18:03:57 +0900 Subject: refactor(downloader): take &self across the downloader hierarchy Concurrent package operations call into the same downloader instances through Rc>; with &mut self methods every call holds a RefMut across its awaits, which panics with 'already mutably borrowed' the moment two operations overlap. This is groundwork for fanning out InstallationManager's download/install loops (same rework HttpDownloader/CurlDownloader already got). - DownloaderInterface/ChangeReportInterface/ArchiveDownloader/ VcsDownloader methods now take &self; as_change_report_interface returns &dyn instead of &mut dyn. - Implementors move their genuinely mutable state behind cells: FileDownloader.additional_cleanup_paths, the archive downloaders' cleanup_executed, ZipDownloader.zip_archive_object, VcsDownloaderBase.has_cleaned_changes, GitDownloader's stash/discard/ cache maps and GitUtil, SvnDownloader.cache_credentials, PerforceDownloader.perforce. FileDownloader.io gains a RefCell layer so get_local_changes can keep PHP's NullIO swap under &self. - ProcessExecutor::execute_async now returns a future that captures everything up front instead of borrowing the executor, and call sites build the future before awaiting, so no borrow on the shared executor is held while a subprocess runs. - Filesystem::remove_directory_async becomes remove_directory_async_via taking the Rc handle: the Filesystem is only borrowed for the sync head/tail, never across the rm subprocess await (sync borrow_mut users like rename/ensure_directory_exists would otherwise collide). - DownloadManager async call sites hold shared borrows only. Co-Authored-By: Claude Fable 5 --- .../shirabe/src/downloader/archive_downloader.rs | 52 +++++++++------------- 1 file changed, 22 insertions(+), 30 deletions(-) (limited to 'crates/shirabe/src/downloader/archive_downloader.rs') diff --git a/crates/shirabe/src/downloader/archive_downloader.rs b/crates/shirabe/src/downloader/archive_downloader.rs index eaa985de..9f203517 100644 --- a/crates/shirabe/src/downloader/archive_downloader.rs +++ b/crates/shirabe/src/downloader/archive_downloader.rs @@ -17,42 +17,39 @@ use std::path::{Path, PathBuf}; pub trait ArchiveDownloader { fn inner(&self) -> &FileDownloader; - fn inner_mut(&mut self) -> &mut FileDownloader; - fn cleanup_executed(&self) -> &IndexMap; - fn cleanup_executed_mut(&mut self) -> &mut IndexMap; + fn cleanup_executed(&self) -> &std::cell::RefCell>; async fn extract( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, ) -> anyhow::Result>; async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, prev_package: Option, ) -> anyhow::Result> { - self.cleanup_executed_mut() + self.cleanup_executed() + .borrow_mut() .shift_remove(&package.get_name()); - self.inner_mut() - .prepare(r#type, package, path, prev_package) - .await + self.inner().prepare(r#type, package, path, prev_package).await } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, prev_package: Option, ) -> anyhow::Result> { - self.cleanup_executed_mut().insert(package.get_name(), true); - self.inner_mut() - .cleanup(r#type, package, path, prev_package) - .await + self.cleanup_executed() + .borrow_mut() + .insert(package.get_name(), true); + self.inner().cleanup(r#type, package, path, prev_package).await } /// @inheritDoc @@ -60,13 +57,13 @@ pub trait ArchiveDownloader { /// @throws \RuntimeException /// @throws \UnexpectedValueException async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, ) -> anyhow::Result> { if output { - self.inner().io.write_error(&format!( + self.inner().io.borrow().write_error(&format!( " - {}{}", InstallOperation::format(package.clone(), false), self.get_install_operation_appendix(package.clone(), path) @@ -98,7 +95,7 @@ pub trait ArchiveDownloader { .normalize_path(&format!("{}{}", path, DIRECTORY_SEPARATOR)), ) { - self.inner_mut() + self.inner() .filesystem .borrow_mut() .empty_directory(path, true); @@ -111,15 +108,14 @@ pub trait ArchiveDownloader { } }; - self.inner_mut() - .add_cleanup_path(package.clone(), &temporary_dir); + self.inner().add_cleanup_path(package.clone(), &temporary_dir); // avoid cleaning up $path if installing in "." for eg create-project as we can not // delete the directory we are currently in on windows if !is_dir(path) || realpath(path) != Some(Platform::get_cwd(false).unwrap_or_default()) { - self.inner_mut().add_cleanup_path(package.clone(), path); + self.inner().add_cleanup_path(package.clone(), path); } - self.inner_mut() + self.inner() .filesystem .borrow_mut() .ensure_directory_exists(&temporary_dir); @@ -130,7 +126,7 @@ pub trait ArchiveDownloader { .await { Err(e) => { - install_cleanup(self.inner_mut(), package.clone(), path, &temporary_dir)?; + install_cleanup(self.inner(), package.clone(), path, &temporary_dir)?; Err(e) } Ok(_) => { @@ -191,14 +187,10 @@ pub trait ArchiveDownloader { )?; } - self.inner() - .filesystem - .borrow_mut() - .remove_directory_async(&temporary_dir) + Filesystem::remove_directory_async_via(&self.inner().filesystem, &temporary_dir) .await?; - self.inner_mut() - .remove_cleanup_path(package.clone(), &temporary_dir); - self.inner_mut().remove_cleanup_path(package, path); + self.inner().remove_cleanup_path(package.clone(), &temporary_dir); + self.inner().remove_cleanup_path(package, path); Ok(None) } @@ -216,7 +208,7 @@ pub trait ArchiveDownloader { } fn install_cleanup( - inner: &mut FileDownloader, + inner: &FileDownloader, package: PackageInterfaceHandle, path: &str, temporary_dir: &str, -- cgit v1.3.1