From 55a65e726f801f2cfab6d1e2033b047fcc09b7a6 Mon Sep 17 00:00:00 2001 From: roble Date: Sun, 26 Apr 2026 10:03:24 +0100 Subject: [PATCH 1/2] feat: add path repository handling in Installer with corresponding tests --- src/Installer.php | 148 +++++++++++++++++++++++++---- tests/ModuleInstallerTest.php | 172 ++++++++++++++++++++++++++++++++++ 2 files changed, 302 insertions(+), 18 deletions(-) diff --git a/src/Installer.php b/src/Installer.php index dea0718..9e820ab 100644 --- a/src/Installer.php +++ b/src/Installer.php @@ -28,6 +28,8 @@ class Installer extends LibraryInstaller const DEFAULT_UPDATE_STRATEGY = 'merge'; + protected bool $skipUpdateCode = false; + const UPDATE_STRATEGY_MERGE = 'merge'; const UPDATE_STRATEGY_OVERWRITE = 'overwrite'; @@ -273,13 +275,30 @@ protected function mergeStash(string $stash, string $base, string $install): voi } } + /** + * Returns true when the package is served by a local path repository. + * Path repos have source == install path, so the normal download/delete cycle would + * wipe the user's files before trying to copy from the now-missing source. + */ + protected function isPathRepository(PackageInterface $package): bool + { + return $package->getDistType() === 'path'; + } + /** * Override install to remove excluded directories after installation. + * Skips entirely for path repositories — their files are already in place. * * {@inheritDoc} */ public function install(InstalledRepositoryInterface $repo, PackageInterface $package): ?PromiseInterface { + if ($this->isPathRepository($package)) { + $this->io->write(" - Skipping install for path repository: {$package->getPrettyName()}"); + + return \React\Promise\resolve(null); + } + $promise = parent::install($repo, $package); return $promise->then(function () use ($package) { @@ -287,13 +306,73 @@ public function install(InstalledRepositoryInterface $repo, PackageInterface $pa }); } + /** + * Call parent::update() solely for its installed.json tracking side-effects. + * Sets the skip flag so updateCode() is a no-op, then resets it regardless of outcome. + */ + protected function delegateRepoTracking(InstalledRepositoryInterface $repo, PackageInterface $initial, PackageInterface $target): PromiseInterface + { + $this->skipUpdateCode = true; + + $resetFlag = function (): void { $this->skipUpdateCode = false; }; + + return parent::update($repo, $initial, $target)->then( + $resetFlag, + function (\Throwable $e) use ($resetFlag): never { $resetFlag(); throw $e; } + ); + } + + /** + * No-op hook for parent::update() — skipped when we have already handled the download ourselves. + * Overriding this lets parent::update() run only for its repo-tracking side-effects. + * + * {@inheritDoc} + */ + protected function updateCode(PackageInterface $initial, PackageInterface $target): PromiseInterface + { + if ($this->skipUpdateCode) { + return \React\Promise\resolve(null); + } + + return $this->invokeParentUpdateCode($initial, $target); + } + + protected function invokeParentUpdateCode(PackageInterface $initial, PackageInterface $target): PromiseInterface + { + return parent::updateCode($initial, $target); + } + + /** + * Download a specific package version to the given path using Composer's DownloadManager. + * Used for both base (original) and target (new) version fetches during update. + */ + protected function downloadFresh(PackageInterface $package, string $path): PromiseInterface + { + $dm = $this->composer->getDownloadManager(); + + return $dm->download($package, $path) + ->then(fn () => $dm->install($package, $path)); + } + /** * Override update to preserve user customisations (merge strategy) or replace entirely (overwrite). + * Skips entirely for path repositories — their files are managed by git/the path repo mechanism. + * + * Does NOT call parent::update() because that uses GitDownloader::update() which requires a .git + * directory. Since we remove .git after initial install (copy-and-own model), we instead do a + * fresh download of the target version directly to the install path. * * {@inheritDoc} */ public function update(InstalledRepositoryInterface $repo, PackageInterface $initial, PackageInterface $target): ?PromiseInterface { + if ($this->isPathRepository($target)) { + $this->io->write(" - Skipping update for path repository: {$target->getPrettyName()}"); + + return \React\Promise\resolve(null); + } + + $installPath = $this->getInstallPath($target); $stashPath = null; $basePath = null; @@ -302,31 +381,64 @@ public function update(InstalledRepositoryInterface $repo, PackageInterface $ini if ($stashPath !== null) { $basePath = sys_get_temp_dir().'/module-base-'.uniqid('', true); } + } else { + // Overwrite: stash for rollback safety rather than deleting outright. + // $basePath stays null, which signals overwrite mode in the callbacks. + $stashPath = $this->stashModuleDir($installPath); } $prepareBase = ($basePath !== null) ? $this->downloadBase($initial, $basePath) : \React\Promise\resolve(null); - return $prepareBase->then(fn () => parent::update($repo, $initial, $target))->then( - function () use ($target, $stashPath, $basePath) { - $this->removeExcludedDirectories($target); - if ($stashPath !== null) { - $installPath = $this->getInstallPath($target); - $this->mergeStash($stashPath, $basePath, $installPath); - (new Filesystem)->removeDirectory($stashPath); - (new Filesystem)->removeDirectory($basePath); - } - }, - function (\Throwable $e) use ($stashPath, $basePath, $initial) { - if ($stashPath !== null) { - $this->restoreStash($stashPath, $initial); + return $prepareBase + ->then(fn () => $this->downloadFresh($target, $installPath)) + ->then( + function () use ($repo, $initial, $target, $installPath, $stashPath, $basePath) { + $this->removeExcludedDirectories($target); + if ($basePath !== null) { + // Merge strategy: apply 3-way merge then clean up base temp dir + $this->mergeStash($stashPath, $basePath, $installPath); + (new Filesystem)->removeDirectory($basePath); + } + if ($stashPath !== null) { + // Both strategies: download succeeded — discard the stash + (new Filesystem)->removeDirectory($stashPath); + } + // Delegate repo tracking (installed.json) to parent — skip the download step + // since we have already placed the files ourselves. + return $this->delegateRepoTracking($repo, $initial, $target); + }, + function (\Throwable $e) use ($stashPath, $basePath, $initial) { + if ($stashPath !== null) { + $this->restoreStash($stashPath, $initial); + } + if ($basePath !== null) { + (new Filesystem)->removeDirectory($basePath); + } + throw $e; } - if ($basePath !== null) { - (new Filesystem)->removeDirectory($basePath); - } - throw $e; + ); + } + + /** + * Override uninstall to protect path repository files from deletion. + * A `composer remove` on a path repo must never wipe the working source directory. + * + * {@inheritDoc} + */ + public function uninstall(InstalledRepositoryInterface $repo, PackageInterface $package): ?PromiseInterface + { + if ($this->isPathRepository($package)) { + $this->io->write(" - Skipping uninstall for path repository: {$package->getPrettyName()}"); + + if ($repo->hasPackage($package)) { + $repo->removePackage($package); } - ); + + return \React\Promise\resolve(null); + } + + return parent::uninstall($repo, $package); } } diff --git a/tests/ModuleInstallerTest.php b/tests/ModuleInstallerTest.php index c947f84..c0450c9 100644 --- a/tests/ModuleInstallerTest.php +++ b/tests/ModuleInstallerTest.php @@ -10,6 +10,7 @@ use Composer\Package\PackageInterface; use Composer\Package\RootPackage; use Composer\PartialComposer; +use Composer\Repository\InstalledRepositoryInterface; use PHPUnit\Framework\TestCase; use Saucebase\ModuleInstaller\Exceptions\ModuleInstallerException; use Saucebase\ModuleInstaller\Installer; @@ -58,6 +59,30 @@ public function callRestoreStash(string $stashPath, PackageInterface $package): { parent::restoreStash($stashPath, $package); } + + public function callIsPathRepository(PackageInterface $package): bool + { + return parent::isPathRepository($package); + } + + public function callUpdateCode(PackageInterface $initial, PackageInterface $target): \React\Promise\PromiseInterface + { + return $this->updateCode($initial, $target); + } + + public function setSkipUpdateCode(bool $value): void + { + $this->skipUpdateCode = $value; + } + + public bool $parentUpdateCodeInvoked = false; + + protected function invokeParentUpdateCode(PackageInterface $initial, PackageInterface $target): \React\Promise\PromiseInterface + { + $this->parentUpdateCodeInvoked = true; + + return parent::invokeParentUpdateCode($initial, $target); + } } final class ModuleInstallerTest extends TestCase @@ -470,4 +495,151 @@ public function test_restore_stash_discards_stash_when_original_path_already_exi (new Filesystem)->remove($originalPath); } + + // ------------------------------------------------------------------------- + // isPathRepository + // ------------------------------------------------------------------------- + + public function test_is_path_repository_returns_true_for_path_dist_type(): void + { + $io = $this->createStub(IOInterface::class); + $installer = new TestableInstaller($io, null); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn('path'); + + $this->assertTrue($installer->callIsPathRepository($pkg)); + } + + public function test_is_path_repository_returns_false_for_zip_dist_type(): void + { + $io = $this->createStub(IOInterface::class); + $installer = new TestableInstaller($io, null); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn('zip'); + + $this->assertFalse($installer->callIsPathRepository($pkg)); + } + + public function test_is_path_repository_returns_false_when_dist_type_is_null(): void + { + $io = $this->createStub(IOInterface::class); + $installer = new TestableInstaller($io, null); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn(null); + + $this->assertFalse($installer->callIsPathRepository($pkg)); + } + + // ------------------------------------------------------------------------- + // install() path-repository guard + // ------------------------------------------------------------------------- + + public function test_install_logs_skip_message_and_returns_promise_for_path_repo(): void + { + $io = $this->createMock(IOInterface::class); + $io->expects($this->once()) + ->method('write') + ->with($this->stringContains('Skipping install')); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn('path'); + $pkg->method('getPrettyName')->willReturn('saucebase/test'); + + $repo = $this->createStub(InstalledRepositoryInterface::class); + $installer = new TestableInstaller($io, null); + + $promise = $installer->install($repo, $pkg); + + $this->assertNotNull($promise); + } + + // ------------------------------------------------------------------------- + // update() path-repository guard + // ------------------------------------------------------------------------- + + public function test_update_logs_skip_message_and_returns_promise_for_path_repo(): void + { + $io = $this->createMock(IOInterface::class); + $io->expects($this->once()) + ->method('write') + ->with($this->stringContains('Skipping update')); + + $initial = $this->createStub(PackageInterface::class); + + $target = $this->createStub(PackageInterface::class); + $target->method('getDistType')->willReturn('path'); + $target->method('getPrettyName')->willReturn('saucebase/test'); + + $repo = $this->createStub(InstalledRepositoryInterface::class); + $installer = new TestableInstaller($io, null); + + $promise = $installer->update($repo, $initial, $target); + + $this->assertNotNull($promise); + } + + // ------------------------------------------------------------------------- + // uninstall() path-repository guard + // ------------------------------------------------------------------------- + + public function test_uninstall_logs_skip_message_and_removes_package_from_repo(): void + { + $io = $this->createMock(IOInterface::class); + $io->expects($this->once()) + ->method('write') + ->with($this->stringContains('Skipping uninstall')); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn('path'); + $pkg->method('getPrettyName')->willReturn('saucebase/test'); + + $repo = $this->createMock(InstalledRepositoryInterface::class); + $repo->method('hasPackage')->willReturn(true); + $repo->expects($this->once())->method('removePackage')->with($pkg); + + $installer = new TestableInstaller($io, null); + $installer->uninstall($repo, $pkg); + } + + public function test_uninstall_skips_remove_package_when_package_not_in_repo(): void + { + $io = $this->createMock(IOInterface::class); + $io->expects($this->once())->method('write'); + + $pkg = $this->createStub(PackageInterface::class); + $pkg->method('getDistType')->willReturn('path'); + $pkg->method('getPrettyName')->willReturn('saucebase/test'); + + $repo = $this->createMock(InstalledRepositoryInterface::class); + $repo->method('hasPackage')->willReturn(false); + $repo->expects($this->never())->method('removePackage'); + + $installer = new TestableInstaller($io, null); + $installer->uninstall($repo, $pkg); + } + + // ------------------------------------------------------------------------- + // updateCode() skip flag + // ------------------------------------------------------------------------- + + public function test_update_code_returns_resolved_promise_when_skip_flag_is_set(): void + { + $io = $this->createStub(IOInterface::class); + $installer = new TestableInstaller($io, null); + $installer->setSkipUpdateCode(true); + + $initial = $this->createStub(PackageInterface::class); + $target = $this->createStub(PackageInterface::class); + + $resolved = false; + $installer->callUpdateCode($initial, $target)->then(function () use (&$resolved) { + $resolved = true; + }); + + $this->assertTrue($resolved, 'updateCode should resolve immediately when skip flag is set'); + $this->assertFalse($installer->parentUpdateCodeInvoked, 'parent::updateCode() must not be called when skip flag is set'); + } } From da65387002c35ee82afc043dff2062543332d8f2 Mon Sep 17 00:00:00 2001 From: roble <3231587+roble@users.noreply.github.com> Date: Sun, 26 Apr 2026 09:04:11 +0000 Subject: [PATCH 2/2] PHP Linting (Pint) --- src/Installer.php | 10 ++++++++-- tests/ModuleInstallerTest.php | 5 +++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Installer.php b/src/Installer.php index 9e820ab..8ea3794 100644 --- a/src/Installer.php +++ b/src/Installer.php @@ -314,11 +314,16 @@ protected function delegateRepoTracking(InstalledRepositoryInterface $repo, Pack { $this->skipUpdateCode = true; - $resetFlag = function (): void { $this->skipUpdateCode = false; }; + $resetFlag = function (): void { + $this->skipUpdateCode = false; + }; return parent::update($repo, $initial, $target)->then( $resetFlag, - function (\Throwable $e) use ($resetFlag): never { $resetFlag(); throw $e; } + function (\Throwable $e) use ($resetFlag): never { + $resetFlag(); + throw $e; + } ); } @@ -405,6 +410,7 @@ function () use ($repo, $initial, $target, $installPath, $stashPath, $basePath) // Both strategies: download succeeded — discard the stash (new Filesystem)->removeDirectory($stashPath); } + // Delegate repo tracking (installed.json) to parent — skip the download step // since we have already placed the files ourselves. return $this->delegateRepoTracking($repo, $initial, $target); diff --git a/tests/ModuleInstallerTest.php b/tests/ModuleInstallerTest.php index c0450c9..5b2c87a 100644 --- a/tests/ModuleInstallerTest.php +++ b/tests/ModuleInstallerTest.php @@ -12,6 +12,7 @@ use Composer\PartialComposer; use Composer\Repository\InstalledRepositoryInterface; use PHPUnit\Framework\TestCase; +use React\Promise\PromiseInterface; use Saucebase\ModuleInstaller\Exceptions\ModuleInstallerException; use Saucebase\ModuleInstaller\Installer; use Symfony\Component\Filesystem\Filesystem; @@ -65,7 +66,7 @@ public function callIsPathRepository(PackageInterface $package): bool return parent::isPathRepository($package); } - public function callUpdateCode(PackageInterface $initial, PackageInterface $target): \React\Promise\PromiseInterface + public function callUpdateCode(PackageInterface $initial, PackageInterface $target): PromiseInterface { return $this->updateCode($initial, $target); } @@ -77,7 +78,7 @@ public function setSkipUpdateCode(bool $value): void public bool $parentUpdateCodeInvoked = false; - protected function invokeParentUpdateCode(PackageInterface $initial, PackageInterface $target): \React\Promise\PromiseInterface + protected function invokeParentUpdateCode(PackageInterface $initial, PackageInterface $target): PromiseInterface { $this->parentUpdateCodeInvoked = true;