feat(downloader): wire VcsDownloader subclasses via shared downloaders

Share downloaders as Rc<RefCell<dyn DownloaderInterface>> 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 <noreply@anthropic.com>

1 files changed, 115 insertions, 89 deletions

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<PackageInterfaceHandle>,
-    ) -> Result<Option<PhpMixed>> {
-        Ok(None)
-    }
-
-    pub async fn do_install(
-        &mut self,
-        package: PackageInterfaceHandle,
-        path: String,
-        url: String,
-    ) -> Result<Option<PhpMixed>> {
-        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<String> {
         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<std::cell::RefCell<dyn IOInterface>> {
+        self.inner.io.clone()
+    }
+
+    fn config(&self) -> &std::rc::Rc<std::cell::RefCell<Config>> {
+        &self.inner.config
+    }
+
+    fn process(&self) -> &std::rc::Rc<std::cell::RefCell<ProcessExecutor>> {
+        &self.inner.process
+    }
+
+    fn filesystem(&self) -> &std::rc::Rc<std::cell::RefCell<Filesystem>> {
+        &self.inner.filesystem
+    }
+
+    fn has_cleaned_changes(&self) -> &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,
+        _package: PackageInterfaceHandle,
+        _path: &str,
+        _url: &str,
+        _prev_package: Option<PackageInterfaceHandle>,
+    ) -> Result<Option<PhpMixed>> {
+        Ok(None)
+    }
+
+    async fn do_install(
+        &mut self,
+        package: PackageInterfaceHandle,
+        path: &str,
+        url: &str,
+    ) -> Result<Option<PhpMixed>> {
+        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<Option<PhpMixed>> {
         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<String> {
         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<Option<String>> {
@@ -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!()
+        <Self as VcsDownloader>::get_installation_source(self)
     }
 
     async fn download(
-        &self,
-        _package: PackageInterfaceHandle,
-        _path: &str,
-        _prev_package: Option<PackageInterfaceHandle>,
+        &mut self,
+        package: PackageInterfaceHandle,
+        path: &str,
+        prev_package: Option<PackageInterfaceHandle>,
         _output: bool,
     ) -> Result<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::download(self, package, path, prev_package).await
     }
 
     async fn prepare(
-        &self,
-        _type: &str,
-        _package: PackageInterfaceHandle,
-        _path: &str,
-        _prev_package: Option<PackageInterfaceHandle>,
+        &mut self,
+        r#type: &str,
+        package: PackageInterfaceHandle,
+        path: &str,
+        prev_package: Option<PackageInterfaceHandle>,
     ) -> Result<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::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<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::install(self, package, path).await
     }
 
     async fn update(
-        &self,
-        _initial: PackageInterfaceHandle,
-        _target: PackageInterfaceHandle,
-        _path: &str,
+        &mut self,
+        initial: PackageInterfaceHandle,
+        target: PackageInterfaceHandle,
+        path: &str,
     ) -> Result<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::update(self, initial, target, path).await
     }
 
     async fn remove(
-        &self,
-        _package: PackageInterfaceHandle,
-        _path: &str,
+        &mut self,
+        package: PackageInterfaceHandle,
+        path: &str,
         _output: bool,
     ) -> Result<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::remove(self, package, path).await
     }
 
     async fn cleanup(
-        &self,
-        _type: &str,
-        _package: PackageInterfaceHandle,
-        _path: &str,
-        _prev_package: Option<PackageInterfaceHandle>,
+        &mut self,
+        r#type: &str,
+        package: PackageInterfaceHandle,
+        path: &str,
+        prev_package: Option<PackageInterfaceHandle>,
     ) -> Result<Option<PhpMixed>> {
-        todo!()
+        <Self as VcsDownloader>::cleanup(self, r#type, package, path, prev_package).await
     }
 }