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 --- crates/shirabe/src/downloader/vcs_downloader.rs | 44 ++++++++++++------------- 1 file changed, 21 insertions(+), 23 deletions(-) (limited to 'crates/shirabe/src/downloader/vcs_downloader.rs') diff --git a/crates/shirabe/src/downloader/vcs_downloader.rs b/crates/shirabe/src/downloader/vcs_downloader.rs index 7c5dfb85..26394341 100644 --- a/crates/shirabe/src/downloader/vcs_downloader.rs +++ b/crates/shirabe/src/downloader/vcs_downloader.rs @@ -28,7 +28,7 @@ pub struct VcsDownloaderBase { pub config: std::rc::Rc>, pub process: std::rc::Rc>, pub filesystem: std::rc::Rc>, - pub has_cleaned_changes: IndexMap, + pub has_cleaned_changes: std::cell::RefCell>, } impl VcsDownloaderBase { @@ -48,7 +48,7 @@ impl VcsDownloaderBase { config, process, filesystem, - has_cleaned_changes: IndexMap::new(), + has_cleaned_changes: std::cell::RefCell::new(IndexMap::new()), } } @@ -78,12 +78,11 @@ pub trait VcsDownloader: fn config(&self) -> &std::rc::Rc>; fn process(&self) -> &std::rc::Rc>; fn filesystem(&self) -> &std::rc::Rc>; - fn has_cleaned_changes(&self) -> &IndexMap; - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap; + fn has_cleaned_changes(&self) -> &std::cell::RefCell>; /// Downloads data needed to run an install/update later async fn do_download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -92,7 +91,7 @@ pub trait VcsDownloader: /// Downloads specific package into specific folder. async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -100,7 +99,7 @@ pub trait VcsDownloader: /// Updates specific package in specific folder from initial to target version. async fn do_update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -109,7 +108,7 @@ pub trait VcsDownloader: /// Fetches the commit logs between two commits fn get_commit_logs( - &mut self, + &self, from_reference: &str, to_reference: &str, path: &str, @@ -124,7 +123,7 @@ pub trait VcsDownloader: } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option, @@ -180,7 +179,7 @@ pub trait VcsDownloader: } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -189,7 +188,8 @@ pub trait VcsDownloader: if r#type == "update" { self.clean_changes(prev_package.clone().unwrap(), path, true) .await?; - self.has_cleaned_changes_mut() + self.has_cleaned_changes() + .borrow_mut() .insert(prev_package.unwrap().get_unique_name(), true); } else if r#type == "install" { self.filesystem().borrow_mut().empty_directory(path, true)?; @@ -201,7 +201,7 @@ pub trait VcsDownloader: } async fn cleanup( - &mut self, + &self, r#type: &str, _package: PackageInterfaceHandle, path: &str, @@ -212,12 +212,14 @@ pub trait VcsDownloader: .clone() .map(|p| { self.has_cleaned_changes() + .borrow() .contains_key(&p.get_unique_name()) }) .unwrap_or(false) { self.reapply_changes(path)?; - self.has_cleaned_changes_mut() + self.has_cleaned_changes() + .borrow_mut() .shift_remove(&prev_package.unwrap().get_unique_name()); } @@ -225,7 +227,7 @@ pub trait VcsDownloader: } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { @@ -284,7 +286,7 @@ pub trait VcsDownloader: } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -386,7 +388,7 @@ pub trait VcsDownloader: } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { @@ -396,11 +398,7 @@ pub trait VcsDownloader: io_interface::NORMAL, ); - let result = self - .filesystem() - .borrow_mut() - .remove_directory_async(path) - .await?; + let result = Filesystem::remove_directory_async_via(self.filesystem(), path).await?; if !result { return Err(RuntimeException { message: format!("Could not completely delete {}, aborting.", path), @@ -436,7 +434,7 @@ pub trait VcsDownloader: /// @param bool $update if true (update) the changes can be stashed and reapplied after an update, /// if false (remove) the changes should be assumed to be lost if the operation is not aborted async fn clean_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _update: bool, @@ -454,7 +452,7 @@ pub trait VcsDownloader: } /// Reapply previously stashed changes if applicable, only called after an update (regardless if successful or not) - fn reapply_changes(&mut self, _path: &str) -> anyhow::Result<()> { + fn reapply_changes(&self, _path: &str) -> anyhow::Result<()> { Ok(()) } -- cgit v1.3.1