From 9b291d454b059977639db26c847af4e835cda0c3 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 18 Jul 2026 18:03:57 +0900 Subject: refactor(downloader): take &self across the downloader hierarchy Concurrent package operations call into the same downloader instances through Rc>; with &mut self methods every call holds a RefMut across its awaits, which panics with 'already mutably borrowed' the moment two operations overlap. This is groundwork for fanning out InstallationManager's download/install loops (same rework HttpDownloader/CurlDownloader already got). - DownloaderInterface/ChangeReportInterface/ArchiveDownloader/ VcsDownloader methods now take &self; as_change_report_interface returns &dyn instead of &mut dyn. - Implementors move their genuinely mutable state behind cells: FileDownloader.additional_cleanup_paths, the archive downloaders' cleanup_executed, ZipDownloader.zip_archive_object, VcsDownloaderBase.has_cleaned_changes, GitDownloader's stash/discard/ cache maps and GitUtil, SvnDownloader.cache_credentials, PerforceDownloader.perforce. FileDownloader.io gains a RefCell layer so get_local_changes can keep PHP's NullIO swap under &self. - ProcessExecutor::execute_async now returns a future that captures everything up front instead of borrowing the executor, and call sites build the future before awaiting, so no borrow on the shared executor is held while a subprocess runs. - Filesystem::remove_directory_async becomes remove_directory_async_via taking the Rc handle: the Filesystem is only borrowed for the sync head/tail, never across the rm subprocess await (sync borrow_mut users like rename/ensure_directory_exists would otherwise collide). - DownloadManager async call sites hold shared borrows only. Co-Authored-By: Claude Fable 5 --- .../tests/downloader/git_downloader_test.rs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'crates/shirabe/tests/downloader/git_downloader_test.rs') 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 -- cgit v1.3.1