Skip to content

feat: add path repository handling in Installer with corresponding tests #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
roble merged 2 commits into from
Apr 26, 2026
Merged

feat: add path repository handling in Installer with corresponding tests #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55a65e7
feat: add path repository handling in Installer with corresponding tests
roble Apr 26, 2026
da65387
PHP Linting (Pint)
roble Apr 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 136 additions & 18 deletions src/Installer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -273,27 +275,109 @@ 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(" - <info>Skipping install for path repository:</info> {$package->getPrettyName()}");

return \React\Promise\resolve(null);
}

$promise = parent::install($repo, $package);

return $promise->then(function () use ($package) {
$this->removeExcludedDirectories($package);
});
Comment on lines 302 to 306

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent::install() is declared to return ?PromiseInterface, but the code unconditionally calls ->then() on its return value. If Composer returns null for synchronous installers, this will throw. Consider handling the null case (performing removeExcludedDirectories() immediately and returning null, or wrapping with React\Promise\resolve($promise) before chaining).

Copilot uses AI. Check for mistakes.
}

/**
* 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(

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delegateRepoTracking() assumes parent::update() always returns a promise and immediately calls ->then(). Since Composer’s installer API allows update() to return null, this can fatal and also leave $skipUpdateCode stuck true. Handle the null return (reset the flag and return React\Promise\resolve(null) or similar) so the flag is always cleared.

Suggested change
return parent::update($repo, $initial, $target)->then(
$promise = parent::update($repo, $initial, $target);
if ($promise === null) {
$resetFlag();
return \React\Promise\resolve(null);
}
return $promise->then(

Copilot uses AI. Check for mistakes.
$resetFlag,
Comment on lines +321 to +322

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior in delegateRepoTracking() (skip-flag lifecycle + parent update delegation) isn’t covered by tests. Adding tests for (1) flag reset on success, and (2) flag reset when parent::update() rejects/throws would help prevent regressions, especially since leaving the flag enabled would break subsequent updates.

Suggested change
return parent::update($repo, $initial, $target)->then(
$resetFlag,
try {
$promise = parent::update($repo, $initial, $target);
} catch (\Throwable $e) {
$resetFlag();
throw $e;
}
return $promise->then(
function ($result) use ($resetFlag) {
$resetFlag();
return $result;
},

Copilot uses AI. Check for mistakes.
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(" - <info>Skipping update for path repository:</info> {$target->getPrettyName()}");

return \React\Promise\resolve(null);
}
Comment on lines 369 to +378

Copilot AI Apr 26, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says path-repo install/update/uninstall should skip destructive file ops while still updating Composer’s internal tracking state. In update() the path-repo branch returns a resolved promise without calling the repo-tracking delegation logic, so installed.json/repo state may not be updated. Please clarify intended behavior and, if tracking should still occur, delegate to the parent tracking path here too (while keeping code updates skipped).

Copilot uses AI. Check for mistakes.

$installPath = $this->getInstallPath($target);
$stashPath = null;
$basePath = null;

Expand All @@ -302,31 +386,65 @@ 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);
}
if ($basePath !== null) {
(new Filesystem)->removeDirectory($basePath);
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;
}
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(" - <info>Skipping uninstall for path repository:</info> {$package->getPrettyName()}");

if ($repo->hasPackage($package)) {
$repo->removePackage($package);
}
);

return \React\Promise\resolve(null);
}

return parent::uninstall($repo, $package);
}
}
173 changes: 173 additions & 0 deletions tests/ModuleInstallerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
use Composer\Package\PackageInterface;
use Composer\Package\RootPackage;
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;
Expand Down Expand Up @@ -58,6 +60,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): 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): PromiseInterface
{
$this->parentUpdateCodeInvoked = true;

return parent::invokeParentUpdateCode($initial, $target);
}
}

final class ModuleInstallerTest extends TestCase
Expand Down Expand Up @@ -470,4 +496,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');
}
}