diff options
30 files changed, 527 insertions, 571 deletions
diff --git a/crates/shirabe/src/command/status_command.rs b/crates/shirabe/src/command/status_command.rs index b527d5aa..29f04157 100644 --- a/crates/shirabe/src/command/status_command.rs +++ b/crates/shirabe/src/command/status_command.rs @@ -94,7 +94,7 @@ impl StatusCommand { Some(d) => d, None => continue, }; - let mut downloader = downloader_handle.borrow_mut(); + let downloader = downloader_handle.borrow(); if let Some(change_reporter) = downloader.as_change_report_interface() { if std::path::Path::new(&target_dir).is_symlink() { 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<String, bool>; - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool>; + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>>; async fn extract( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, ) -> anyhow::Result<Option<PhpMixed>>; async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, ) -> anyhow::Result<Option<PhpMixed>> { - 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<PackageInterfaceHandle>, ) -> anyhow::Result<Option<PhpMixed>> { - 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<Option<PhpMixed>> { 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, diff --git a/crates/shirabe/src/downloader/change_report_interface.rs b/crates/shirabe/src/downloader/change_report_interface.rs index ccbea491..244ba0a2 100644 --- a/crates/shirabe/src/downloader/change_report_interface.rs +++ b/crates/shirabe/src/downloader/change_report_interface.rs @@ -4,7 +4,7 @@ use crate::package::PackageInterfaceHandle; pub trait ChangeReportInterface { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>>; diff --git a/crates/shirabe/src/downloader/download_manager.rs b/crates/shirabe/src/downloader/download_manager.rs index 466570eb..7ecdfd96 100644 --- a/crates/shirabe/src/downloader/download_manager.rs +++ b/crates/shirabe/src/downloader/download_manager.rs @@ -222,7 +222,7 @@ impl DownloadManager { }; let result = match downloader - .borrow_mut() + .borrow() .download3(package.clone(), &target_dir, prev_package.clone()) .await { @@ -281,7 +281,7 @@ impl DownloadManager { let target_dir = self.normalize_target_dir(target_dir); if let Some(downloader) = self.get_downloader_for_package(package.clone())? { return downloader - .borrow_mut() + .borrow() .prepare(r#type, package, &target_dir, prev_package) .await; } @@ -304,7 +304,7 @@ impl DownloadManager { ) -> anyhow::Result<Option<PhpMixed>> { let target_dir = self.normalize_target_dir(target_dir); if let Some(downloader) = self.get_downloader_for_package(package.clone())? { - return downloader.borrow_mut().install2(package, &target_dir).await; + return downloader.borrow().install2(package, &target_dir).await; } Ok(None) @@ -338,7 +338,7 @@ impl DownloadManager { return initial_downloader .as_ref() .unwrap() - .borrow_mut() + .borrow() .remove2(initial, &target_dir) .await; } @@ -349,7 +349,7 @@ impl DownloadManager { match downloader .as_ref() .unwrap() - .borrow_mut() + .borrow() .update(initial.clone(), target.clone(), &target_dir) .await { @@ -388,7 +388,7 @@ impl DownloadManager { let _ = initial_downloader .as_ref() .unwrap() - .borrow_mut() + .borrow() .remove2(initial, &target_dir) .await?; self.install(target, &target_dir).await @@ -406,7 +406,7 @@ impl DownloadManager { ) -> anyhow::Result<Option<PhpMixed>> { let target_dir = self.normalize_target_dir(target_dir); if let Some(downloader) = self.get_downloader_for_package(package.clone())? { - return downloader.borrow_mut().remove2(package, &target_dir).await; + return downloader.borrow().remove2(package, &target_dir).await; } Ok(None) @@ -429,7 +429,7 @@ impl DownloadManager { let target_dir = self.normalize_target_dir(target_dir); if let Some(downloader) = self.get_downloader_for_package(package.clone())? { return downloader - .borrow_mut() + .borrow() .cleanup(r#type, package, &target_dir, prev_package) .await; } diff --git a/crates/shirabe/src/downloader/downloader_interface.rs b/crates/shirabe/src/downloader/downloader_interface.rs index f63ac4e8..873438e2 100644 --- a/crates/shirabe/src/downloader/downloader_interface.rs +++ b/crates/shirabe/src/downloader/downloader_interface.rs @@ -8,7 +8,7 @@ pub trait DownloaderInterface: std::fmt::Debug { fn get_installation_source(&self) -> String; async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -17,7 +17,7 @@ pub trait DownloaderInterface: std::fmt::Debug { /// Convenience for the PHP default `$output = true` overload. async fn download3( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -26,7 +26,7 @@ pub trait DownloaderInterface: std::fmt::Debug { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -34,7 +34,7 @@ pub trait DownloaderInterface: std::fmt::Debug { ) -> anyhow::Result<Option<PhpMixed>>; async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -42,7 +42,7 @@ pub trait DownloaderInterface: std::fmt::Debug { /// Convenience for the PHP default `$output = true` overload. async fn install2( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<PhpMixed>> { @@ -50,14 +50,14 @@ pub trait DownloaderInterface: std::fmt::Debug { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<PhpMixed>>; async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -65,7 +65,7 @@ pub trait DownloaderInterface: std::fmt::Debug { /// Convenience for the PHP default `$output = true` overload. async fn remove2( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<PhpMixed>> { @@ -73,16 +73,14 @@ pub trait DownloaderInterface: std::fmt::Debug { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, ) -> anyhow::Result<Option<PhpMixed>>; - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { None } diff --git a/crates/shirabe/src/downloader/file_downloader.rs b/crates/shirabe/src/downloader/file_downloader.rs index 6286918a..9742fd70 100644 --- a/crates/shirabe/src/downloader/file_downloader.rs +++ b/crates/shirabe/src/downloader/file_downloader.rs @@ -51,7 +51,10 @@ pub static RESPONSE_HEADERS: LazyLock<Mutex<IndexMap<String, Vec<String>>>> = #[derive(Debug)] pub struct FileDownloader { /// @var IOInterface - pub(crate) io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, + /// + /// Behind a RefCell so `get_local_changes` can temporarily swap in a NullIO through `&self` + /// (PHP: `$this->io = new NullIO;` ... restore), now that the downloader methods take `&self`. + pub(crate) io: std::cell::RefCell<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>, /// @var Config pub(crate) config: std::rc::Rc<std::cell::RefCell<Config>>, /// @var HttpDownloader @@ -72,7 +75,10 @@ pub struct FileDownloader { /// this write needs guarding, so it is the only field isolated behind a lock. last_cache_writes: Mutex<IndexMap<String, String>>, /// @var array<string, string[]> Map of package name to list of paths - additional_cleanup_paths: IndexMap<String, Vec<String>>, + /// + /// Behind a RefCell so add/remove_cleanup_path work through `&self` (needed since install() + /// mutates it while downloads of other packages may be in flight on sibling futures). + additional_cleanup_paths: std::cell::RefCell<IndexMap<String, Vec<String>>>, } impl FileDownloader { @@ -106,7 +112,7 @@ impl FileDownloader { }); let this = Self { - io, + io: std::cell::RefCell::new(io), config, http_downloader, event_dispatcher, @@ -114,14 +120,14 @@ impl FileDownloader { process, filesystem, last_cache_writes: Mutex::new(IndexMap::new()), - additional_cleanup_paths: IndexMap::new(), + additional_cleanup_paths: std::cell::RefCell::new(IndexMap::new()), }; if let Some(cache) = &this.cache && cache.borrow().gc_is_necessary() { // PHP: writeError('Running cache garbage collection', true, io_interface::VERY_VERBOSE) - this.io.write_error("Running cache garbage collection"); + this.io.borrow().write_error("Running cache garbage collection"); let ttl = this .config .borrow_mut() @@ -148,15 +154,13 @@ impl DownloaderInterface for FileDownloader { "dist".to_owned() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } /// @inheritDoc async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _prev_package: Option<PackageInterfaceHandle>, @@ -234,7 +238,7 @@ impl DownloaderInterface for FileDownloader { if from_cache { if output { - self.io.write_error3( + self.io.borrow().write_error3( &format!( " - Loading <info>{}</info> (<comment>{}</comment>) from cache", package.get_name(), @@ -260,7 +264,7 @@ impl DownloaderInterface for FileDownloader { } } else { if output { - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " - Downloading <info>{}</info> (<comment>{}</comment>)", package.get_name(), package.get_full_pretty_version( @@ -272,7 +276,7 @@ impl DownloaderInterface for FileDownloader { let add_copy_result = self .http_downloader - .borrow_mut() + .borrow() .add_copy(&url.processed, &file_name, package.get_transport_options()) .await; match add_copy_result { @@ -367,20 +371,20 @@ impl DownloaderInterface for FileDownloader { let code = e .downcast_ref::<TransportException>() .map_or(0, |te| te.get_code()); - if self.io.is_debug() { - self.io.write_error(&format!( + if self.io.borrow().is_debug() { + self.io.borrow().write_error(&format!( " Failed downloading {}: [{}] {}: {}", package.get_name(), get_class(&PhpMixed::Null), code, e )); - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " Trying the next URL for {}", package.get_name() )); } else { - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " Failed downloading {}, trying the next URL ({}: {})", package.get_name(), code, @@ -433,7 +437,7 @@ impl DownloaderInterface for FileDownloader { /// @inheritDoc async fn prepare( - &mut self, + &self, _type: &str, _package: PackageInterfaceHandle, _path: &str, @@ -444,7 +448,7 @@ impl DownloaderInterface for FileDownloader { /// @inheritDoc async fn cleanup( - &mut self, + &self, _type: &str, package: PackageInterfaceHandle, path: &str, @@ -477,6 +481,7 @@ impl DownloaderInterface for FileDownloader { if let Some(paths) = self .additional_cleanup_paths + .borrow() .get(&package.get_name()) .cloned() { @@ -499,13 +504,13 @@ impl DownloaderInterface for FileDownloader { /// @inheritDoc async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, ) -> anyhow::Result<Option<PhpMixed>> { if output { - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " - {}", InstallOperation::format(package.clone(), false) )); @@ -556,12 +561,12 @@ impl DownloaderInterface for FileDownloader { /// @inheritDoc async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<PhpMixed>> { - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " - {}{}", UpdateOperation::format(initial.clone(), target.clone(), false), self.get_install_operation_appendix(target.clone(), path) @@ -574,22 +579,18 @@ impl DownloaderInterface for FileDownloader { /// @inheritDoc async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, ) -> anyhow::Result<Option<PhpMixed>> { if output { - self.io.write_error(&format!( + self.io.borrow().write_error(&format!( " - {}", UninstallOperation::format(package, false) )); } - 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), @@ -606,15 +607,16 @@ impl ChangeReportInterface for FileDownloader { /// @inheritDoc /// @throws \RuntimeException fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { let prev_io = std::mem::replace( - &mut self.io, + &mut *self.io.borrow_mut(), std::rc::Rc::new(std::cell::RefCell::new(NullIO::new())), ); self.io + .borrow() .borrow_mut() .load_configuration(&mut self.config.borrow_mut())?; @@ -652,7 +654,7 @@ impl ChangeReportInterface for FileDownloader { Ok(output) })(); - self.io = prev_io; + *self.io.borrow_mut() = prev_io; let (e, output) = match result { Ok(output) => (None, output), @@ -660,7 +662,7 @@ impl ChangeReportInterface for FileDownloader { }; if let Some(err) = e { - if self.io.is_debug() { + if self.io.borrow().is_debug() { return Err(err); } @@ -712,15 +714,20 @@ impl FileDownloader { } } - pub(crate) fn add_cleanup_path(&mut self, package: PackageInterfaceHandle, path: &str) { + pub(crate) fn add_cleanup_path(&self, package: PackageInterfaceHandle, path: &str) { self.additional_cleanup_paths + .borrow_mut() .entry(package.get_name()) .or_default() .push(path.to_string()); } - pub(crate) fn remove_cleanup_path(&mut self, package: PackageInterfaceHandle, path: &str) { - if let Some(paths) = self.additional_cleanup_paths.get_mut(&package.get_name()) { + pub(crate) fn remove_cleanup_path(&self, package: PackageInterfaceHandle, path: &str) { + if let Some(paths) = self + .additional_cleanup_paths + .borrow_mut() + .get_mut(&package.get_name()) + { // PHP: array_search($path, ..., true) let idx = paths.iter().position(|p| p == path); if let Some(i) = idx { diff --git a/crates/shirabe/src/downloader/fossil_downloader.rs b/crates/shirabe/src/downloader/fossil_downloader.rs index 7f610463..9442e87c 100644 --- a/crates/shirabe/src/downloader/fossil_downloader.rs +++ b/crates/shirabe/src/downloader/fossil_downloader.rs @@ -76,16 +76,12 @@ impl VcsDownloader for FossilDownloader { &self.inner.filesystem } - fn has_cleaned_changes(&self) -> &IndexMap<String, bool> { + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.inner.has_cleaned_changes } - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.inner.has_cleaned_changes - } - async fn do_download( - &mut self, + &self, _package: PackageInterfaceHandle, _path: &str, _url: &str, @@ -95,7 +91,7 @@ impl VcsDownloader for FossilDownloader { } async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -155,7 +151,7 @@ impl VcsDownloader for FossilDownloader { } async fn do_update( - &mut self, + &self, _initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -207,7 +203,7 @@ impl VcsDownloader for FossilDownloader { } fn get_commit_logs( - &mut self, + &self, _from_reference: &str, to_reference: &str, path: &str, @@ -258,7 +254,7 @@ impl VcsDownloader for FossilDownloader { impl ChangeReportInterface for FossilDownloader { fn get_local_changes( - &mut self, + &self, _package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -291,9 +287,7 @@ impl VcsCapableDownloaderInterface for FossilDownloader { #[async_trait::async_trait(?Send)] impl DownloaderInterface for FossilDownloader { - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -308,7 +302,7 @@ impl DownloaderInterface for FossilDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -318,7 +312,7 @@ impl DownloaderInterface for FossilDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -328,7 +322,7 @@ impl DownloaderInterface for FossilDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -337,7 +331,7 @@ impl DownloaderInterface for FossilDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -346,7 +340,7 @@ impl DownloaderInterface for FossilDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -355,7 +349,7 @@ impl DownloaderInterface for FossilDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/git_downloader.rs b/crates/shirabe/src/downloader/git_downloader.rs index 52fd7b1b..7ca31e24 100644 --- a/crates/shirabe/src/downloader/git_downloader.rs +++ b/crates/shirabe/src/downloader/git_downloader.rs @@ -27,12 +27,12 @@ use shirabe_php_shim::{ pub struct GitDownloader { inner: VcsDownloaderBase, /// @var array<string, bool> - has_stashed_changes: IndexMap<String, bool>, + has_stashed_changes: std::cell::RefCell<IndexMap<String, bool>>, /// @var array<string, bool> - has_discarded_changes: IndexMap<String, bool>, - git_util: GitUtil, + has_discarded_changes: std::cell::RefCell<IndexMap<String, bool>>, + git_util: std::cell::RefCell<GitUtil>, /// @var array<int, array<string, bool>> - cached_packages: IndexMap<i64, IndexMap<String, bool>>, + cached_packages: std::cell::RefCell<IndexMap<i64, IndexMap<String, bool>>>, } impl GitDownloader { @@ -51,10 +51,10 @@ impl GitDownloader { ); Self { inner, - has_stashed_changes: IndexMap::new(), - has_discarded_changes: IndexMap::new(), - git_util, - cached_packages: IndexMap::new(), + has_stashed_changes: std::cell::RefCell::new(IndexMap::new()), + has_discarded_changes: std::cell::RefCell::new(IndexMap::new()), + git_util: std::cell::RefCell::new(git_util), + cached_packages: std::cell::RefCell::new(IndexMap::new()), } } @@ -256,7 +256,7 @@ impl GitDownloader { /// @throws \RuntimeException /// @return null|string if a string is returned, it is the commit reference that was checked out if the original could not be found pub(crate) fn update_to_commit( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, reference: &str, @@ -264,10 +264,16 @@ impl GitDownloader { ) -> anyhow::Result<Option<String>> { let force: Vec<String> = if self .has_discarded_changes + .borrow() .get(path) .copied() .unwrap_or(false) - || self.has_stashed_changes.get(path).copied().unwrap_or(false) + || self + .has_stashed_changes + .borrow() + .get(path) + .copied() + .unwrap_or(false) { vec!["-f".to_string()] } else { @@ -489,7 +495,7 @@ impl GitDownloader { .into()) } - pub(crate) fn update_origin_url(&mut self, path: &str, url: &str) { + pub(crate) fn update_origin_url(&self, path: &str, url: &str) { let mut output = String::new(); self.inner.process.borrow_mut().execute_args( &[ @@ -506,7 +512,7 @@ impl GitDownloader { self.set_push_url(path, url); } - pub(crate) fn set_push_url(&mut self, path: &str, url: &str) { + pub(crate) fn set_push_url(&self, path: &str, url: &str) { // set push url for github projects let mut match_: IndexMap<CaptureKey, String> = IndexMap::new(); if Preg::is_match3( @@ -553,7 +559,7 @@ impl GitDownloader { /// @phpstan-return PromiseInterface<void|null> /// @throws \RuntimeException - pub(crate) async fn discard_changes(&mut self, path: &str) -> anyhow::Result<Option<PhpMixed>> { + pub(crate) async fn discard_changes(&self, path: &str) -> anyhow::Result<Option<PhpMixed>> { let path = self.normalize_path(path); let mut output = String::new(); if self.inner.process.borrow_mut().execute_args( @@ -582,14 +588,14 @@ impl GitDownloader { .into()); } - self.has_discarded_changes.insert(path, true); + self.has_discarded_changes.borrow_mut().insert(path, true); Ok(None) } /// @phpstan-return PromiseInterface<void|null> /// @throws \RuntimeException - pub(crate) async fn stash_changes(&mut self, path: &str) -> anyhow::Result<Option<PhpMixed>> { + pub(crate) async fn stash_changes(&self, path: &str) -> anyhow::Result<Option<PhpMixed>> { let path = self.normalize_path(path); let mut output = String::new(); if self.inner.process.borrow_mut().execute_args( @@ -609,13 +615,13 @@ impl GitDownloader { .into()); } - self.has_stashed_changes.insert(path, true); + self.has_stashed_changes.borrow_mut().insert(path, true); Ok(None) } /// @throws \RuntimeException - pub(crate) fn view_diff(&mut self, path: &str) -> anyhow::Result<()> { + pub(crate) fn view_diff(&self, path: &str) -> anyhow::Result<()> { let path = self.normalize_path(path); let mut output = String::new(); if self.inner.process.borrow_mut().execute_args( @@ -680,7 +686,7 @@ impl GitDownloader { /// The default `VcsDownloader::clean_changes()` behavior: fail if the working copy has /// local changes. fn fail_on_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<()> { @@ -708,7 +714,7 @@ impl DvcsDownloaderInterface for GitDownloader { impl ChangeReportInterface for GitDownloader { fn get_local_changes( - &mut self, + &self, _package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -775,16 +781,12 @@ impl VcsDownloader for GitDownloader { &self.inner.filesystem } - fn has_cleaned_changes(&self) -> &IndexMap<String, bool> { + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.inner.has_cleaned_changes } - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.inner.has_cleaned_changes - } - async fn do_download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -831,7 +833,7 @@ impl VcsDownloader for GitDownloader { ); let r#ref = package.get_source_reference(); let pretty_version = package.get_pretty_version(); - if self.git_util.fetch_ref_or_sync_mirror( + if self.git_util.borrow_mut().fetch_ref_or_sync_mirror( url, &cache_path, r#ref.as_deref().unwrap_or(""), @@ -839,6 +841,7 @@ impl VcsDownloader for GitDownloader { )? && is_dir(&cache_path) { self.cached_packages + .borrow_mut() .entry(package.get_id()) .or_default() .insert(r#ref.as_deref().unwrap_or("").to_string(), true); @@ -855,7 +858,7 @@ impl VcsDownloader for GitDownloader { } async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -878,6 +881,7 @@ impl VcsDownloader for GitDownloader { let commands: Vec<Vec<String>>; let has_cached = self .cached_packages + .borrow() .get(&package.get_id()) .and_then(|m| m.get(&r#ref)) .copied() @@ -983,6 +987,7 @@ impl VcsDownloader for GitDownloader { self.inner.io.write_error3(&msg, true, io_interface::NORMAL); self.git_util + .borrow_mut() .run_commands(commands, url, Some(&path), true, ())?; let source_url = package.get_source_url(); @@ -1006,7 +1011,7 @@ impl VcsDownloader for GitDownloader { } async fn do_update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -1041,6 +1046,7 @@ impl VcsDownloader for GitDownloader { let remote_url; let has_cached = self .cached_packages + .borrow() .get(&target.get_id()) .and_then(|m| m.get(&r#ref)) .copied() @@ -1101,6 +1107,7 @@ impl VcsDownloader for GitDownloader { ]; self.git_util + .borrow_mut() .run_commands(commands, url, Some(&path), false, ())?; } @@ -1113,6 +1120,7 @@ impl VcsDownloader for GitDownloader { "%sanitizedUrl%".to_string(), ]; self.git_util + .borrow_mut() .run_commands(vec![command], url, Some(&path), false, ())?; let pretty_version = target.get_pretty_version(); @@ -1167,7 +1175,7 @@ impl VcsDownloader for GitDownloader { } async fn clean_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, update: bool, @@ -1330,15 +1338,16 @@ impl VcsDownloader for GitDownloader { Ok(None) } - fn reapply_changes(&mut self, path: &str) -> anyhow::Result<()> { + fn reapply_changes(&self, path: &str) -> anyhow::Result<()> { let path = self.normalize_path(path); if self .has_stashed_changes + .borrow() .get(&path) .copied() .unwrap_or(false) { - self.has_stashed_changes.shift_remove(&path); + self.has_stashed_changes.borrow_mut().shift_remove(&path); self.inner.io.write_error3( " <info>Re-applying stashed changes</info>", true, @@ -1362,12 +1371,12 @@ impl VcsDownloader for GitDownloader { } } - self.has_discarded_changes.shift_remove(&path); + self.has_discarded_changes.borrow_mut().shift_remove(&path); Ok(()) } fn get_commit_logs( - &mut self, + &self, from_reference: &str, to_reference: &str, path: &str, @@ -1421,9 +1430,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { Some(self) } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -1434,7 +1441,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -1444,7 +1451,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -1454,7 +1461,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -1463,7 +1470,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -1472,7 +1479,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -1481,7 +1488,7 @@ impl crate::downloader::DownloaderInterface for GitDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/gzip_downloader.rs b/crates/shirabe/src/downloader/gzip_downloader.rs index 4ea2a327..4ea8f3b9 100644 --- a/crates/shirabe/src/downloader/gzip_downloader.rs +++ b/crates/shirabe/src/downloader/gzip_downloader.rs @@ -22,7 +22,7 @@ use shirabe_php_shim::{ #[derive(Debug)] pub struct GzipDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, } impl GzipDownloader { @@ -45,7 +45,7 @@ impl GzipDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), } } @@ -69,20 +69,12 @@ impl ArchiveDownloader for GzipDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, @@ -149,7 +141,7 @@ impl ArchiveDownloader for GzipDownloader { impl ChangeReportInterface for GzipDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -163,14 +155,12 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -182,7 +172,7 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -192,7 +182,7 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -201,7 +191,7 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -210,7 +200,7 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -219,7 +209,7 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/hg_downloader.rs b/crates/shirabe/src/downloader/hg_downloader.rs index 5cc6f34e..98ba4174 100644 --- a/crates/shirabe/src/downloader/hg_downloader.rs +++ b/crates/shirabe/src/downloader/hg_downloader.rs @@ -50,16 +50,12 @@ impl VcsDownloader for HgDownloader { &self.inner.filesystem } - fn has_cleaned_changes(&self) -> &IndexMap<String, bool> { + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.inner.has_cleaned_changes } - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.inner.has_cleaned_changes - } - async fn do_download( - &mut self, + &self, _package: PackageInterfaceHandle, _path: &str, _url: &str, @@ -77,7 +73,7 @@ impl VcsDownloader for HgDownloader { } async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -131,7 +127,7 @@ impl VcsDownloader for HgDownloader { } async fn do_update( - &mut self, + &self, _initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -182,7 +178,7 @@ impl VcsDownloader for HgDownloader { } fn get_commit_logs( - &mut self, + &self, from_reference: &str, to_reference: &str, path: &str, @@ -224,7 +220,7 @@ impl VcsDownloader for HgDownloader { impl ChangeReportInterface for HgDownloader { fn get_local_changes( - &mut self, + &self, _package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -257,9 +253,7 @@ impl VcsCapableDownloaderInterface for HgDownloader { #[async_trait::async_trait(?Send)] impl DownloaderInterface for HgDownloader { - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -274,7 +268,7 @@ impl DownloaderInterface for HgDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -284,7 +278,7 @@ impl DownloaderInterface for HgDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -294,7 +288,7 @@ impl DownloaderInterface for HgDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -303,7 +297,7 @@ impl DownloaderInterface for HgDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -312,7 +306,7 @@ impl DownloaderInterface for HgDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -321,7 +315,7 @@ impl DownloaderInterface for HgDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/path_downloader.rs b/crates/shirabe/src/downloader/path_downloader.rs index 5dc51b30..b799de5a 100644 --- a/crates/shirabe/src/downloader/path_downloader.rs +++ b/crates/shirabe/src/downloader/path_downloader.rs @@ -65,7 +65,7 @@ impl PathDownloader { self.inner.config.clone(), self.inner.process.clone(), parser.clone(), - Some(self.inner.io.clone()), + Some(self.inner.io.borrow().clone()), ); let dumper = ArrayDumper::new(); @@ -209,7 +209,7 @@ impl VcsCapableDownloaderInterface for PathDownloader { impl crate::downloader::ChangeReportInterface for PathDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -223,9 +223,7 @@ impl DownloaderInterface for PathDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -236,7 +234,7 @@ impl DownloaderInterface for PathDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -298,7 +296,7 @@ impl DownloaderInterface for PathDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -310,7 +308,7 @@ impl DownloaderInterface for PathDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -331,7 +329,7 @@ impl DownloaderInterface for PathDownloader { if realpath(&path).as_deref() == Some(&real_url) { if output { let appendix = self.get_install_operation_appendix(package.clone(), &path)?; - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!( " - {}{}", InstallOperation::format(package.clone(), false), @@ -358,7 +356,7 @@ impl DownloaderInterface for PathDownloader { self.inner.filesystem.borrow_mut().remove_directory(&path); if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!(" - {}: ", InstallOperation::format(package, false)), false, io_interface::NORMAL, @@ -372,7 +370,7 @@ impl DownloaderInterface for PathDownloader { if Platform::is_windows() { // Implement symlinks as NTFS junctions on Windows if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!("Junctioning from {}", url), false, io_interface::NORMAL, @@ -386,7 +384,7 @@ impl DownloaderInterface for PathDownloader { } else { let path = path.trim_end_matches('/').to_string(); if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!("Symlinking from {}", url), false, io_interface::NORMAL, @@ -429,8 +427,8 @@ impl DownloaderInterface for PathDownloader { Err(_e) => { if allowed_strategies.contains(&Self::STRATEGY_MIRROR) { if output { - self.inner.io.write_error3("", true, io_interface::NORMAL); - self.inner.io.write_error3( + self.inner.io.borrow().write_error3("", true, io_interface::NORMAL); + self.inner.io.borrow().write_error3( " <error>Symlink failed, fallback to use mirroring!</error>", true, io_interface::NORMAL, @@ -457,7 +455,7 @@ impl DownloaderInterface for PathDownloader { let real_url = self.inner.filesystem.borrow_mut().normalize_path(&real_url); if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!( "{}Mirroring from {}", if is_fallback { " " } else { "" }, @@ -477,14 +475,14 @@ impl DownloaderInterface for PathDownloader { } if output { - self.inner.io.write_error3("", true, io_interface::NORMAL); + self.inner.io.borrow().write_error3("", true, io_interface::NORMAL); } Ok(None) } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -493,7 +491,7 @@ impl DownloaderInterface for PathDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -509,7 +507,7 @@ impl DownloaderInterface for PathDownloader { // to fail hard. if Platform::is_windows() && self.inner.filesystem.borrow_mut().is_junction(&path) { if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!( " - {}, source is still present in {}", UninstallOperation::format(package.clone(), false), @@ -520,7 +518,7 @@ impl DownloaderInterface for PathDownloader { ); } if !self.inner.filesystem.borrow_mut().remove_junction(&path)? { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!( " <warning>Could not remove junction at {} - is another process locking it?</warning>", path @@ -566,7 +564,7 @@ impl DownloaderInterface for PathDownloader { }; if fs.normalize_path(&abs_path) == fs.normalize_path(&abs_dist_url) { if output { - self.inner.io.write_error3( + self.inner.io.borrow().write_error3( &format!( " - {}, source is still present in {}", UninstallOperation::format(package.clone(), false), @@ -584,7 +582,7 @@ impl DownloaderInterface for PathDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/perforce_downloader.rs b/crates/shirabe/src/downloader/perforce_downloader.rs index 4ca9b644..2bbf9b72 100644 --- a/crates/shirabe/src/downloader/perforce_downloader.rs +++ b/crates/shirabe/src/downloader/perforce_downloader.rs @@ -20,7 +20,7 @@ use shirabe_php_shim::PhpMixed; #[derive(Debug)] pub struct PerforceDownloader { inner: VcsDownloaderBase, - pub(crate) perforce: Option<Box<dyn PerforceInterface>>, + pub(crate) perforce: std::cell::RefCell<Option<Box<dyn PerforceInterface>>>, } impl PerforceDownloader { @@ -32,7 +32,7 @@ impl PerforceDownloader { ) -> Self { Self { inner: VcsDownloaderBase::new(io, config, Some(process), Some(fs)), - perforce: None, + perforce: std::cell::RefCell::new(None), } } @@ -45,8 +45,8 @@ impl PerforceDownloader { None } - pub fn init_perforce(&mut self, package: PackageInterfaceHandle, path: String, url: String) { - if let Some(perforce) = self.perforce.as_mut() { + pub fn init_perforce(&self, package: PackageInterfaceHandle, path: String, url: String) { + if let Some(perforce) = self.perforce.borrow_mut().as_mut() { perforce.initialize_path(&path); return; } @@ -61,7 +61,7 @@ impl PerforceDownloader { } else { None }; - self.perforce = Some(Box::new(Perforce::create( + *self.perforce.borrow_mut() = Some(Box::new(Perforce::create( repo_config.unwrap_or_default(), url, path, @@ -74,8 +74,8 @@ impl PerforceDownloader { repository.get_repo_config().clone() } - pub fn set_perforce(&mut self, perforce: Box<dyn PerforceInterface>) { - self.perforce = Some(perforce); + pub fn set_perforce(&self, perforce: Box<dyn PerforceInterface>) { + *self.perforce.borrow_mut() = Some(perforce); } } @@ -96,16 +96,12 @@ impl VcsDownloader for PerforceDownloader { &self.inner.filesystem } - fn has_cleaned_changes(&self) -> &IndexMap<String, bool> { + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.inner.has_cleaned_changes } - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.inner.has_cleaned_changes - } - async fn do_download( - &mut self, + &self, _package: PackageInterfaceHandle, _path: &str, _url: &str, @@ -115,7 +111,7 @@ impl VcsDownloader for PerforceDownloader { } async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -128,21 +124,22 @@ impl VcsDownloader for PerforceDownloader { source_ref.clone().unwrap_or_default() )); self.init_perforce(package, path.to_string(), url.to_string()); - self.perforce - .as_mut() - .unwrap() - .set_stream(&source_ref.clone().unwrap_or_default()); - self.perforce.as_mut().unwrap().p4_login(); - self.perforce.as_mut().unwrap().write_p4_client_spec(); - self.perforce.as_mut().unwrap().connect_client(); - self.perforce.as_mut().unwrap().sync_code_base(label); - self.perforce.as_mut().unwrap().cleanup_client_spec(); + { + let mut perforce = self.perforce.borrow_mut(); + let perforce = perforce.as_mut().unwrap(); + perforce.set_stream(&source_ref.clone().unwrap_or_default()); + perforce.p4_login(); + perforce.write_p4_client_spec(); + perforce.connect_client(); + perforce.sync_code_base(label); + perforce.cleanup_client_spec(); + } Ok(None) } async fn do_update( - &mut self, + &self, _initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -152,13 +149,14 @@ impl VcsDownloader for PerforceDownloader { } fn get_commit_logs( - &mut self, + &self, from_reference: &str, to_reference: &str, _path: &str, ) -> anyhow::Result<String> { Ok(self .perforce + .borrow_mut() .as_mut() .unwrap() .get_commit_logs(from_reference, to_reference) @@ -172,7 +170,7 @@ impl VcsDownloader for PerforceDownloader { impl ChangeReportInterface for PerforceDownloader { fn get_local_changes( - &mut self, + &self, _package: PackageInterfaceHandle, _path: &str, ) -> anyhow::Result<Option<String>> { @@ -192,9 +190,7 @@ impl VcsCapableDownloaderInterface for PerforceDownloader { #[async_trait::async_trait(?Send)] impl DownloaderInterface for PerforceDownloader { - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -209,7 +205,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -219,7 +215,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -229,7 +225,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -238,7 +234,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -247,7 +243,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -256,7 +252,7 @@ impl DownloaderInterface for PerforceDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/phar_downloader.rs b/crates/shirabe/src/downloader/phar_downloader.rs index 491f07e5..cbbb8252 100644 --- a/crates/shirabe/src/downloader/phar_downloader.rs +++ b/crates/shirabe/src/downloader/phar_downloader.rs @@ -18,7 +18,7 @@ use shirabe_php_shim::{Phar, PhpMixed}; #[derive(Debug)] pub struct PharDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, } impl PharDownloader { @@ -41,7 +41,7 @@ impl PharDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), } } } @@ -51,20 +51,12 @@ impl ArchiveDownloader for PharDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, _package: PackageInterfaceHandle, file: &str, path: &str, @@ -83,7 +75,7 @@ impl ArchiveDownloader for PharDownloader { impl ChangeReportInterface for PharDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -97,14 +89,12 @@ impl DownloaderInterface for PharDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -116,7 +106,7 @@ impl DownloaderInterface for PharDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -126,7 +116,7 @@ impl DownloaderInterface for PharDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -135,7 +125,7 @@ impl DownloaderInterface for PharDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -144,7 +134,7 @@ impl DownloaderInterface for PharDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -153,7 +143,7 @@ impl DownloaderInterface for PharDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/rar_downloader.rs b/crates/shirabe/src/downloader/rar_downloader.rs index bf69d146..7eaa5a8c 100644 --- a/crates/shirabe/src/downloader/rar_downloader.rs +++ b/crates/shirabe/src/downloader/rar_downloader.rs @@ -21,7 +21,7 @@ use shirabe_php_shim::{ #[derive(Debug)] pub struct RarDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, } impl RarDownloader { @@ -44,7 +44,7 @@ impl RarDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), } } } @@ -54,20 +54,12 @@ impl ArchiveDownloader for RarDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, _package: PackageInterfaceHandle, file: &str, path: &str, @@ -163,7 +155,7 @@ impl ArchiveDownloader for RarDownloader { impl ChangeReportInterface for RarDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -177,14 +169,12 @@ impl crate::downloader::DownloaderInterface for RarDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -196,7 +186,7 @@ impl crate::downloader::DownloaderInterface for RarDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -206,7 +196,7 @@ impl crate::downloader::DownloaderInterface for RarDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -215,7 +205,7 @@ impl crate::downloader::DownloaderInterface for RarDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -224,7 +214,7 @@ impl crate::downloader::DownloaderInterface for RarDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -233,7 +223,7 @@ impl crate::downloader::DownloaderInterface for RarDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/svn_downloader.rs b/crates/shirabe/src/downloader/svn_downloader.rs index b2dd7f58..576a2676 100644 --- a/crates/shirabe/src/downloader/svn_downloader.rs +++ b/crates/shirabe/src/downloader/svn_downloader.rs @@ -21,7 +21,7 @@ use shirabe_php_shim::{PhpMixed, RuntimeException, is_dir, php_regex, version_co #[derive(Debug)] pub struct SvnDownloader { inner: VcsDownloaderBase, - pub(crate) cache_credentials: bool, + pub(crate) cache_credentials: std::cell::Cell<bool>, } impl SvnDownloader { @@ -33,7 +33,7 @@ impl SvnDownloader { ) -> Self { Self { inner: VcsDownloaderBase::new(io, config, Some(process), Some(fs)), - cache_credentials: true, + cache_credentials: std::cell::Cell::new(true), } } @@ -52,7 +52,7 @@ impl SvnDownloader { self.inner.config.clone(), Some(self.inner.process.clone()), ); - util.set_cache_credentials(self.cache_credentials); + util.set_cache_credentials(self.cache_credentials.get()); util.execute(command, url, cwd, path, self.inner.io.is_verbose()) .map_err(|e| { anyhow::anyhow!( @@ -87,7 +87,7 @@ impl SvnDownloader { /// The default `VcsDownloader::clean_changes()` behavior: fail if the working copy has /// local changes. fn fail_on_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<()> { @@ -120,16 +120,12 @@ impl VcsDownloader for SvnDownloader { &self.inner.filesystem } - fn has_cleaned_changes(&self) -> &IndexMap<String, bool> { + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.inner.has_cleaned_changes } - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.inner.has_cleaned_changes - } - async fn do_download( - &mut self, + &self, _package: PackageInterfaceHandle, _path: &str, url: &str, @@ -154,7 +150,7 @@ impl VcsDownloader for SvnDownloader { } async fn do_install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, url: &str, @@ -173,7 +169,7 @@ impl VcsDownloader for SvnDownloader { .get("svn-cache-credentials") .and_then(|v| v.as_bool()) { - self.cache_credentials = val; + self.cache_credentials.set(val); } } } @@ -200,7 +196,7 @@ impl VcsDownloader for SvnDownloader { } async fn do_update( - &mut self, + &self, _initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -251,7 +247,7 @@ impl VcsDownloader for SvnDownloader { } async fn clean_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, update: bool, @@ -359,7 +355,7 @@ impl VcsDownloader for SvnDownloader { } fn get_commit_logs( - &mut self, + &self, from_reference: &str, to_reference: &str, path: &str, @@ -428,7 +424,7 @@ impl VcsDownloader for SvnDownloader { self.inner.config.clone(), Some(self.inner.process.clone()), ); - util.set_cache_credentials(self.cache_credentials); + util.set_cache_credentials(self.cache_credentials.get()); util.execute_local(command.clone(), path, None, self.inner.io.is_verbose()) .map_err(|e| { RuntimeException { @@ -452,7 +448,7 @@ impl VcsDownloader for SvnDownloader { impl ChangeReportInterface for SvnDownloader { fn get_local_changes( - &mut self, + &self, _package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -489,9 +485,7 @@ impl DownloaderInterface for SvnDownloader { <Self as VcsDownloader>::get_installation_source(self) } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -502,7 +496,7 @@ impl DownloaderInterface for SvnDownloader { } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -512,7 +506,7 @@ impl DownloaderInterface for SvnDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -522,7 +516,7 @@ impl DownloaderInterface for SvnDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -531,7 +525,7 @@ impl DownloaderInterface for SvnDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -540,7 +534,7 @@ impl DownloaderInterface for SvnDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, _output: bool, @@ -549,7 +543,7 @@ impl DownloaderInterface for SvnDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/tar_downloader.rs b/crates/shirabe/src/downloader/tar_downloader.rs index 6de38c9c..dfb825e4 100644 --- a/crates/shirabe/src/downloader/tar_downloader.rs +++ b/crates/shirabe/src/downloader/tar_downloader.rs @@ -18,7 +18,7 @@ use shirabe_php_shim::{PharData, PhpMixed}; #[derive(Debug)] pub struct TarDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, } impl TarDownloader { @@ -41,7 +41,7 @@ impl TarDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), } } } @@ -51,20 +51,12 @@ impl ArchiveDownloader for TarDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, _package: PackageInterfaceHandle, file: &str, path: &str, @@ -78,7 +70,7 @@ impl ArchiveDownloader for TarDownloader { impl ChangeReportInterface for TarDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -92,14 +84,12 @@ impl DownloaderInterface for TarDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -111,7 +101,7 @@ impl DownloaderInterface for TarDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -121,7 +111,7 @@ impl DownloaderInterface for TarDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -130,7 +120,7 @@ impl DownloaderInterface for TarDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -139,7 +129,7 @@ impl DownloaderInterface for TarDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -148,7 +138,7 @@ impl DownloaderInterface for TarDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, 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<std::cell::RefCell<Config>>, pub process: std::rc::Rc<std::cell::RefCell<ProcessExecutor>>, pub filesystem: std::rc::Rc<std::cell::RefCell<Filesystem>>, - pub has_cleaned_changes: IndexMap<String, bool>, + pub has_cleaned_changes: std::cell::RefCell<IndexMap<String, bool>>, } 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<std::cell::RefCell<Config>>; fn process(&self) -> &std::rc::Rc<std::cell::RefCell<ProcessExecutor>>; fn filesystem(&self) -> &std::rc::Rc<std::cell::RefCell<Filesystem>>; - fn has_cleaned_changes(&self) -> &IndexMap<String, bool>; - fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap<String, bool>; + fn has_cleaned_changes(&self) -> &std::cell::RefCell<IndexMap<String, bool>>; /// 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<PackageInterfaceHandle>, @@ -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<Option<PhpMixed>> { @@ -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<Option<PhpMixed>> { @@ -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(()) } diff --git a/crates/shirabe/src/downloader/xz_downloader.rs b/crates/shirabe/src/downloader/xz_downloader.rs index fe35c926..d3defae5 100644 --- a/crates/shirabe/src/downloader/xz_downloader.rs +++ b/crates/shirabe/src/downloader/xz_downloader.rs @@ -18,7 +18,7 @@ use shirabe_php_shim::PhpMixed; #[derive(Debug)] pub struct XzDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, } impl XzDownloader { @@ -41,7 +41,7 @@ impl XzDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), } } } @@ -51,20 +51,12 @@ impl ArchiveDownloader for XzDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, _package: PackageInterfaceHandle, file: &str, path: &str, @@ -98,7 +90,7 @@ impl ArchiveDownloader for XzDownloader { impl ChangeReportInterface for XzDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -112,14 +104,12 @@ impl crate::downloader::DownloaderInterface for XzDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -131,7 +121,7 @@ impl crate::downloader::DownloaderInterface for XzDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -141,7 +131,7 @@ impl crate::downloader::DownloaderInterface for XzDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -150,7 +140,7 @@ impl crate::downloader::DownloaderInterface for XzDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -159,7 +149,7 @@ impl crate::downloader::DownloaderInterface for XzDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -168,7 +158,7 @@ impl crate::downloader::DownloaderInterface for XzDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/downloader/zip_downloader.rs b/crates/shirabe/src/downloader/zip_downloader.rs index c887e50b..c149505b 100644 --- a/crates/shirabe/src/downloader/zip_downloader.rs +++ b/crates/shirabe/src/downloader/zip_downloader.rs @@ -25,9 +25,9 @@ static IS_WINDOWS: Mutex<Option<bool>> = Mutex::new(None); #[derive(Debug)] pub struct ZipDownloader { inner: FileDownloader, - cleanup_executed: IndexMap<String, bool>, + cleanup_executed: std::cell::RefCell<IndexMap<String, bool>>, // @phpstan-ignore property.onlyRead (helper property that is set via reflection for testing purposes) - zip_archive_object: Option<ZipArchive>, + zip_archive_object: std::cell::RefCell<Option<ZipArchive>>, } impl ZipDownloader { @@ -52,13 +52,13 @@ impl ZipDownloader { Some(filesystem), Some(process), ), - cleanup_executed: IndexMap::new(), - zip_archive_object: None, + cleanup_executed: std::cell::RefCell::new(IndexMap::new()), + zip_archive_object: std::cell::RefCell::new(None), } } async fn extract_with_system_unzip( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, @@ -119,7 +119,7 @@ impl ZipDownloader { ) { let m1 = m.get(&CaptureKey::ByIndex(1)).cloned().unwrap_or_default(); if version_compare(&m1, "21.01", "<") { - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( " <warning>Unzipping using {} {} may result in incorrect file permissions. Install {} 21.01+ or unzip to ensure you get correct permissions.</warning>", executable, m1, executable, )); @@ -128,16 +128,18 @@ impl ZipDownloader { } } - let process_result = self - .inner - .process - .borrow() - .execute_async(&command, None) - .await; + // Build the future first so the executor borrow is released before awaiting; a borrow + // held across the await would collide with sibling extracts or sync execute() calls. + let process_future = self.inner.process.borrow().execute_async(&command, None); + let process_result = process_future.await; match process_result { Ok(mut process) => { if !process.is_successful() { - if self.cleanup_executed.contains_key(&package.get_name()) { + if self + .cleanup_executed + .borrow() + .contains_key(&package.get_name()) + { return Err(RuntimeException { message: format!( "Failed to extract {} as the installation was aborted by another package operation.", @@ -187,7 +189,7 @@ impl ZipDownloader { } async fn try_fallback( - &mut self, + &self, process_error: anyhow::Error, is_last_chance: bool, file: &str, @@ -206,15 +208,17 @@ impl ZipDownloader { if !is_file(file) { self.inner .io + .borrow() .write_error(&format!(" <warning>{}</warning>", process_error)); - self.inner.io.write_error(" <warning>This most likely is due to a custom installer plugin not handling the returned Promise from the downloader</warning>"); - self.inner.io.write_error(" <warning>See https://github.com/composer/installers/commit/5006d0c28730ade233a8f42ec31ac68fb1c5c9bb for an example fix</warning>"); + self.inner.io.borrow().write_error(" <warning>This most likely is due to a custom installer plugin not handling the returned Promise from the downloader</warning>"); + self.inner.io.borrow().write_error(" <warning>See https://github.com/composer/installers/commit/5006d0c28730ade233a8f42ec31ac68fb1c5c9bb for an example fix</warning>"); } else { self.inner .io + .borrow() .write_error(&format!(" <warning>{}</warning>", process_error)); - self.inner.io.write_error(" The archive may contain identical file names with different capitalization (which fails on case insensitive filesystems)"); - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(" The archive may contain identical file names with different capitalization (which fails on case insensitive filesystems)"); + self.inner.io.borrow().write_error(&format!( " Unzip with {} command failed, falling back to ZipArchive class", executable )); @@ -223,30 +227,30 @@ impl ZipDownloader { if Platform::get_env("GITHUB_ACTIONS").is_some() && Platform::get_env("COMPOSER_TESTS_ARE_RUNNING").is_none() { - self.inner.io.write_error(" <warning>Additional debug info, please report to https://github.com/composer/composer/issues/11148 if you see this:</warning>"); - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(" <warning>Additional debug info, please report to https://github.com/composer/composer/issues/11148 if you see this:</warning>"); + self.inner.io.borrow().write_error(&format!( "File size: {}", filesize(file).map(|s| s.to_string()).unwrap_or_default() )); - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( "File SHA1: {}", hash_file("sha1", file).unwrap_or_default() )); - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( "First 100 bytes (hex): {}", bin2hex( substr(&file_get_contents(file).unwrap_or_default(), 0, Some(100)) .as_bytes() ) )); - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( "Last 100 bytes (hex): {}", bin2hex( substr(&file_get_contents(file).unwrap_or_default(), -100, None).as_bytes() ) )); if strlen(&package.get_dist_url().unwrap_or_default()) > 0 { - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( "Origin URL: {}", self.inner.process_url( package.clone(), @@ -264,7 +268,7 @@ impl ZipDownloader { None => PhpMixed::List(vec![]), } }; - self.inner.io.write_error(&format!( + self.inner.io.borrow().write_error(&format!( "Response Headers: {}", json_encode(&headers).unwrap_or_default() )); @@ -276,12 +280,12 @@ impl ZipDownloader { } async fn extract_with_zip_archive( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, ) -> anyhow::Result<Option<PhpMixed>> { - let mut zip_archive = self.zip_archive_object.take().unwrap_or_default(); + let mut zip_archive = self.zip_archive_object.borrow_mut().take().unwrap_or_default(); let result: anyhow::Result<Option<PhpMixed>> = (|| { let retval = if !file_exists(file) || filesize(file).is_none_or(|s| s == 0) { @@ -412,8 +416,8 @@ impl ZipDownloader { /// For testing only. Mirrors the test's `setPrivateProperty('zipArchiveObject', $zipArchive, $obj)` /// reflection on the instance's `$zipArchiveObject` property. - pub fn __set_zip_archive_object(&mut self, value: Option<ZipArchive>) { - self.zip_archive_object = value; + pub fn __set_zip_archive_object(&self, value: Option<ZipArchive>) { + *self.zip_archive_object.borrow_mut() = value; } } @@ -422,20 +426,12 @@ impl ArchiveDownloader for ZipDownloader { &self.inner } - fn inner_mut(&mut self) -> &mut FileDownloader { - &mut self.inner - } - - fn cleanup_executed(&self) -> &IndexMap<String, bool> { + fn cleanup_executed(&self) -> &std::cell::RefCell<IndexMap<String, bool>> { &self.cleanup_executed } - fn cleanup_executed_mut(&mut self) -> &mut IndexMap<String, bool> { - &mut self.cleanup_executed - } - async fn extract( - &mut self, + &self, package: PackageInterfaceHandle, file: &str, path: &str, @@ -446,7 +442,7 @@ impl ArchiveDownloader for ZipDownloader { impl ChangeReportInterface for ZipDownloader { fn get_local_changes( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<String>> { @@ -460,14 +456,12 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { self.inner.get_installation_source() } - fn as_change_report_interface( - &mut self, - ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { Some(self) } async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, @@ -588,13 +582,13 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { if !is_windows_guard.unwrap() && unzip_commands_empty { if proc_open_missing { - self.inner.io.write_error("<warning>proc_open is disabled so 'unzip' and '7z' commands cannot be used, zip files are being unpacked using the PHP zip extension.</warning>"); - self.inner.io.write_error("<warning>This may cause invalid reports of corrupted archives. Besides, any UNIX permissions (e.g. executable) defined in the archives will be lost.</warning>"); - self.inner.io.write_error("<warning>Enabling proc_open and installing 'unzip' or '7z' (21.01+) may remediate them.</warning>"); + self.inner.io.borrow().write_error("<warning>proc_open is disabled so 'unzip' and '7z' commands cannot be used, zip files are being unpacked using the PHP zip extension.</warning>"); + self.inner.io.borrow().write_error("<warning>This may cause invalid reports of corrupted archives. Besides, any UNIX permissions (e.g. executable) defined in the archives will be lost.</warning>"); + self.inner.io.borrow().write_error("<warning>Enabling proc_open and installing 'unzip' or '7z' (21.01+) may remediate them.</warning>"); } else { - self.inner.io.write_error("<warning>As there is no 'unzip' nor '7z' command installed zip files are being unpacked using the PHP zip extension.</warning>"); - self.inner.io.write_error("<warning>This may cause invalid reports of corrupted archives. Besides, any UNIX permissions (e.g. executable) defined in the archives will be lost.</warning>"); - self.inner.io.write_error("<warning>Installing 'unzip' or '7z' (21.01+) may remediate them.</warning>"); + self.inner.io.borrow().write_error("<warning>As there is no 'unzip' nor '7z' command installed zip files are being unpacked using the PHP zip extension.</warning>"); + self.inner.io.borrow().write_error("<warning>This may cause invalid reports of corrupted archives. Besides, any UNIX permissions (e.g. executable) defined in the archives will be lost.</warning>"); + self.inner.io.borrow().write_error("<warning>Installing 'unzip' or '7z' (21.01+) may remediate them.</warning>"); } } } @@ -606,7 +600,7 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { } async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, @@ -616,7 +610,7 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { } async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -625,7 +619,7 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { } async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, @@ -634,7 +628,7 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { } async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, @@ -643,7 +637,7 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { } async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/src/package/version/version_guesser.rs b/crates/shirabe/src/package/version/version_guesser.rs index f2a7c3ce..349f08e1 100644 --- a/crates/shirabe/src/package/version/version_guesser.rs +++ b/crates/shirabe/src/package/version/version_guesser.rs @@ -576,9 +576,9 @@ impl VersionGuesser { }, &scm_cmdline, ); - let mut process = sync_executor::block_on( - self.process.borrow().execute_async(&cmd_line, Some(path)), - )?; + let process_future = + self.process.borrow().execute_async(&cmd_line, Some(path)); + let mut process = sync_executor::block_on(process_future)?; if !process.is_successful() { continue; } diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs index e12d7fee..f3cb2e63 100644 --- a/crates/shirabe/src/util/filesystem.rs +++ b/crates/shirabe/src/util/filesystem.rs @@ -163,38 +163,48 @@ impl Filesystem { /// /// Uses the process component if proc_open is enabled on the PHP /// installation. - pub async fn remove_directory_async(&mut self, directory: &str) -> anyhow::Result<bool> { - if let Some(mock) = self.mock.as_mut() - && let Some(result) = mock.remove_directory_async_result - { - mock.remove_directory_async_calls += 1; - return Ok(result); - } + /// + /// Takes the shared handle instead of `&mut self`: the Filesystem is borrowed only for the + /// synchronous head and tail, never across the subprocess await, so sibling futures can keep + /// using the same `Rc<RefCell<Filesystem>>` while the removal runs. + pub async fn remove_directory_async_via( + this: &std::rc::Rc<std::cell::RefCell<Filesystem>>, + directory: &str, + ) -> anyhow::Result<bool> { + let (process_executor, cmd) = { + let mut fs = this.borrow_mut(); - let edge_case_result = self.remove_edge_cases(directory, true)?; - if let Some(r) = edge_case_result { - return Ok(r); - } + if let Some(mock) = fs.mock.as_mut() + && let Some(result) = mock.remove_directory_async_result + { + mock.remove_directory_async_calls += 1; + return Ok(result); + } - let cmd: Vec<String> = if Platform::is_windows() { - vec![ - "rmdir".to_string(), - "/S".to_string(), - "/Q".to_string(), - Platform::realpath(directory), - ] - } else { - vec!["rm".to_string(), "-rf".to_string(), directory.to_string()] + let edge_case_result = fs.remove_edge_cases(directory, true)?; + if let Some(r) = edge_case_result { + return Ok(r); + } + + let cmd: Vec<String> = if Platform::is_windows() { + vec![ + "rmdir".to_string(), + "/S".to_string(), + "/Q".to_string(), + Platform::realpath(directory), + ] + } else { + vec!["rm".to_string(), "-rf".to_string(), directory.to_string()] + }; + + (fs.get_process_handle(), cmd) }; - let process_executor = self.get_process_handle(); - let mut process = process_executor - .borrow() - .execute_async( - PhpMixed::List(cmd.iter().map(|s| PhpMixed::String(s.clone())).collect()), - None, - ) - .await?; + let process_future = process_executor.borrow().execute_async( + PhpMixed::List(cmd.iter().map(|s| PhpMixed::String(s.clone())).collect()), + None, + ); + let mut process = process_future.await?; // clear stat cache because external processes aren't tracked by the php stat cache clearstatcache2(false, ""); @@ -203,7 +213,7 @@ impl Filesystem { return Ok(true); } - self.remove_directory_php(directory) + this.borrow_mut().remove_directory_php(directory) } /// Returns null when no edge case was hit. Otherwise a bool whether removal was successful diff --git a/crates/shirabe/src/util/process_executor.rs b/crates/shirabe/src/util/process_executor.rs index 767e6a00..ab4768a4 100644 --- a/crates/shirabe/src/util/process_executor.rs +++ b/crates/shirabe/src/util/process_executor.rs @@ -548,9 +548,16 @@ impl ProcessExecutor { /// starts a process on the commandline in async mode /// - /// `&self` so that concurrent calls through the same `Rc<RefCell<ProcessExecutor>>` can - /// coexist (shared borrows); the max_jobs throttle is enforced by the semaphore. - pub async fn execute_async<C>(&self, command: C, cwd: Option<&str>) -> anyhow::Result<Process> + /// Returns a future that does NOT borrow the executor: everything it needs is captured up + /// front, so callers can drop their `Ref`/`RefMut` on the shared `Rc<RefCell<ProcessExecutor>>` + /// before awaiting (`let fut = pe.borrow().execute_async(...); fut.await`). Holding a borrow + /// across the await would panic as soon as a sibling future or a sync `execute()` call touches + /// the same executor. The max_jobs throttle is enforced by the semaphore. + pub fn execute_async<C>( + &self, + command: C, + cwd: Option<&str>, + ) -> std::pin::Pin<Box<dyn std::future::Future<Output = anyhow::Result<Process>>>> where C: IntoExecCommand, { @@ -563,64 +570,70 @@ impl ProcessExecutor { // returning a misleading Process. todo!("ProcessExecutorMock async path needs a Process mock seam in external-packages"); } - if !self.allow_async { - return Err(LogicException { - message: "You must use the ProcessExecutor instance which is part of a Composer\\Loop instance to be able to run async processes".to_string(), - code: 0, + let allow_async = self.allow_async; + let semaphore = self.semaphore.clone(); + let io = self.io.clone(); + let cwd = cwd.map(ToOwned::to_owned); + + Box::pin(async move { + if !allow_async { + return Err(LogicException { + message: "You must use the ProcessExecutor instance which is part of a Composer\\Loop instance to be able to run async processes".to_string(), + code: 0, + } + .into()); } - .into()); - } - // PHP queues the job and only startJob()s it once runningJobs < maxJobs; the permit is the - // equivalent gate, so everything below (including the "Executing async command" debug - // line PHP prints from startJob) happens only once a slot is free. - let semaphore = self.semaphore.clone(); - let _permit = semaphore - .acquire() - .await - .expect("the semaphore is never closed"); + // PHP queues the job and only startJob()s it once runningJobs < maxJobs; the permit is + // the equivalent gate, so everything below (including the "Executing async command" + // debug line PHP prints from startJob) happens only once a slot is free. + let _permit = semaphore + .acquire() + .await + .expect("the semaphore is never closed"); - self.output_command_run(&command, cwd, true); + Self::output_command_run_with(&io, &command, cwd.as_deref(), true); - // PHP: $job['reject']($e) on process construction/start failure — surfaced as Err here. - let mut process = if is_string(&command) { - Process::from_shell_commandline( - command.as_string().unwrap_or(""), - cwd, - None, - PhpMixed::Null, - Some(Self::get_timeout() as f64), - )? - } else if let PhpMixed::List(ref list) = command { - Process::new( - list.iter() - .map(|v| v.as_string().unwrap_or("").to_string()) - .collect(), - cwd.map(ToOwned::to_owned), - None, - PhpMixed::Null, - Some(Self::get_timeout() as f64), - )? - } else { - return Err(LogicException { - message: "Invalid command type".to_string(), - code: 0, - } - .into()); - }; + // PHP: $job['reject']($e) on process construction/start failure — surfaced as Err here. + let mut process = if is_string(&command) { + Process::from_shell_commandline( + command.as_string().unwrap_or(""), + cwd.as_deref(), + None, + PhpMixed::Null, + Some(Self::get_timeout() as f64), + )? + } else if let PhpMixed::List(ref list) = command { + Process::new( + list.iter() + .map(|v| v.as_string().unwrap_or("").to_string()) + .collect(), + cwd.clone(), + None, + PhpMixed::Null, + Some(Self::get_timeout() as f64), + )? + } else { + return Err(LogicException { + message: "Invalid command type".to_string(), + code: 0, + } + .into()); + }; - process.start(None, IndexMap::new())?; + process.start(None, IndexMap::new())?; - // PHP's countActiveJobs tick: pump the process until it exits, checking the timeout each - // round. The async sleep yields to the reactor so sibling jobs genuinely overlap. - while process.is_running() { - process.check_timeout()?; - tokio::time::sleep(std::time::Duration::from_millis(1)).await; - } + // PHP's countActiveJobs tick: pump the process until it exits, checking the timeout + // each round. The async sleep yields to the reactor so sibling jobs genuinely overlap. + while process.is_running() { + process.check_timeout()?; + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + } - // PHP resolves the promise with the Process regardless of its exit status; callers - // inspect is_successful() themselves. - Ok(process) + // PHP resolves the promise with the Process regardless of its exit status; callers + // inspect is_successful() themselves. + Ok(process) + }) } fn output_handler( @@ -712,7 +725,18 @@ impl ProcessExecutor { /// @param string|list<string> $command fn output_command_run(&self, command: &PhpMixed, cwd: Option<&str>, r#async: bool) { - if self.io.is_none() || !self.io.as_ref().unwrap().is_debug() { + Self::output_command_run_with(&self.io, command, cwd, r#async); + } + + /// `output_command_run` body as an associated fn so the `execute_async` future can carry a + /// clone of the io handle instead of borrowing the executor. + fn output_command_run_with( + io: &Option<std::rc::Rc<std::cell::RefCell<dyn IOInterface>>>, + command: &PhpMixed, + cwd: Option<&str>, + r#async: bool, + ) { + if io.is_none() || !io.as_ref().unwrap().is_debug() { return; } @@ -754,7 +778,7 @@ impl ProcessExecutor { "--password '***' ", &safe_command, ); - self.io.as_ref().unwrap().write_error(&format!( + io.as_ref().unwrap().write_error(&format!( "Executing{} command ({}): {}", if r#async { " async" } else { "" }, cwd.unwrap_or("CWD"), diff --git a/crates/shirabe/tests/downloader/download_manager_test.rs b/crates/shirabe/tests/downloader/download_manager_test.rs index 6f6ed352..9b45d47a 100644 --- a/crates/shirabe/tests/downloader/download_manager_test.rs +++ b/crates/shirabe/tests/downloader/download_manager_test.rs @@ -18,39 +18,39 @@ mockall::mock! { impl DownloaderInterface for Downloader { fn get_installation_source(&self) -> String; async fn download( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, output: bool, ) -> anyhow::Result<Option<PhpMixed>>; async fn prepare( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, prev_package: Option<PackageInterfaceHandle>, ) -> anyhow::Result<Option<PhpMixed>>; async fn install( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, ) -> anyhow::Result<Option<PhpMixed>>; async fn update( - &mut self, + &self, initial: PackageInterfaceHandle, target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result<Option<PhpMixed>>; async fn remove( - &mut self, + &self, package: PackageInterfaceHandle, path: &str, output: bool, ) -> anyhow::Result<Option<PhpMixed>>; async fn cleanup( - &mut self, + &self, r#type: &str, package: PackageInterfaceHandle, path: &str, diff --git a/crates/shirabe/tests/downloader/file_downloader_test.rs b/crates/shirabe/tests/downloader/file_downloader_test.rs index a9af69ed..451b792e 100644 --- a/crates/shirabe/tests/downloader/file_downloader_test.rs +++ b/crates/shirabe/tests/downloader/file_downloader_test.rs @@ -79,7 +79,7 @@ fn get_downloader( fn test_download_for_package_without_dist_reference() { let package = get_package("dummy/pkg", "1.0.0"); - let (mut downloader, _http_downloader, _guard) = get_downloader(None, None); + let (downloader, _http_downloader, _guard) = get_downloader(None, None); let result = run(downloader.download(package, "/path", None, true)); let e = result.expect_err("expected InvalidArgumentException"); @@ -101,7 +101,7 @@ fn test_download_to_existing_file() { std::fs::write(&path, b"").unwrap(); let path = path.to_string_lossy().into_owned(); - let (mut downloader, _http_downloader, _guard) = get_downloader(None, None); + let (downloader, _http_downloader, _guard) = get_downloader(None, None); let result = run(downloader.download(package, &path, None, true)); @@ -154,7 +154,7 @@ fn test_download_but_file_is_unsaved() { ); let config = get_config(config_options); - let (mut downloader, http_downloader, _guard) = get_downloader(None, Some(config)); + let (downloader, http_downloader, _guard) = get_downloader(None, Some(config)); let mut loop_ = Loop::new(http_downloader, None); let promise = Box::pin(async { @@ -272,7 +272,7 @@ fn test_download_file_with_invalid_checksum() { // The PHP test injects a Filesystem mock so cleanup is a no-op; a real Filesystem behaves the // same here since nothing under the temp dir needs preserving. - let (mut downloader, http_downloader, _guard) = get_downloader(None, Some(config)); + let (downloader, http_downloader, _guard) = get_downloader(None, Some(config)); // make sure the file expected to be downloaded is on disk already let dl_file = downloader.__get_file_name(package.clone(), &path); @@ -350,7 +350,7 @@ fn test_downgrade_shows_appropriate_message() { ); let io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>> = io_mock.clone(); - let mut downloader = FileDownloader::new( + let downloader = FileDownloader::new( io, config, http_downloader.clone(), diff --git a/crates/shirabe/tests/downloader/fossil_downloader_test.rs b/crates/shirabe/tests/downloader/fossil_downloader_test.rs index e473b2dd..d3b50a9f 100644 --- a/crates/shirabe/tests/downloader/fossil_downloader_test.rs +++ b/crates/shirabe/tests/downloader/fossil_downloader_test.rs @@ -87,7 +87,7 @@ fn test_install_for_package_without_source_reference() { let package = get_package(None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let path = format!("{}/path", working_dir.path().to_string_lossy()); let result = run(downloader.install(package, &path)); @@ -121,7 +121,7 @@ fn test_install() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(downloader.install(package, &working_dir_str)).unwrap(); } @@ -134,7 +134,7 @@ fn test_updatefor_package_without_source_reference() { let source_package = get_package(None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(async { downloader @@ -182,7 +182,7 @@ fn test_update() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(async { downloader .prepare( @@ -229,7 +229,7 @@ fn test_remove() { }); let filesystem = std::rc::Rc::new(std::cell::RefCell::new(filesystem)); - let mut downloader = get_downloader_mock(None, None, process, Some(filesystem.clone())); + let downloader = get_downloader_mock(None, None, process, Some(filesystem.clone())); run(async { downloader .prepare("uninstall", package.clone(), &working_dir_str, None) diff --git a/crates/shirabe/tests/downloader/git_downloader_test.rs b/crates/shirabe/tests/downloader/git_downloader_test.rs index d4f40b77..f7c82066 100644 --- a/crates/shirabe/tests/downloader/git_downloader_test.rs +++ b/crates/shirabe/tests/downloader/git_downloader_test.rs @@ -120,7 +120,7 @@ fn test_download_for_package_without_source_reference() { let package = get_package("dummy/pkg", "1.0.0", None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(async { downloader.download(package.clone(), "/path", None).await?; @@ -183,7 +183,7 @@ fn test_download() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(async { downloader @@ -303,7 +303,7 @@ fn test_download_with_cache() { Default::default(), ); - let mut downloader = get_downloader_mock(None, Some(config), process, None); + let downloader = get_downloader_mock(None, Some(config), process, None); run(async { downloader @@ -428,7 +428,7 @@ fn test_download_uses_various_protocols_and_sets_push_url_for_github() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(async { downloader @@ -529,7 +529,7 @@ fn test_download_and_set_push_url_use_custom_various_protocols_for_github() { top.insert("config".to_string(), PhpMixed::Array(section)); config.merge(&top, Config::SOURCE_UNKNOWN); - let mut downloader = get_downloader_mock(None, Some(config), process, None); + let downloader = get_downloader_mock(None, Some(config), process, None); run(async { downloader @@ -578,7 +578,7 @@ fn test_download_throws_runtime_exception_if_git_command_fails() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(async { downloader @@ -610,7 +610,7 @@ fn test_updatefor_package_without_source_reference() { let source_package = get_package("dummy/pkg", "1.0.0", None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(async { downloader @@ -680,7 +680,7 @@ fn test_update() { .unwrap(); let working_dir_str = working_dir.path().to_string_lossy().into_owned(); - let mut downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); + let downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); run(async { downloader @@ -760,7 +760,7 @@ fn test_update_with_new_repo_url() { .unwrap(); let working_dir_str = working_dir.path().to_string_lossy().into_owned(); - let mut downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); + let downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); run(async { downloader @@ -842,7 +842,7 @@ fn test_update_throws_runtime_exception_if_git_command_fails() { let config = Config::new(false, None); - let mut downloader = get_downloader_mock(None, Some(config), process, None); + let downloader = get_downloader_mock(None, Some(config), process, None); let result = run(async { downloader @@ -945,7 +945,7 @@ fn test_update_doesnt_throws_runtime_exception_if_git_command_fails_at_first_but .unwrap(); let working_dir_str = working_dir.path().to_string_lossy().into_owned(); - let mut downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); + let downloader = get_downloader_mock(None, Some(Config::new(false, None)), process, None); run(async { downloader @@ -1001,7 +1001,7 @@ fn test_downgrade_shows_appropriate_message() { .unwrap(); let working_dir_str = working_dir.path().to_string_lossy().into_owned(); - let mut downloader = get_downloader_mock(Some(io), None, process, None); + let downloader = get_downloader_mock(Some(io), None, process, None); run(async { downloader @@ -1079,7 +1079,7 @@ fn test_not_using_downgrading_with_references() { .unwrap(); let working_dir_str = working_dir.path().to_string_lossy().into_owned(); - let mut downloader = get_downloader_mock(Some(io), None, process, None); + let downloader = get_downloader_mock(Some(io), None, process, None); run(async { downloader diff --git a/crates/shirabe/tests/downloader/hg_downloader_test.rs b/crates/shirabe/tests/downloader/hg_downloader_test.rs index ac3d50b6..1ba5cf1c 100644 --- a/crates/shirabe/tests/downloader/hg_downloader_test.rs +++ b/crates/shirabe/tests/downloader/hg_downloader_test.rs @@ -87,7 +87,7 @@ fn test_download_for_package_without_source_reference() { let package = get_package(None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(downloader.install(package, "/path")); let e = result.expect_err("missing source reference should throw"); @@ -117,7 +117,7 @@ fn test_download() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(downloader.install(package, &working_dir_str)).unwrap(); } @@ -131,7 +131,7 @@ fn test_updatefor_package_without_source_reference() { let source_package = get_package(None, None); let (process, _guard) = get_process_executor_mock(vec![], false, Default::default()); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); let result = run(async { downloader @@ -177,7 +177,7 @@ fn test_update() { Default::default(), ); - let mut downloader = get_downloader_mock(None, None, process, None); + let downloader = get_downloader_mock(None, None, process, None); run(async { downloader .prepare( @@ -221,7 +221,7 @@ fn test_remove() { }); let filesystem = std::rc::Rc::new(std::cell::RefCell::new(filesystem)); - let mut downloader = get_downloader_mock(None, None, process, Some(filesystem.clone())); + let downloader = get_downloader_mock(None, None, process, Some(filesystem.clone())); run(async { downloader .prepare("uninstall", package.clone(), &working_dir_str, None) diff --git a/crates/shirabe/tests/downloader/perforce_downloader_test.rs b/crates/shirabe/tests/downloader/perforce_downloader_test.rs index bdf24a2e..c657e244 100644 --- a/crates/shirabe/tests/downloader/perforce_downloader_test.rs +++ b/crates/shirabe/tests/downloader/perforce_downloader_test.rs @@ -83,7 +83,7 @@ fn test_init_perforce_instantiates_a_new_perforce_object() { let (process, _process_guard) = get_process_executor_mock(vec![], false, MockHandler::default()); let fs = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new(None))); - let mut downloader = PerforceDownloader::new(io, config, process, fs); + let downloader = PerforceDownloader::new(io, config, process, fs); downloader.init_perforce( package, @@ -101,7 +101,7 @@ fn test_init_perforce_does_nothing_if_perforce_already_set() { let (process, _process_guard) = get_process_executor_mock(vec![], false, MockHandler::default()); let fs = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new(None))); - let mut downloader = PerforceDownloader::new(io, config, process, fs); + let downloader = PerforceDownloader::new(io, config, process, fs); // The already-set perforce only sees initializePath; the repository's getRepoConfig is // never consulted (the early return happens before reaching it). @@ -146,7 +146,7 @@ fn do_install_workflow(source_ref: &'static str, expected_label: Option<String>) let (process, _process_guard) = get_process_executor_mock(vec![], false, MockHandler::default()); let fs = std::rc::Rc::new(std::cell::RefCell::new(Filesystem::new(None))); - let mut downloader = PerforceDownloader::new(io, config, process, fs); + let downloader = PerforceDownloader::new(io, config, process, fs); let mut perforce = MockPerforce::new(); let expected_path = test_path_str.clone(); diff --git a/crates/shirabe/tests/downloader/xz_downloader_test.rs b/crates/shirabe/tests/downloader/xz_downloader_test.rs index 0002f0e6..db4659f9 100644 --- a/crates/shirabe/tests/downloader/xz_downloader_test.rs +++ b/crates/shirabe/tests/downloader/xz_downloader_test.rs @@ -92,7 +92,7 @@ fn test_error_messages() { let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(Some( io.clone(), )))); - let mut downloader = XzDownloader::new( + let downloader = XzDownloader::new( io, config, http_downloader.clone(), diff --git a/crates/shirabe/tests/downloader/zip_downloader_test.rs b/crates/shirabe/tests/downloader/zip_downloader_test.rs index 878c8054..593f3e76 100644 --- a/crates/shirabe/tests/downloader/zip_downloader_test.rs +++ b/crates/shirabe/tests/downloader/zip_downloader_test.rs @@ -155,7 +155,7 @@ fn test_error_messages() { let process = std::rc::Rc::new(std::cell::RefCell::new(ProcessExecutor::new(Some( io.clone(), )))); - let mut downloader = ZipDownloader::new( + let downloader = ZipDownloader::new( io, config, http_downloader.clone(), @@ -173,13 +173,13 @@ fn test_error_messages() { let result: anyhow::Result<()> = (|| { let mut loop_ = Loop::new(http_downloader.clone(), None); let promise = Box::pin(async { - DownloaderInterface::download(&mut downloader, package.clone(), &path, None, true) + DownloaderInterface::download(&downloader, package.clone(), &path, None, true) .await .map(|_| ()) }); run(loop_.wait(vec![promise], None))?; run(DownloaderInterface::install( - &mut downloader, + &downloader, package.clone(), &path, true, @@ -198,7 +198,7 @@ fn test_zip_archive_only_failed() { let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf()); ZipDownloader::__set_has_zip_archive(Some(true)); - let mut downloader = make_downloader(&set_up); + let downloader = make_downloader(&set_up); let zip_archive = ZipArchive::__mock(ZipArchiveMock { open: Ok(()), count: 0, @@ -224,7 +224,7 @@ fn test_zip_archive_extract_only_failed() { let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf()); ZipDownloader::__set_has_zip_archive(Some(true)); - let mut downloader = make_downloader(&set_up); + let downloader = make_downloader(&set_up); let zip_archive = ZipArchive::__mock(ZipArchiveMock { open: Ok(()), count: 0, @@ -252,7 +252,7 @@ fn test_zip_archive_only_good() { let _tear_down = TearDown::new(set_up.test_dir.path().to_path_buf()); ZipDownloader::__set_has_zip_archive(Some(true)); - let mut downloader = make_downloader(&set_up); + let downloader = make_downloader(&set_up); let zip_archive = ZipArchive::__mock(ZipArchiveMock { open: Ok(()), count: 0, |