From 3e367f78eec3521106979461fda717926717515a Mon Sep 17 00:00:00 2001 From: nsfisis 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 --- crates/shirabe/src/downloader/file_downloader.rs | 55 +++++++++++++++++++----- crates/shirabe/src/downloader/gzip_downloader.rs | 7 ++- crates/shirabe/src/downloader/path_downloader.rs | 7 ++- crates/shirabe/src/downloader/phar_downloader.rs | 7 ++- crates/shirabe/src/downloader/rar_downloader.rs | 7 ++- crates/shirabe/src/downloader/tar_downloader.rs | 7 ++- crates/shirabe/src/downloader/xz_downloader.rs | 7 ++- crates/shirabe/src/downloader/zip_downloader.rs | 7 ++- 8 files changed, 79 insertions(+), 25 deletions(-) (limited to 'crates/shirabe/src/downloader') 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> { - 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> { + 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> { 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> { + 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 { diff --git a/crates/shirabe/src/downloader/gzip_downloader.rs b/crates/shirabe/src/downloader/gzip_downloader.rs index 6b5f3872..70b9f7bb 100644 --- a/crates/shirabe/src/downloader/gzip_downloader.rs +++ b/crates/shirabe/src/downloader/gzip_downloader.rs @@ -146,7 +146,7 @@ impl ChangeReportInterface for GzipDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -197,7 +197,10 @@ impl crate::downloader::DownloaderInterface for GzipDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/path_downloader.rs b/crates/shirabe/src/downloader/path_downloader.rs index 1f5498ad..72b77fc1 100644 --- a/crates/shirabe/src/downloader/path_downloader.rs +++ b/crates/shirabe/src/downloader/path_downloader.rs @@ -213,7 +213,7 @@ impl crate::downloader::ChangeReportInterface for PathDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -493,7 +493,10 @@ impl DownloaderInterface for PathDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path)?; + self.inner + .base_update(self, &appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/phar_downloader.rs b/crates/shirabe/src/downloader/phar_downloader.rs index b33fa74a..32ac0b3f 100644 --- a/crates/shirabe/src/downloader/phar_downloader.rs +++ b/crates/shirabe/src/downloader/phar_downloader.rs @@ -79,7 +79,7 @@ impl ChangeReportInterface for PharDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -130,7 +130,10 @@ impl DownloaderInterface for PharDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/rar_downloader.rs b/crates/shirabe/src/downloader/rar_downloader.rs index 7eaa5a8c..39ffc23b 100644 --- a/crates/shirabe/src/downloader/rar_downloader.rs +++ b/crates/shirabe/src/downloader/rar_downloader.rs @@ -159,7 +159,7 @@ impl ChangeReportInterface for RarDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -210,7 +210,10 @@ impl crate::downloader::DownloaderInterface for RarDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/tar_downloader.rs b/crates/shirabe/src/downloader/tar_downloader.rs index ae600971..a4446287 100644 --- a/crates/shirabe/src/downloader/tar_downloader.rs +++ b/crates/shirabe/src/downloader/tar_downloader.rs @@ -74,7 +74,7 @@ impl ChangeReportInterface for TarDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -125,7 +125,10 @@ impl DownloaderInterface for TarDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/xz_downloader.rs b/crates/shirabe/src/downloader/xz_downloader.rs index d3defae5..2041c356 100644 --- a/crates/shirabe/src/downloader/xz_downloader.rs +++ b/crates/shirabe/src/downloader/xz_downloader.rs @@ -94,7 +94,7 @@ impl ChangeReportInterface for XzDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -145,7 +145,10 @@ impl crate::downloader::DownloaderInterface for XzDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( diff --git a/crates/shirabe/src/downloader/zip_downloader.rs b/crates/shirabe/src/downloader/zip_downloader.rs index f70846e4..d8f7d05e 100644 --- a/crates/shirabe/src/downloader/zip_downloader.rs +++ b/crates/shirabe/src/downloader/zip_downloader.rs @@ -454,7 +454,7 @@ impl ChangeReportInterface for ZipDownloader { package: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.get_local_changes(package, path) + self.inner.base_get_local_changes(self, package, path) } } @@ -632,7 +632,10 @@ impl crate::downloader::DownloaderInterface for ZipDownloader { target: PackageInterfaceHandle, path: &str, ) -> anyhow::Result> { - self.inner.update(initial, target, path).await + let appendix = self.get_install_operation_appendix(target.clone(), path); + self.inner + .base_update(self, appendix, initial, target, path) + .await } async fn remove( -- cgit v1.3.1