From dab2173868ec8939bf4a30b37bfb7050e5505076 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 7 Jan 2025 07:48:46 +0000 Subject: [PATCH 1/6] Reword package skip message in PiePackageInstaller --- src/ComposerIntegration/PiePackageInstaller.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ComposerIntegration/PiePackageInstaller.php b/src/ComposerIntegration/PiePackageInstaller.php index 3604907..d2185cc 100644 --- a/src/ComposerIntegration/PiePackageInstaller.php +++ b/src/ComposerIntegration/PiePackageInstaller.php @@ -12,6 +12,7 @@ use Composer\Repository\InstalledRepositoryInterface; use Composer\Util\Filesystem; use Php\Pie\ExtensionType; +use Symfony\Component\Console\Output\OutputInterface; use function sprintf; @@ -39,11 +40,14 @@ public function install(InstalledRepositoryInterface $repo, PackageInterface $pa $output = $this->composerRequest->pieOutput; if ($this->composerRequest->requestedPackage->package !== $composerPackage->getName()) { - $output->writeln(sprintf( - 'Not using PIE to install %s as it was not the expected package %s', - $composerPackage->getName(), - $this->composerRequest->requestedPackage->package, - )); + $output->writeln( + sprintf( + 'Skipping %s install request from Composer as it was not the expected PIE package %s', + $composerPackage->getName(), + $this->composerRequest->requestedPackage->package, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); return null; } From 66f8d9e2c04c24b7b90306dc7be3f154ec9fcfa7 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 7 Jan 2025 10:19:36 +0000 Subject: [PATCH 2/6] Add a VendorCleanup process to remove all non-essential vendor-dir components --- src/ComposerIntegration/VendorCleanup.php | 89 +++++++++++++++++++ test/assets/vendor-cleanup-dir/autoload.php | 0 test/assets/vendor-cleanup-dir/composer/dummy | 0 test/assets/vendor-cleanup-dir/vendor1/dummy | 0 test/assets/vendor-cleanup-dir/vendor2/dummy | 0 .../ComposerIntegration/VendorCleanupTest.php | 82 +++++++++++++++++ 6 files changed, 171 insertions(+) create mode 100644 src/ComposerIntegration/VendorCleanup.php create mode 100644 test/assets/vendor-cleanup-dir/autoload.php create mode 100644 test/assets/vendor-cleanup-dir/composer/dummy create mode 100644 test/assets/vendor-cleanup-dir/vendor1/dummy create mode 100644 test/assets/vendor-cleanup-dir/vendor2/dummy create mode 100644 test/unit/ComposerIntegration/VendorCleanupTest.php diff --git a/src/ComposerIntegration/VendorCleanup.php b/src/ComposerIntegration/VendorCleanup.php new file mode 100644 index 0000000..675ee7b --- /dev/null +++ b/src/ComposerIntegration/VendorCleanup.php @@ -0,0 +1,89 @@ +getConfig()->get('vendor-dir'); + $vendorContents = scandir($vendorDir); + + if (! is_array($vendorContents)) { + $this->output->writeln( + sprintf( + 'Vendor directory (vendor-dir config) %s seemed invalid?/comment>', + $vendorDir, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + + return; + } + + $toRemove = array_filter( + $vendorContents, + static function (string $path): bool { + return ! in_array( + $path, + [ + '.', + '..', + 'autoload.php', + 'composer', + ], + ); + }, + ); + + array_walk( + $toRemove, + function (string $pathToRemove) use ($vendorDir): void { + $this->output->writeln( + sprintf( + 'Removing: %s/%s', + $vendorDir, + $pathToRemove, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + + $fullPathToRemove = $vendorDir . DIRECTORY_SEPARATOR . $pathToRemove; + + if ($this->filesystem->remove($fullPathToRemove)) { + return; + } + + $this->output->writeln( + sprintf( + 'Warning: failed to remove %s', + $fullPathToRemove, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + }, + ); + } +} diff --git a/test/assets/vendor-cleanup-dir/autoload.php b/test/assets/vendor-cleanup-dir/autoload.php new file mode 100644 index 0000000..e69de29 diff --git a/test/assets/vendor-cleanup-dir/composer/dummy b/test/assets/vendor-cleanup-dir/composer/dummy new file mode 100644 index 0000000..e69de29 diff --git a/test/assets/vendor-cleanup-dir/vendor1/dummy b/test/assets/vendor-cleanup-dir/vendor1/dummy new file mode 100644 index 0000000..e69de29 diff --git a/test/assets/vendor-cleanup-dir/vendor2/dummy b/test/assets/vendor-cleanup-dir/vendor2/dummy new file mode 100644 index 0000000..e69de29 diff --git a/test/unit/ComposerIntegration/VendorCleanupTest.php b/test/unit/ComposerIntegration/VendorCleanupTest.php new file mode 100644 index 0000000..b50ed90 --- /dev/null +++ b/test/unit/ComposerIntegration/VendorCleanupTest.php @@ -0,0 +1,82 @@ +output = $this->createMock(OutputInterface::class); + $this->filesystem = $this->createMock(Filesystem::class); + + $this->vendorCleanup = new VendorCleanup($this->output, $this->filesystem); + } + + private function composerWithVendorDirConfig(string $vendorDirConfig): Composer&MockObject + { + $config = $this->createMock(Config::class); + $config->method('get') + ->with('vendor-dir') + ->willReturn($vendorDirConfig); + + $composer = $this->createMock(Composer::class); + $composer->method('getConfig') + ->willReturn($config); + + return $composer; + } + + public function testInvalidVendorDirectory(): void + { + $this->output + ->expects(self::once()) + ->method('writeln') + ->with( + 'Vendor directory (vendor-dir config) /path/that/does/not/exist seemed invalid?/comment>', + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + + ($this->vendorCleanup)($this->composerWithVendorDirConfig('/path/that/does/not/exist')); + } + + public function testVendorDirIsCleaned(): void + { + $vendor1Removed = false; + $vendor2Removed = false; + + $this->filesystem + ->expects(self::exactly(2)) + ->method('remove') + ->willReturnCallback(static function (string $path) use (&$vendor1Removed, &$vendor2Removed): bool { + return match ($path) { + self::VENDOR_DIR . '/vendor1' => $vendor1Removed = true, + self::VENDOR_DIR . '/vendor2' => $vendor2Removed = true, + }; + }); + + ($this->vendorCleanup)($this->composerWithVendorDirConfig(self::VENDOR_DIR)); + + self::assertTrue($vendor1Removed); + self::assertTrue($vendor2Removed); + } +} From 213b49b306290825ad1b135793b64f3874ee7a0f Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 8 Jan 2025 09:45:40 +0000 Subject: [PATCH 3/6] Added component to remove unrelated install operations --- .../RemoveUnrelatedInstallOperations.php | 90 ++++++++++++ .../RemoveUnrelatedInstallOperationsTest.php | 136 ++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 src/ComposerIntegration/RemoveUnrelatedInstallOperations.php create mode 100644 test/unit/ComposerIntegration/RemoveUnrelatedInstallOperationsTest.php diff --git a/src/ComposerIntegration/RemoveUnrelatedInstallOperations.php b/src/ComposerIntegration/RemoveUnrelatedInstallOperations.php new file mode 100644 index 0000000..874ce7e --- /dev/null +++ b/src/ComposerIntegration/RemoveUnrelatedInstallOperations.php @@ -0,0 +1,90 @@ +getEventDispatcher() + ->addListener( + InstallerEvents::PRE_OPERATIONS_EXEC, + new self($composerRequest), + ); + } + + /** + * @psalm-suppress InternalProperty + * @psalm-suppress InternalMethod + */ + public function __invoke(InstallerEvent $installerEvent): void + { + $pieOutput = $this->composerRequest->pieOutput; + + $newOperations = array_filter( + $installerEvent->getTransaction()?->getOperations() ?? [], + function (OperationInterface $operation) use ($pieOutput): bool { + if (! $operation instanceof InstallOperation) { + $pieOutput->writeln( + sprintf( + 'Unexpected operation during installer: %s', + $operation::class, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + + return false; + } + + $isRequestedPiePackage = $this->composerRequest->requestedPackage->package === $operation->getPackage()->getName(); + + if (! $isRequestedPiePackage) { + $pieOutput->writeln( + sprintf( + 'Filtering package %s from install operations, as it was not the requested package', + $operation->getPackage()->getName(), + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + } + + return $isRequestedPiePackage; + }, + ); + + $overrideOperations = Closure::Bind( + static function (Transaction $transaction) use ($newOperations): void { + /** @psalm-suppress InaccessibleProperty */ + $transaction->operations = $newOperations; + }, + null, + Transaction::class, + ); + assert($overrideOperations !== null); + $overrideOperations($installerEvent->getTransaction()); + } +} diff --git a/test/unit/ComposerIntegration/RemoveUnrelatedInstallOperationsTest.php b/test/unit/ComposerIntegration/RemoveUnrelatedInstallOperationsTest.php new file mode 100644 index 0000000..0703e3b --- /dev/null +++ b/test/unit/ComposerIntegration/RemoveUnrelatedInstallOperationsTest.php @@ -0,0 +1,136 @@ +composer = $this->createMock(Composer::class); + } + + public function testEventListenerRegistration(): void + { + $eventDispatcher = $this->createMock(EventDispatcher::class); + $eventDispatcher + ->expects(self::once()) + ->method('addListener') + ->with( + InstallerEvents::PRE_OPERATIONS_EXEC, + self::isInstanceOf(RemoveUnrelatedInstallOperations::class), + ); + + $this->composer + ->expects(self::once()) + ->method('getEventDispatcher') + ->willReturn($eventDispatcher); + + RemoveUnrelatedInstallOperations::selfRegister( + $this->composer, + new PieComposerRequest( + $this->createMock(OutputInterface::class), + new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + PhpBinaryPath::fromCurrentProcess(), + Architecture::x86_64, + ThreadSafetyMode::NonThreadSafe, + 1, + WindowsCompiler::VC15, + ), + new RequestedPackageAndVersion('foo/bar', '^1.1'), + PieOperation::Install, + [], + null, + false, + ), + ); + } + + /** @psalm-suppress InternalMethod */ + public function testUnrelatedInstallOperationsAreRemoved(): void + { + $composerPackage1 = new CompletePackage('foo/bar', '1.2.3.0', '1.2.3'); + $composerPackage2 = new CompletePackage('bat/baz', '3.4.5.0', '3.4.5'); + $composerPackage3 = new CompletePackage('qux/quux', '5.6.7.0', '5.6.7'); + + /** + * @psalm-suppress InternalClass + * @psalm-suppress InternalMethod + */ + $installerEvent = new InstallerEvent( + InstallerEvents::PRE_OPERATIONS_EXEC, + $this->composer, + $this->createMock(IOInterface::class), + false, + true, + new Transaction([], [$composerPackage1, $composerPackage2, $composerPackage3]), + ); + + (new RemoveUnrelatedInstallOperations( + new PieComposerRequest( + $this->createMock(OutputInterface::class), + new TargetPlatform( + OperatingSystem::Windows, + OperatingSystemFamily::Linux, + PhpBinaryPath::fromCurrentProcess(), + Architecture::x86_64, + ThreadSafetyMode::NonThreadSafe, + 1, + WindowsCompiler::VC15, + ), + new RequestedPackageAndVersion('bat/baz', '^3.2'), + PieOperation::Install, + [], + null, + false, + ), + ))($installerEvent); + + self::assertSame( + ['bat/baz'], + array_map( + static fn (InstallOperation $operation): string => $operation->getPackage()->getName(), + array_filter( + $installerEvent->getTransaction()?->getOperations() ?? [], + static fn (OperationInterface $operation): bool => $operation instanceof InstallOperation, + ), + ), + ); + } +} From 20fc62cf0b31e1f1a284be68d2266f65e13d9acc Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 8 Jan 2025 09:53:08 +0000 Subject: [PATCH 4/6] Enable vendor cleanup components in ComposerIntegration --- src/ComposerIntegration/ComposerIntegrationHandler.php | 3 +++ src/ComposerIntegration/PieComposerFactory.php | 1 + 2 files changed, 4 insertions(+) diff --git a/src/ComposerIntegration/ComposerIntegrationHandler.php b/src/ComposerIntegration/ComposerIntegrationHandler.php index 05d9c8b..d246a49 100644 --- a/src/ComposerIntegration/ComposerIntegrationHandler.php +++ b/src/ComposerIntegration/ComposerIntegrationHandler.php @@ -25,6 +25,7 @@ class ComposerIntegrationHandler public function __construct( private readonly ContainerInterface $container, private readonly QuieterConsoleIO $arrayCollectionIo, + private readonly VendorCleanup $vendorCleanup, ) { } @@ -86,5 +87,7 @@ public function __invoke( throw ComposerRunFailed::fromExitCode($resultCode); } + + ($this->vendorCleanup)($composer); } } diff --git a/src/ComposerIntegration/PieComposerFactory.php b/src/ComposerIntegration/PieComposerFactory.php index 97063ab..d135e61 100644 --- a/src/ComposerIntegration/PieComposerFactory.php +++ b/src/ComposerIntegration/PieComposerFactory.php @@ -75,6 +75,7 @@ public static function createPieComposer( ); OverrideWindowsUrlInstallListener::selfRegister($composer, $io, $container, $composerRequest); + RemoveUnrelatedInstallOperations::selfRegister($composer, $composerRequest); $composer->getConfig()->merge(['config' => ['__PIE_REQUEST__' => $composerRequest]]); $io->loadConfiguration($composer->getConfig()); From 3b18514a834f744ac0fa8776af82cff6235b75ca Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 8 Jan 2025 10:29:08 +0000 Subject: [PATCH 5/6] Handle warning emitted by scandir --- .../ComposerIntegration/VendorCleanupTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/unit/ComposerIntegration/VendorCleanupTest.php b/test/unit/ComposerIntegration/VendorCleanupTest.php index b50ed90..cb96158 100644 --- a/test/unit/ComposerIntegration/VendorCleanupTest.php +++ b/test/unit/ComposerIntegration/VendorCleanupTest.php @@ -13,6 +13,11 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Output\OutputInterface; +use function restore_error_handler; +use function set_error_handler; + +use const E_WARNING; + #[CoversClass(VendorCleanup::class)] final class VendorCleanupTest extends TestCase { @@ -56,7 +61,19 @@ public function testInvalidVendorDirectory(): void OutputInterface::VERBOSITY_VERY_VERBOSE, ); + /** + * scandir will emit a warning in this case, causing phpunit to fail with warning + * + * @psalm-suppress InvalidArgument + */ + set_error_handler( + static function (): bool|null { + return null; + }, + E_WARNING, + ); ($this->vendorCleanup)($this->composerWithVendorDirConfig('/path/that/does/not/exist')); + restore_error_handler(); } public function testVendorDirIsCleaned(): void From 4ae2f35e366bcf5d38147f1b94cf654a1985d3f8 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 9 Jan 2025 08:03:07 +0000 Subject: [PATCH 6/6] Fix VendorCleanup for Windows --- src/ComposerIntegration/VendorCleanup.php | 9 ++++----- test/unit/ComposerIntegration/VendorCleanupTest.php | 5 +++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ComposerIntegration/VendorCleanup.php b/src/ComposerIntegration/VendorCleanup.php index 675ee7b..035c2d8 100644 --- a/src/ComposerIntegration/VendorCleanup.php +++ b/src/ComposerIntegration/VendorCleanup.php @@ -61,17 +61,16 @@ static function (string $path): bool { array_walk( $toRemove, function (string $pathToRemove) use ($vendorDir): void { + $fullPathToRemove = $vendorDir . DIRECTORY_SEPARATOR . $pathToRemove; + $this->output->writeln( sprintf( - 'Removing: %s/%s', - $vendorDir, - $pathToRemove, + 'Removing: %s', + $fullPathToRemove, ), OutputInterface::VERBOSITY_VERY_VERBOSE, ); - $fullPathToRemove = $vendorDir . DIRECTORY_SEPARATOR . $pathToRemove; - if ($this->filesystem->remove($fullPathToRemove)) { return; } diff --git a/test/unit/ComposerIntegration/VendorCleanupTest.php b/test/unit/ComposerIntegration/VendorCleanupTest.php index cb96158..0d45528 100644 --- a/test/unit/ComposerIntegration/VendorCleanupTest.php +++ b/test/unit/ComposerIntegration/VendorCleanupTest.php @@ -16,6 +16,7 @@ use function restore_error_handler; use function set_error_handler; +use const DIRECTORY_SEPARATOR; use const E_WARNING; #[CoversClass(VendorCleanup::class)] @@ -86,8 +87,8 @@ public function testVendorDirIsCleaned(): void ->method('remove') ->willReturnCallback(static function (string $path) use (&$vendor1Removed, &$vendor2Removed): bool { return match ($path) { - self::VENDOR_DIR . '/vendor1' => $vendor1Removed = true, - self::VENDOR_DIR . '/vendor2' => $vendor2Removed = true, + self::VENDOR_DIR . DIRECTORY_SEPARATOR . 'vendor1' => $vendor1Removed = true, + self::VENDOR_DIR . DIRECTORY_SEPARATOR . 'vendor2' => $vendor2Removed = true, }; });