From 3e367f78eec3521106979461fda717926717515a Mon Sep 17 00:00:00 2001
From: nsfisis <nsfisis@gmail.com>
Date: Sun, 2 Aug 2026 08:42:53 +0900
Subject: fix(downloader): restore late binding in getLocalChanges/update paths

PHP's FileDownloader::getLocalChanges and ::update call $this->download()
/ $this->install() / $this->remove() / $this->getInstallOperationAppendix(),
which late-bind to the concrete downloader class. The Rust port embeds the
parent as `inner`, so delegating these methods to FileDownloader pinned the
calls to FileDownloader's own implementations: `status` built the compare
tree without extracting the archive (flagging every file of dist-installed
packages as changed), and `update` re-installed the raw dist file instead
of extracting it.

Thread the concrete downloader in as `this: &dyn DownloaderInterface` via
shared helpers (base_get_local_changes / base_update) and pass `self` from
each delegating downloader.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 crates/shirabe/src/downloader/file_downloader.rs | 55 +++++++++++++++++++-----
 1 file changed, 44 insertions(+), 11 deletions(-)

(limited to 'crates/shirabe/src/downloader/file_downloader.rs')

diff --git a/crates/shirabe/src/downloader/file_downloader.rs b/crates/shirabe/src/downloader/file_downloader.rs
index bb5c9705..19ce8b4a 100644
--- a/crates/shirabe/src/downloader/file_downloader.rs
+++ b/crates/shirabe/src/downloader/file_downloader.rs
@@ -569,15 +569,9 @@ impl DownloaderInterface for FileDownloader {
         target: PackageInterfaceHandle,
         path: &str,
     ) -> anyhow::Result<Option<PhpMixed>> {
-        self.io.borrow().write_error(&format!(
-            "  - {}{}",
-            UpdateOperation::format(initial.clone(), target.clone(), false),
-            self.get_install_operation_appendix(target.clone(), path)
-        ));
-
-        // PHP: return $this->remove($initial, $path, false)->then(fn () => $this->install($target, $path, false));
-        let _ = self.remove(initial, path, false).await?;
-        self.install(target, path, false).await
+        let appendix = self.get_install_operation_appendix(target.clone(), path);
+        self.base_update(self, &appendix, initial, target, path)
+            .await
     }
 
     /// @inheritDoc
@@ -613,6 +607,23 @@ impl ChangeReportInterface for FileDownloader {
         &self,
         package: PackageInterfaceHandle,
         path: &str,
+    ) -> anyhow::Result<Option<String>> {
+        self.base_get_local_changes(self, package, path)
+    }
+}
+
+impl FileDownloader {
+    /// Shared body of `ChangeReportInterface::get_local_changes`.
+    ///
+    /// PHP's `getLocalChanges` calls `$this->download()` / `$this->install()`, which late-bind
+    /// to the concrete downloader class (e.g. `ArchiveDownloader::install` extracts the archive
+    /// instead of copying the dist file). The Rust port embeds the parent class as `inner`, so
+    /// delegating downloaders must pass themselves as `this` to preserve that dispatch.
+    pub(crate) fn base_get_local_changes(
+        &self,
+        this: &dyn DownloaderInterface,
+        package: PackageInterfaceHandle,
+        path: &str,
     ) -> anyhow::Result<Option<String>> {
         let prev_io = std::mem::replace(
             &mut *self.io.borrow_mut(),
@@ -634,13 +645,13 @@ impl ChangeReportInterface for FileDownloader {
                     .remove_directory(format!("{}_compare", target_dir))?;
             }
 
-            sync_executor::block_on(self.download(
+            sync_executor::block_on(this.download(
                 package.clone(),
                 &format!("{}_compare", target_dir),
                 None,
                 false,
             ))?;
-            sync_executor::block_on(self.install(
+            sync_executor::block_on(this.install(
                 package.clone(),
                 &format!("{}_compare", target_dir),
                 false,
@@ -684,6 +695,28 @@ impl ChangeReportInterface for FileDownloader {
             None
         })
     }
+
+    /// Shared body of `DownloaderInterface::update`; see `base_get_local_changes` for why the
+    /// concrete downloader is threaded in as `this`. The appendix is computed by the caller
+    /// because `getInstallOperationAppendix` is protected and not part of `DownloaderInterface`.
+    pub(crate) async fn base_update(
+        &self,
+        this: &dyn DownloaderInterface,
+        install_operation_appendix: &str,
+        initial: PackageInterfaceHandle,
+        target: PackageInterfaceHandle,
+        path: &str,
+    ) -> anyhow::Result<Option<PhpMixed>> {
+        self.io.borrow().write_error(&format!(
+            "  - {}{}",
+            UpdateOperation::format(initial.clone(), target.clone(), false),
+            install_operation_appendix
+        ));
+
+        // PHP: return $this->remove($initial, $path, false)->then(fn () => $this->install($target, $path, false));
+        let _ = this.remove(initial, path, false).await?;
+        this.install(target, path, false).await
+    }
 }
 
 impl FileDownloader {
-- 
cgit v1.3.1