Skip to content

feat: enhance isPathRepository logic to differentiate between symlink… #15

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
Jun 5, 2026
Merged

feat: enhance isPathRepository logic to differentiate between symlink… #15

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
9ebc92c
feat: enhance isPathRepository logic to differentiate between symlink…
roble Jun 5, 2026
eecc662
PHP Linting (Pint)
roble Jun 5, 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
13 changes: 12 additions & 1 deletion src/Installer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,21 @@ 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.
*
* With symlink:true, a genuine path repo creates a real OS symlink. A module installed
* from Packagist into the same directory is a real directory — not a true path repo.
*/
protected function isPathRepository(PackageInterface $package): bool
{
return $package->getDistType() === 'path';
if ($package->getDistType() !== 'path') {
return false;
}

$path = $this->getInstallPath($package);

// Genuine path repos: OS symlink (symlink:true) OR a git-tracked dev clone (.git present).
// Packagist-installed modules have .git removed by DEFAULT_EXCLUDED_DIRS — they are neither.
return is_link($path) || is_dir($path.'/.git');
Comment on lines +294 to +298
}

/**
Expand Down
93 changes: 68 additions & 25 deletions tests/ModuleInstallerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,17 @@ public function callFlattenFrameworkFiles(string $jsRoot, string $framework): vo
parent::flattenFrameworkFiles($jsRoot, $framework);
}

public ?bool $forcePathRepository = null;

protected function isPathRepository(PackageInterface $package): bool
{
if ($this->forcePathRepository !== null) {
return $this->forcePathRepository;
}

return parent::isPathRepository($package);
}

public bool $copyFrameworkFilesInvoked = false;

protected function copyFrameworkFiles(PackageInterface $package): void
Expand Down Expand Up @@ -543,39 +554,71 @@ public function test_restore_stash_discards_stash_when_original_path_already_exi
// isPathRepository
// -------------------------------------------------------------------------

public function test_is_path_repository_returns_true_for_path_dist_type(): void
public function test_is_path_repository_returns_true_when_dist_type_is_path_and_install_path_is_symlink(): void
{
$io = $this->createStub(IOInterface::class);
$installer = new TestableInstaller($io, null);
$baseDir = sys_get_temp_dir().'/path-repo-symlink-'.uniqid('', true);
$target = $baseDir.'/source';
mkdir($target, 0755, true);
symlink($target, $baseDir.'/test-module');

$pkg = $this->createStub(PackageInterface::class);
$pkg->method('getDistType')->willReturn('path');
$installer = $this->makeInstallerWithModuleDir($baseDir);
$pkg = new Package('saucebase/test-module', '1.0.0.0', '1.0.0');
$pkg->setDistType('path');

$this->assertTrue($installer->callIsPathRepository($pkg));

(new Filesystem)->remove($baseDir);
}

public function test_is_path_repository_returns_false_for_zip_dist_type(): void
public function test_is_path_repository_returns_false_when_dist_type_is_path_but_install_path_is_real_directory(): void
{
$io = $this->createStub(IOInterface::class);
$installer = new TestableInstaller($io, null);
// Bug scenario: module installed from Packagist lands in modules/ as a real directory.
$baseDir = sys_get_temp_dir().'/path-repo-realdir-'.uniqid('', true);
mkdir($baseDir.'/test-module', 0755, true);

$pkg = $this->createStub(PackageInterface::class);
$pkg->method('getDistType')->willReturn('zip');
$installer = $this->makeInstallerWithModuleDir($baseDir);
$pkg = new Package('saucebase/test-module', '1.0.0.0', '1.0.0');
$pkg->setDistType('path');

$this->assertFalse($installer->callIsPathRepository($pkg));

(new Filesystem)->remove($baseDir);
}

public function test_is_path_repository_returns_false_when_dist_type_is_null(): void
public function test_is_path_repository_returns_false_when_dist_type_is_path_but_install_path_does_not_exist(): void
{
$io = $this->createStub(IOInterface::class);
$installer = new TestableInstaller($io, null);

$pkg = $this->createStub(PackageInterface::class);
$pkg->method('getDistType')->willReturn(null);
$installer = new TestableInstaller($this->createStub(IOInterface::class), null);
$pkg = new Package('saucebase/nonexistent-module', '1.0.0.0', '1.0.0');
$pkg->setDistType('path');
Comment on lines +590 to +592

$this->assertFalse($installer->callIsPathRepository($pkg));
}

public function test_is_path_repository_returns_true_when_dist_type_is_path_and_install_path_has_git_dir(): void
{
$baseDir = sys_get_temp_dir().'/path-repo-gitclone-'.uniqid('', true);
mkdir($baseDir.'/test-module/.git', 0755, true);

$installer = $this->makeInstallerWithModuleDir($baseDir);
$pkg = new Package('saucebase/test-module', '1.0.0.0', '1.0.0');
$pkg->setDistType('path');

$this->assertTrue($installer->callIsPathRepository($pkg));

(new Filesystem)->remove($baseDir);
}

public function test_is_path_repository_returns_false_for_non_path_dist_types(): void
{
$installer = new TestableInstaller($this->createStub(IOInterface::class), null);

foreach (['zip', 'tar', null] as $distType) {
$pkg = $this->createStub(PackageInterface::class);
$pkg->method('getDistType')->willReturn($distType);
$this->assertFalse($installer->callIsPathRepository($pkg), "Expected false for distType=$distType");
}
}

// -------------------------------------------------------------------------
// install() path-repository guard
// -------------------------------------------------------------------------
Expand All @@ -588,11 +631,11 @@ public function test_install_logs_skip_message_and_returns_promise_for_path_repo
->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);
$installer->forcePathRepository = true;

$promise = $installer->install($repo, $pkg);

Expand All @@ -604,14 +647,14 @@ public function test_install_registers_package_in_repo_when_not_already_present_
$io = $this->createStub(IOInterface::class);

$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->once())->method('addPackage');

$installer = new TestableInstaller($io, null);
$installer->forcePathRepository = true;
$installer->install($repo, $pkg);
}

Expand All @@ -620,14 +663,14 @@ public function test_install_does_not_register_package_in_repo_when_already_pres
$io = $this->createStub(IOInterface::class);

$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->never())->method('addPackage');

$installer = new TestableInstaller($io, null);
$installer->forcePathRepository = true;
$installer->install($repo, $pkg);
}

Expand All @@ -645,11 +688,11 @@ public function test_update_logs_skip_message_and_returns_promise_for_path_repo(
$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);
$installer->forcePathRepository = true;

$promise = $installer->update($repo, $initial, $target);

Expand All @@ -663,7 +706,6 @@ public function test_update_registers_package_in_repo_when_not_already_present_f
$initial = $this->createStub(PackageInterface::class);

$target = $this->createStub(PackageInterface::class);
$target->method('getDistType')->willReturn('path');
$target->method('getPrettyName')->willReturn('saucebase/test');

$repo = $this->createMock(InstalledRepositoryInterface::class);
Expand All @@ -672,6 +714,7 @@ public function test_update_registers_package_in_repo_when_not_already_present_f
$repo->expects($this->once())->method('addPackage');

$installer = new TestableInstaller($io, null);
$installer->forcePathRepository = true;
$installer->update($repo, $initial, $target);
}

Expand All @@ -682,7 +725,6 @@ public function test_update_replaces_initial_with_target_in_repo_for_path_repo()
$initial = $this->createStub(PackageInterface::class);

$target = $this->createStub(PackageInterface::class);
$target->method('getDistType')->willReturn('path');
$target->method('getPrettyName')->willReturn('saucebase/test');

$repo = $this->createMock(InstalledRepositoryInterface::class);
Expand All @@ -692,6 +734,7 @@ public function test_update_replaces_initial_with_target_in_repo_for_path_repo()
$repo->expects($this->once())->method('addPackage');

$installer = new TestableInstaller($io, null);
$installer->forcePathRepository = true;
$installer->update($repo, $initial, $target);
}

Expand All @@ -702,7 +745,6 @@ public function test_update_does_not_register_package_in_repo_when_already_prese
$initial = $this->createStub(PackageInterface::class);

$target = $this->createStub(PackageInterface::class);
$target->method('getDistType')->willReturn('path');
$target->method('getPrettyName')->willReturn('saucebase/test');

$repo = $this->createMock(InstalledRepositoryInterface::class);
Expand All @@ -711,6 +753,7 @@ public function test_update_does_not_register_package_in_repo_when_already_prese
$repo->expects($this->never())->method('addPackage');

$installer = new TestableInstaller($io, null);
$installer->forcePathRepository = true;
$installer->update($repo, $initial, $target);
}

Expand All @@ -726,14 +769,14 @@ public function test_uninstall_logs_skip_message_and_removes_package_from_repo()
->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->forcePathRepository = true;
$installer->uninstall($repo, $pkg);
}

Expand All @@ -743,14 +786,14 @@ public function test_uninstall_skips_remove_package_when_package_not_in_repo():
$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->forcePathRepository = true;
$installer->uninstall($repo, $pkg);
}

Expand Down