From c5bcf222f98d13b104231713bf4a0aa0833c420a Mon Sep 17 00:00:00 2001 From: nsfisis Date: Fri, 5 Jun 2026 00:50:47 +0900 Subject: feat(downloader): wire VcsDownloader subclasses via shared downloaders Share downloaders as Rc> in DownloadManager and make DownloaderInterface/VcsDownloader take &mut self, matching PHP's mutable-by-reference downloader objects. This lets Git/Svn/Hg/Fossil/Perforce implement VcsDownloader and route download/install/update/prepare/cleanup through the trait instead of todo!(), and wires the as_* downcast hooks end-to-end. ChangeReportInterface::get_local_changes becomes &mut self since FileDownloader downloads to a compare dir; get_commit_logs/reapply_changes gain &mut self / Result to match the concrete implementations. Co-Authored-By: Claude Opus 4.8 --- .../shirabe/src/downloader/perforce_downloader.rs | 204 ++++++++++++--------- 1 file changed, 115 insertions(+), 89 deletions(-) (limited to 'crates/shirabe/src/downloader/perforce_downloader.rs') diff --git a/crates/shirabe/src/downloader/perforce_downloader.rs b/crates/shirabe/src/downloader/perforce_downloader.rs index e33b159..2d91c60 100644 --- a/crates/shirabe/src/downloader/perforce_downloader.rs +++ b/crates/shirabe/src/downloader/perforce_downloader.rs @@ -4,6 +4,7 @@ use crate::config::Config; use crate::downloader::ChangeReportInterface; use crate::downloader::DownloaderInterface; use crate::downloader::VcsCapableDownloaderInterface; +use crate::downloader::VcsDownloader; use crate::downloader::VcsDownloaderBase; use crate::io::IOInterface; use crate::io::IOInterfaceImmutable; @@ -37,46 +38,6 @@ impl PerforceDownloader { } } - pub(crate) async fn do_download( - &self, - _package: PackageInterfaceHandle, - _path: String, - _url: String, - _prev_package: Option, - ) -> Result> { - Ok(None) - } - - pub async fn do_install( - &mut self, - package: PackageInterfaceHandle, - path: String, - url: String, - ) -> Result> { - let source_ref = package.get_source_reference().map(|s| s.to_string()); - let label = self.get_label_from_source_reference(source_ref.clone().unwrap_or_default()); - - self.inner.io.write_error(&format!( - "Cloning {}", - source_ref.clone().unwrap_or_default() - )); - self.init_perforce(package, path.clone(), url); - 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.as_deref()); - self.perforce.as_mut().unwrap().cleanup_client_spec(); - - Ok(None) - } - fn get_label_from_source_reference(&self, source_ref: String) -> Option { let pos = source_ref.find('@'); if let Some(pos) = pos { @@ -116,42 +77,108 @@ impl PerforceDownloader { repository.get_repo_config().clone() } - pub(crate) async fn do_update( + pub fn set_perforce(&mut self, perforce: Perforce) { + self.perforce = Some(perforce); + } +} + +impl VcsDownloader for PerforceDownloader { + fn io(&self) -> std::rc::Rc> { + self.inner.io.clone() + } + + fn config(&self) -> &std::rc::Rc> { + &self.inner.config + } + + fn process(&self) -> &std::rc::Rc> { + &self.inner.process + } + + fn filesystem(&self) -> &std::rc::Rc> { + &self.inner.filesystem + } + + fn has_cleaned_changes(&self) -> &IndexMap { + &self.inner.has_cleaned_changes + } + + fn has_cleaned_changes_mut(&mut self) -> &mut IndexMap { + &mut self.inner.has_cleaned_changes + } + + async fn do_download( + &mut self, + _package: PackageInterfaceHandle, + _path: &str, + _url: &str, + _prev_package: Option, + ) -> Result> { + Ok(None) + } + + async fn do_install( + &mut self, + package: PackageInterfaceHandle, + path: &str, + url: &str, + ) -> Result> { + let source_ref = package.get_source_reference().map(|s| s.to_string()); + let label = self.get_label_from_source_reference(source_ref.clone().unwrap_or_default()); + + self.inner.io.write_error(&format!( + "Cloning {}", + 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.as_deref()); + self.perforce.as_mut().unwrap().cleanup_client_spec(); + + Ok(None) + } + + async fn do_update( &mut self, _initial: PackageInterfaceHandle, target: PackageInterfaceHandle, - path: String, - url: String, + path: &str, + url: &str, ) -> Result> { self.do_install(target, path, url).await } - pub(crate) fn get_commit_logs( + fn get_commit_logs( &mut self, - from_reference: String, - to_reference: String, - _path: String, + from_reference: &str, + to_reference: &str, + _path: &str, ) -> Result { Ok(self .perforce .as_mut() .unwrap() - .get_commit_logs(&from_reference, &to_reference) + .get_commit_logs(from_reference, to_reference) .unwrap_or_default()) } - pub fn set_perforce(&mut self, perforce: Perforce) { - self.perforce = Some(perforce); - } - - pub(crate) fn has_metadata_repository(&self, _path: &str) -> bool { + fn has_metadata_repository(&self, _path: &str) -> bool { true } } impl ChangeReportInterface for PerforceDownloader { fn get_local_changes( - &self, + &mut self, _package: PackageInterfaceHandle, _path: &str, ) -> Result> { @@ -169,12 +196,11 @@ impl VcsCapableDownloaderInterface for PerforceDownloader { } } -// TODO(phase-b): wire up VcsDownloader trait properly. PerforceDownloader extends VcsDownloader -// which implements DownloaderInterface in PHP. Delegating each trait method to todo!() until the -// inner VcsDownloaderBase exposes the matching impl surface. #[async_trait::async_trait(?Send)] impl DownloaderInterface for PerforceDownloader { - fn as_change_report_interface(&self) -> Option<&dyn crate::downloader::ChangeReportInterface> { + fn as_change_report_interface( + &mut self, + ) -> Option<&mut dyn crate::downloader::ChangeReportInterface> { Some(self) } @@ -185,63 +211,63 @@ impl DownloaderInterface for PerforceDownloader { } fn get_installation_source(&self) -> String { - todo!() + ::get_installation_source(self) } async fn download( - &self, - _package: PackageInterfaceHandle, - _path: &str, - _prev_package: Option, + &mut self, + package: PackageInterfaceHandle, + path: &str, + prev_package: Option, _output: bool, ) -> Result> { - todo!() + ::download(self, package, path, prev_package).await } async fn prepare( - &self, - _type: &str, - _package: PackageInterfaceHandle, - _path: &str, - _prev_package: Option, + &mut self, + r#type: &str, + package: PackageInterfaceHandle, + path: &str, + prev_package: Option, ) -> Result> { - todo!() + ::prepare(self, r#type, package, path, prev_package).await } async fn install( - &self, - _package: PackageInterfaceHandle, - _path: &str, + &mut self, + package: PackageInterfaceHandle, + path: &str, _output: bool, ) -> Result> { - todo!() + ::install(self, package, path).await } async fn update( - &self, - _initial: PackageInterfaceHandle, - _target: PackageInterfaceHandle, - _path: &str, + &mut self, + initial: PackageInterfaceHandle, + target: PackageInterfaceHandle, + path: &str, ) -> Result> { - todo!() + ::update(self, initial, target, path).await } async fn remove( - &self, - _package: PackageInterfaceHandle, - _path: &str, + &mut self, + package: PackageInterfaceHandle, + path: &str, _output: bool, ) -> Result> { - todo!() + ::remove(self, package, path).await } async fn cleanup( - &self, - _type: &str, - _package: PackageInterfaceHandle, - _path: &str, - _prev_package: Option, + &mut self, + r#type: &str, + package: PackageInterfaceHandle, + path: &str, + prev_package: Option, ) -> Result> { - todo!() + ::cleanup(self, r#type, package, path, prev_package).await } } -- cgit v1.3.1