From 3fccef188309f1d6789424567bcd7ffefdbd6a8b Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 7 Feb 2025 12:04:43 +0000 Subject: [PATCH 01/25] Fix help for package+version command hint --- src/Command/CommandHelper.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index 8614fd2..a456314 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -64,6 +64,12 @@ public static function configurePhpConfigOptions(Command $command): void InputOption::VALUE_REQUIRED, 'The path to the `php` binary to use as the target PHP platform on ' . OperatingSystem::Windows->asFriendlyName() . ', e.g. --' . self::OPTION_WITH_PHP_PATH . '=C:\usr\php7.4.33\php.exe', ); + $command->addOption( + self::OPTION_WITH_PHPIZE_PATH, + null, + InputOption::VALUE_REQUIRED, + 'The path to the `phpize` binary to use as the target PHP platform, e.g. --' . self::OPTION_WITH_PHPIZE_PATH . '=/usr/bin/phpize7.4', + ); } public static function configureDownloadBuildInstallOptions(Command $command): void @@ -71,7 +77,7 @@ public static function configureDownloadBuildInstallOptions(Command $command): v $command->addArgument( self::ARG_REQUESTED_PACKAGE_AND_VERSION, InputArgument::REQUIRED, - 'The extension name and version constraint to use, in the format {ext-name}{?:{?version-constraint}{?@stability}}, for example `xdebug/xdebug:^3.4@alpha`, `xdebug/xdebug:@alpha`, `xdebug/xdebug:^3.4`, etc.', + 'The PIE package name and version constraint to use, in the format {vendor/package}{?:{?version-constraint}{?@stability}}, for example `xdebug/xdebug:^3.4@alpha`, `xdebug/xdebug:@alpha`, `xdebug/xdebug:^3.4`, etc.', ); $command->addOption( self::OPTION_MAKE_PARALLEL_JOBS, @@ -79,12 +85,6 @@ public static function configureDownloadBuildInstallOptions(Command $command): v InputOption::VALUE_REQUIRED, 'Override many jobs to run in parallel when running compiling (this is passed to "make -jN" during build). PIE will try to detect this by default.', ); - $command->addOption( - self::OPTION_WITH_PHPIZE_PATH, - null, - InputOption::VALUE_REQUIRED, - 'The path to the `phpize` binary to use as the target PHP platform, e.g. --' . self::OPTION_WITH_PHPIZE_PATH . '=/usr/bin/phpize7.4', - ); $command->addOption( self::OPTION_SKIP_ENABLE_EXTENSION, null, From ea263078fd98b2fd1d9e8121029c89c349ba90c3 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 7 Feb 2025 12:12:05 +0000 Subject: [PATCH 02/25] Split out PIE package installed.json processor --- src/Command/ShowCommand.php | 53 +------------- src/Platform/InstalledPiePackages.php | 71 +++++++++++++++++++ .../Platform/InstalledPiePackagesTest.php | 18 +++++ 3 files changed, 92 insertions(+), 50 deletions(-) create mode 100644 src/Platform/InstalledPiePackages.php create mode 100644 test/unit/Platform/InstalledPiePackagesTest.php diff --git a/src/Command/ShowCommand.php b/src/Command/ShowCommand.php index 2c07282..ad9c9dd 100644 --- a/src/Command/ShowCommand.php +++ b/src/Command/ShowCommand.php @@ -4,25 +4,16 @@ namespace Php\Pie\Command; -use Composer\Package\BasePackage; -use Composer\Package\CompletePackageInterface; use Php\Pie\BinaryFile; -use Php\Pie\ComposerIntegration\PieComposerFactory; -use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; -use Php\Pie\DependencyResolver\Package; +use Php\Pie\Platform\InstalledPiePackages; use Php\Pie\Platform\OperatingSystem; -use Php\Pie\Platform\TargetPlatform; -use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; -use function array_combine; -use function array_filter; use function array_key_exists; -use function array_map; use function array_walk; use function file_exists; use function sprintf; @@ -38,7 +29,7 @@ final class ShowCommand extends Command { public function __construct( - private readonly ContainerInterface $container, + private readonly InstalledPiePackages $installedPiePackages, ) { parent::__construct(); } @@ -54,7 +45,7 @@ public function execute(InputInterface $input, OutputInterface $output): int { $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); - $piePackages = $this->buildListOfPieInstalledPackages($output, $targetPlatform); + $piePackages = $this->installedPiePackages->allPiePackages($targetPlatform); $phpEnabledExtensions = $targetPlatform->phpBinaryPath->extensions(); $extensionPath = $targetPlatform->phpBinaryPath->extensionPath(); $extensionEnding = $targetPlatform->operatingSystem === OperatingSystem::Windows ? '.dll' : '.so'; @@ -121,42 +112,4 @@ private static function verifyChecksumInformation( return ' ✅'; } - - /** @return array */ - private function buildListOfPieInstalledPackages( - OutputInterface $output, - TargetPlatform $targetPlatform, - ): array { - $composerInstalledPackages = array_map( - static function (CompletePackageInterface $package): Package { - return Package::fromComposerCompletePackage($package); - }, - array_filter( - PieComposerFactory::createPieComposer( - $this->container, - PieComposerRequest::noOperation( - $output, - $targetPlatform, - ), - ) - ->getRepositoryManager() - ->getLocalRepository() - ->getPackages(), - static function (BasePackage $basePackage): bool { - return $basePackage instanceof CompletePackageInterface; - }, - ), - ); - - return array_combine( - array_map( - /** @return non-empty-string */ - static function (Package $package): string { - return $package->extensionName()->name(); - }, - $composerInstalledPackages, - ), - $composerInstalledPackages, - ); - } } diff --git a/src/Platform/InstalledPiePackages.php b/src/Platform/InstalledPiePackages.php new file mode 100644 index 0000000..aa1e496 --- /dev/null +++ b/src/Platform/InstalledPiePackages.php @@ -0,0 +1,71 @@ + + */ +class InstalledPiePackages +{ + /** @psalm-suppress PossiblyUnusedMethod no direct reference; used in service locator */ + public function __construct(private readonly ContainerInterface $container) + { + } + + /** + * Returns a list of PIE packages according to PIE; this does NOT check if + * the extension is actually enabled in the target PHP. + * + * @return ListOfPiePackages + */ + public function allPiePackages(TargetPlatform $targetPlatform): array + { + $composerInstalledPackages = array_map( + static function (CompletePackageInterface $package): Package { + return Package::fromComposerCompletePackage($package); + }, + array_filter( + PieComposerFactory::createPieComposer( + $this->container, + PieComposerRequest::noOperation( + new NullOutput(), + $targetPlatform, + ), + ) + ->getRepositoryManager() + ->getLocalRepository() + ->getPackages(), + static function (BasePackage $basePackage): bool { + return $basePackage instanceof CompletePackageInterface; + }, + ), + ); + + return array_combine( + array_map( + /** @return non-empty-string */ + static function (Package $package): string { + return $package->extensionName()->name(); + }, + $composerInstalledPackages, + ), + $composerInstalledPackages, + ); + } +} diff --git a/test/unit/Platform/InstalledPiePackagesTest.php b/test/unit/Platform/InstalledPiePackagesTest.php new file mode 100644 index 0000000..628ca7d --- /dev/null +++ b/test/unit/Platform/InstalledPiePackagesTest.php @@ -0,0 +1,18 @@ + Date: Fri, 7 Feb 2025 12:12:34 +0000 Subject: [PATCH 03/25] Do not purge packages so they remain available in pie show even after vendor cleanup --- src/ComposerIntegration/PieComposerFactory.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/ComposerIntegration/PieComposerFactory.php b/src/ComposerIntegration/PieComposerFactory.php index 7acb235..d3ace61 100644 --- a/src/ComposerIntegration/PieComposerFactory.php +++ b/src/ComposerIntegration/PieComposerFactory.php @@ -9,6 +9,7 @@ use Composer\Installer; use Composer\IO\IOInterface; use Composer\PartialComposer; +use Composer\Repository\InstalledRepositoryInterface; use Composer\Util\Filesystem; use Composer\Util\ProcessExecutor; use Php\Pie\ComposerIntegration\Listeners\OverrideDownloadUrlInstallListener; @@ -70,6 +71,21 @@ public static function createPieComposer( return $composer; } + protected function purgePackages(InstalledRepositoryInterface $repo, Installer\InstallationManager $im): void + { + /** + * This is intentionally a no-op in PIE.... + * + * Why not purge packages? + * + * We have a post install job in {@see VendorCleanup} that cleans up the vendor directory to remove all the + * actual package files; however, this means that Composer thinks they are not installed after that. When + * creating the Composer instance, the last step is to purge packages from the + * {@see InstalledRepositoryInterface} if they no longer exist on disk. But, that means we can't list the + * packages installed with PIE any more! So, we override this method to become a no-op ✅ + */ + } + public static function recreatePieComposer( ContainerInterface $container, Composer $existingComposer, From ff18d8ef8be717f61a2d0eb7758dff5263ad9ff4 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 12 Feb 2025 08:55:46 +0000 Subject: [PATCH 04/25] Improve BinaryFile with verify methods --- src/BinaryFile.php | 24 +++++++ src/Command/ShowCommand.php | 21 ++++-- .../BinaryFileFailedVerification.php | 33 +++++++++ src/Util/FileNotFound.php | 20 ++++++ test/unit/BinaryFileTest.php | 67 +++++++++++++++++++ 5 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 src/Installing/BinaryFileFailedVerification.php create mode 100644 src/Util/FileNotFound.php create mode 100644 test/unit/BinaryFileTest.php diff --git a/src/BinaryFile.php b/src/BinaryFile.php index 5461118..50cac2e 100644 --- a/src/BinaryFile.php +++ b/src/BinaryFile.php @@ -4,6 +4,9 @@ namespace Php\Pie; +use Php\Pie\Installing\BinaryFileFailedVerification; + +use function file_exists; use function hash_file; /** @@ -33,4 +36,25 @@ public static function fromFileWithSha256Checksum(string $filePath): self hash_file(self::HASH_TYPE_SHA256, $filePath), ); } + + public function verify(): void + { + if (! file_exists($this->filePath)) { + throw Util\FileNotFound::fromFilename($this->filePath); + } + + self::verifyAgainstOther(self::fromFileWithSha256Checksum($this->filePath)); + } + + /** @throws BinaryFileFailedVerification */ + public function verifyAgainstOther(self $other): void + { + if ($this->filePath !== $other->filePath) { + throw BinaryFileFailedVerification::fromFilenameMismatch($this, $other); + } + + if ($other->checksum !== $this->checksum) { + throw BinaryFileFailedVerification::fromChecksumMismatch($this, $other); + } + } } diff --git a/src/Command/ShowCommand.php b/src/Command/ShowCommand.php index ad9c9dd..6986784 100644 --- a/src/Command/ShowCommand.php +++ b/src/Command/ShowCommand.php @@ -6,6 +6,7 @@ use Php\Pie\BinaryFile; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; +use Php\Pie\Installing\BinaryFileFailedVerification; use Php\Pie\Platform\InstalledPiePackages; use Php\Pie\Platform\OperatingSystem; use Symfony\Component\Console\Attribute\AsCommand; @@ -90,10 +91,10 @@ private static function verifyChecksumInformation( string $extensionEnding, array $installedJsonMetadata, ): string { - $expectedConventionalBinaryPath = $extensionPath . DIRECTORY_SEPARATOR . $phpExtensionName . $extensionEnding; + $actualBinaryPathByConvention = $extensionPath . DIRECTORY_SEPARATOR . $phpExtensionName . $extensionEnding; // The extension may not be in the usual path (since you can specify a full path to an extension in the INI file) - if (! file_exists($expectedConventionalBinaryPath)) { + if (! file_exists($actualBinaryPathByConvention)) { return ''; } @@ -101,13 +102,21 @@ private static function verifyChecksumInformation( $pieExpectedChecksum = array_key_exists(PieInstalledJsonMetadataKeys::BinaryChecksum->value, $installedJsonMetadata) ? $installedJsonMetadata[PieInstalledJsonMetadataKeys::BinaryChecksum->value] : null; // Some other kind of mismatch of file path, or we don't have a stored checksum available - if ($expectedConventionalBinaryPath !== $pieExpectedBinaryPath || $pieExpectedChecksum === null) { + if ( + $pieExpectedBinaryPath === null + || $pieExpectedChecksum === null + || $pieExpectedBinaryPath !== $actualBinaryPathByConvention + ) { return ''; } - $actualInstalledBinary = BinaryFile::fromFileWithSha256Checksum($expectedConventionalBinaryPath); - if ($actualInstalledBinary->checksum !== $pieExpectedChecksum) { - return ' ⚠️ was ' . substr($actualInstalledBinary->checksum, 0, 8) . '..., expected ' . substr($pieExpectedChecksum, 0, 8) . '...'; + $expectedBinaryFileFromMetadata = new BinaryFile($pieExpectedBinaryPath, $pieExpectedChecksum); + $actualBinaryFile = BinaryFile::fromFileWithSha256Checksum($actualBinaryPathByConvention); + + try { + $expectedBinaryFileFromMetadata->verifyAgainstOther($actualBinaryFile); + } catch (BinaryFileFailedVerification) { + return ' ⚠️ was ' . substr($actualBinaryFile->checksum, 0, 8) . '..., expected ' . substr($expectedBinaryFileFromMetadata->checksum, 0, 8) . '...'; } return ' ✅'; diff --git a/src/Installing/BinaryFileFailedVerification.php b/src/Installing/BinaryFileFailedVerification.php new file mode 100644 index 0000000..dbd8f22 --- /dev/null +++ b/src/Installing/BinaryFileFailedVerification.php @@ -0,0 +1,33 @@ +filePath, + $actual->filePath, + )); + } + + public static function fromChecksumMismatch(BinaryFile $expected, BinaryFile $actual): self + { + return new self(sprintf( + 'File "%s" failed checksum verification. Expected %s..., was %s...', + $expected->filePath, + substr($expected->checksum, 0, 8), + substr($actual->checksum, 0, 8), + )); + } +} diff --git a/src/Util/FileNotFound.php b/src/Util/FileNotFound.php new file mode 100644 index 0000000..445bfe5 --- /dev/null +++ b/src/Util/FileNotFound.php @@ -0,0 +1,20 @@ +expectNotToPerformAssertions(); + $expectation->verify(); + } + + public function testVerifyFailsWithFileThatDoesNotExist(): void + { + $expectation = new BinaryFile( + '/path/to/a/file/that/does/not/exist', + self::TEST_FILE_HASH, + ); + + $this->expectException(FileNotFound::class); + $expectation->verify(); + } + + public function testVerifyFailsWithWrongHash(): void + { + $expectation = new BinaryFile( + self::TEST_FILE, + 'another hash that is wrong', + ); + + $this->expectException(BinaryFileFailedVerification::class); + $this->expectExceptionMessageMatches('/File "[^"]+" failed checksum verification\. Expected [^\.]+\.\.\., was [^\.]+\.\.\./'); + $expectation->verify(); + } + + public function testVerifyFailsWithDifferentFile(): void + { + $expectation = new BinaryFile( + self::TEST_FILE, + self::TEST_FILE_HASH, + ); + + $this->expectException(BinaryFileFailedVerification::class); + $this->expectExceptionMessageMatches('/Expected file "[^"]+" but actual file was "[^"]+"/'); + $expectation->verifyAgainstOther(new BinaryFile( + __FILE__, + self::TEST_FILE_HASH, + )); + } +} From 154921f1717754a8d0e58bd21545b91a8662ca72 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 12 Feb 2025 09:51:55 +0000 Subject: [PATCH 05/25] Added component to remove INI entries for a package --- src/Container.php | 2 + src/Installing/Ini/RemoveIniEntry.php | 15 ++++ .../Ini/RemoveIniEntryWithFileGetContents.php | 89 +++++++++++++++++++ .../RemoveIniEntryWithFileGetContentsTest.php | 18 ++++ 4 files changed, 124 insertions(+) create mode 100644 src/Installing/Ini/RemoveIniEntry.php create mode 100644 src/Installing/Ini/RemoveIniEntryWithFileGetContents.php create mode 100644 test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php diff --git a/src/Container.php b/src/Container.php index 6f8ef41..8ddcae7 100644 --- a/src/Container.php +++ b/src/Container.php @@ -107,6 +107,8 @@ static function (ContainerInterface $container): Install { }, ); + $container->alias(Ini\RemoveIniEntryWithFileGetContents::class, Ini\RemoveIniEntry::class); + return $container; } } diff --git a/src/Installing/Ini/RemoveIniEntry.php b/src/Installing/Ini/RemoveIniEntry.php new file mode 100644 index 0000000..fb5c7fd --- /dev/null +++ b/src/Installing/Ini/RemoveIniEntry.php @@ -0,0 +1,15 @@ + Returns a list of INI files that were updated to remove the extension */ + public function __invoke(Package $package, TargetPlatform $targetPlatform): array; +} diff --git a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php new file mode 100644 index 0000000..df8579f --- /dev/null +++ b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php @@ -0,0 +1,89 @@ + Returns a list of INI files that were updated to remove the extension */ + public function __invoke(Package $package, TargetPlatform $targetPlatform): array + { + $allIniFiles = [$targetPlatform->phpBinaryPath->loadedIniConfigurationFile()]; + + $additionalIniDirectory = $targetPlatform->phpBinaryPath->additionalIniDirectory(); + if ($additionalIniDirectory !== null) { + $allIniFiles = array_merge( + array_map( + static function (string $path) use ($additionalIniDirectory): string { + return $additionalIniDirectory . DIRECTORY_SEPARATOR . $path; + }, + array_filter( + scandir($additionalIniDirectory), + static function (string $path) use ($additionalIniDirectory): bool { + if (in_array($path, ['.', '..'])) { + return false; + } + + return file_exists($additionalIniDirectory . DIRECTORY_SEPARATOR . $path); + }, + ), + ), + $allIniFiles, + ); + } + + $regex = sprintf( + '/^(%s\w*=\w*%s)$/m', + $package->extensionType() === ExtensionType::PhpModule ? 'extension' : 'zend_extension', + $package->extensionName()->name(), + ); + + $updatedIniFiles = []; + array_walk( + $allIniFiles, + static function (string $iniFile) use (&$updatedIniFiles, $regex): void { + $currentContent = file_get_contents($iniFile); + + if ($currentContent === false || $currentContent === '') { + return; + } + + $replacedContent = preg_replace( + $regex, + '; $1 ; removed by PIE', + $currentContent, + ); + + if ($replacedContent === null || $replacedContent === $currentContent) { + return; + } + + // @todo verify it was written; permissions may have failed etc + file_put_contents($iniFile, $replacedContent); + $updatedIniFiles[] = $iniFile; + }, + ); + + return $updatedIniFiles; + } +} diff --git a/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php new file mode 100644 index 0000000..f658f2f --- /dev/null +++ b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php @@ -0,0 +1,18 @@ + Date: Wed, 12 Feb 2025 10:52:50 +0000 Subject: [PATCH 06/25] Added component to uninstall the installed binary --- src/Container.php | 4 ++ src/Installing/PackageMetadataMissing.php | 33 ++++++++++++++ src/Installing/Uninstall.php | 13 ++++++ src/Installing/UninstallUsingUnlink.php | 45 +++++++++++++++++++ .../Installing/PackageMetadataMissingTest.php | 43 ++++++++++++++++++ .../Installing/UninstallUsingUnlinkTest.php | 23 ++++++++++ 6 files changed, 161 insertions(+) create mode 100644 src/Installing/PackageMetadataMissing.php create mode 100644 src/Installing/Uninstall.php create mode 100644 src/Installing/UninstallUsingUnlink.php create mode 100644 test/unit/Installing/PackageMetadataMissingTest.php create mode 100644 test/unit/Installing/UninstallUsingUnlinkTest.php diff --git a/src/Container.php b/src/Container.php index 8ddcae7..e13ad63 100644 --- a/src/Container.php +++ b/src/Container.php @@ -25,6 +25,8 @@ use Php\Pie\Downloading\PackageReleaseAssets; use Php\Pie\Installing\Ini; use Php\Pie\Installing\Install; +use Php\Pie\Installing\Uninstall; +use Php\Pie\Installing\UninstallUsingUnlink; use Php\Pie\Installing\UnixInstall; use Php\Pie\Installing\WindowsInstall; use Psr\Container\ContainerInterface; @@ -107,6 +109,8 @@ static function (ContainerInterface $container): Install { }, ); + $container->alias(UninstallUsingUnlink::class, Uninstall::class); + $container->alias(Ini\RemoveIniEntryWithFileGetContents::class, Ini\RemoveIniEntry::class); return $container; diff --git a/src/Installing/PackageMetadataMissing.php b/src/Installing/PackageMetadataMissing.php new file mode 100644 index 0000000..50c5ff7 --- /dev/null +++ b/src/Installing/PackageMetadataMissing.php @@ -0,0 +1,33 @@ + $actualMetadata + * @param list $wantedKeys + */ + public static function duringUninstall(Package $package, array $actualMetadata, array $wantedKeys): self + { + $missingKeys = array_diff($wantedKeys, array_keys($actualMetadata)); + + return new self(sprintf( + 'PIE metadata was missing for package %s. Missing metadata key%s: %s', + $package->name(), + count($missingKeys) === 1 ? '' : 's', + implode(', ', $missingKeys), + )); + } +} diff --git a/src/Installing/Uninstall.php b/src/Installing/Uninstall.php new file mode 100644 index 0000000..16ffac4 --- /dev/null +++ b/src/Installing/Uninstall.php @@ -0,0 +1,13 @@ +composerPackage()); + + if ( + ! array_key_exists(PieInstalledJsonMetadataKeys::InstalledBinary->value, $pieMetadata) + || ! array_key_exists(PieInstalledJsonMetadataKeys::BinaryChecksum->value, $pieMetadata) + ) { + throw PackageMetadataMissing::duringUninstall( + $package, + $pieMetadata, + [ + PieInstalledJsonMetadataKeys::InstalledBinary->value, + PieInstalledJsonMetadataKeys::BinaryChecksum->value, + ], + ); + } + + $expectedBinaryFile = new BinaryFile( + $pieMetadata[PieInstalledJsonMetadataKeys::InstalledBinary->value], + $pieMetadata[PieInstalledJsonMetadataKeys::BinaryChecksum->value], + ); + + $expectedBinaryFile->verify(); + + unlink($expectedBinaryFile->filePath); + // @todo verify the unlink worked etc, maybe permissions failed + } +} diff --git a/test/unit/Installing/PackageMetadataMissingTest.php b/test/unit/Installing/PackageMetadataMissingTest.php new file mode 100644 index 0000000..5fdc5a7 --- /dev/null +++ b/test/unit/Installing/PackageMetadataMissingTest.php @@ -0,0 +1,43 @@ +createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + ); + + $exception = PackageMetadataMissing::duringUninstall( + $package, + [ + 'a' => 'something', + 'b' => 'something else', + ], + ['b', 'c', 'd'], + ); + + self::assertSame( + 'PIE metadata was missing for package foo/bar. Missing metadata keys: c, d', + $exception->getMessage(), + ); + } +} diff --git a/test/unit/Installing/UninstallUsingUnlinkTest.php b/test/unit/Installing/UninstallUsingUnlinkTest.php new file mode 100644 index 0000000..e6a8e54 --- /dev/null +++ b/test/unit/Installing/UninstallUsingUnlinkTest.php @@ -0,0 +1,23 @@ + Date: Mon, 17 Feb 2025 20:38:55 +0000 Subject: [PATCH 07/25] ComposerIntegrationHandler no longer __invoke --- src/Command/BuildCommand.php | 2 +- src/Command/DownloadCommand.php | 2 +- src/Command/InstallCommand.php | 2 +- src/ComposerIntegration/ComposerIntegrationHandler.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Command/BuildCommand.php b/src/Command/BuildCommand.php index b118d68..fd5588f 100644 --- a/src/Command/BuildCommand.php +++ b/src/Command/BuildCommand.php @@ -85,7 +85,7 @@ public function execute(InputInterface $input, OutputInterface $output): int ); try { - ($this->composerIntegrationHandler)( + $this->composerIntegrationHandler->runInstall( $package, $composer, $targetPlatform, diff --git a/src/Command/DownloadCommand.php b/src/Command/DownloadCommand.php index 7d27add..9c15645 100644 --- a/src/Command/DownloadCommand.php +++ b/src/Command/DownloadCommand.php @@ -69,7 +69,7 @@ public function execute(InputInterface $input, OutputInterface $output): int $output->writeln(sprintf('Found package: %s which provides %s', $package->prettyNameAndVersion(), $package->extensionName()->nameWithExtPrefix())); try { - ($this->composerIntegrationHandler)( + $this->composerIntegrationHandler->runInstall( $package, $composer, $targetPlatform, diff --git a/src/Command/InstallCommand.php b/src/Command/InstallCommand.php index 068f3c3..ae40594 100644 --- a/src/Command/InstallCommand.php +++ b/src/Command/InstallCommand.php @@ -90,7 +90,7 @@ public function execute(InputInterface $input, OutputInterface $output): int ); try { - ($this->composerIntegrationHandler)( + $this->composerIntegrationHandler->runInstall( $package, $composer, $targetPlatform, diff --git a/src/ComposerIntegration/ComposerIntegrationHandler.php b/src/ComposerIntegration/ComposerIntegrationHandler.php index d9b8f7b..ce9543d 100644 --- a/src/ComposerIntegration/ComposerIntegrationHandler.php +++ b/src/ComposerIntegration/ComposerIntegrationHandler.php @@ -26,7 +26,7 @@ public function __construct( ) { } - public function __invoke( + public function runInstall( Package $package, Composer $composer, TargetPlatform $targetPlatform, From 97dcf13376fd525296dff154a6f0a938430448de Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 17 Feb 2025 21:06:44 +0000 Subject: [PATCH 08/25] Add method to remove a require from pie.json --- src/ComposerIntegration/PieJsonEditor.php | 20 +++++++++++++++++++ .../ComposerIntegration/PieJsonEditorTest.php | 6 ++++++ 2 files changed, 26 insertions(+) diff --git a/src/ComposerIntegration/PieJsonEditor.php b/src/ComposerIntegration/PieJsonEditor.php index 973bfbf..475bfc4 100644 --- a/src/ComposerIntegration/PieJsonEditor.php +++ b/src/ComposerIntegration/PieJsonEditor.php @@ -82,6 +82,26 @@ public function addRequire(string $package, string $version): string return $originalPieJsonContent; } + /** + * Remove a package from the `require` section of the given `pie.json`. + * Returns the original `pie.json` content, in case it needs to be + * restored later. + * + * @param non-empty-string $package + */ + public function removeRequire(string $package): string + { + $originalPieJsonContent = file_get_contents($this->pieJsonFilename); + + (new JsonConfigSource( + new JsonFile( + $this->pieJsonFilename, + ), + ))->removeLink('require', $package); + + return $originalPieJsonContent; + } + public function revert(string $originalPieJsonContent): void { file_put_contents($this->pieJsonFilename, $originalPieJsonContent); diff --git a/test/unit/ComposerIntegration/PieJsonEditorTest.php b/test/unit/ComposerIntegration/PieJsonEditorTest.php index 29edc04..164ad9e 100644 --- a/test/unit/ComposerIntegration/PieJsonEditorTest.php +++ b/test/unit/ComposerIntegration/PieJsonEditorTest.php @@ -53,6 +53,12 @@ public function testCanAddRequire(): void EOF), $this->normaliseJson(file_get_contents($testPieJson)), ); + + $editor->removeRequire('foo/bar'); + self::assertSame( + $this->normaliseJson('{}'), + $this->normaliseJson(file_get_contents($testPieJson)), + ); } public function testCanRevert(): void From 0b08684a4a4ea9ee2dd92241d868886abb186438 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 17 Feb 2025 21:07:59 +0000 Subject: [PATCH 09/25] Added uninstall to PieOperation enum --- src/ComposerIntegration/PieOperation.php | 1 + test/unit/ComposerIntegration/PieOperationTest.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/ComposerIntegration/PieOperation.php b/src/ComposerIntegration/PieOperation.php index a3345d5..1967ab2 100644 --- a/src/ComposerIntegration/PieOperation.php +++ b/src/ComposerIntegration/PieOperation.php @@ -11,6 +11,7 @@ enum PieOperation case Download; case Build; case Install; + case Uninstall; public function shouldBuild(): bool { diff --git a/test/unit/ComposerIntegration/PieOperationTest.php b/test/unit/ComposerIntegration/PieOperationTest.php index 8387800..a6f00dd 100644 --- a/test/unit/ComposerIntegration/PieOperationTest.php +++ b/test/unit/ComposerIntegration/PieOperationTest.php @@ -17,6 +17,7 @@ public function testShouldBuild(): void self::assertFalse(PieOperation::Download->shouldBuild()); self::assertTrue(PieOperation::Build->shouldBuild()); self::assertTrue(PieOperation::Install->shouldBuild()); + self::assertFalse(PieOperation::Uninstall->shouldBuild()); } public function testShouldInstall(): void @@ -25,5 +26,6 @@ public function testShouldInstall(): void self::assertFalse(PieOperation::Download->shouldInstall()); self::assertFalse(PieOperation::Build->shouldInstall()); self::assertTrue(PieOperation::Install->shouldInstall()); + self::assertFalse(PieOperation::Uninstall->shouldBuild()); } } From 1ab49d6ca81ee4625beb76fc8747b03bef3dc030 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 17 Feb 2025 21:30:08 +0000 Subject: [PATCH 10/25] Added ComposerIntegrationHandler#runUninstall --- .../ComposerIntegrationHandler.php | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/ComposerIntegration/ComposerIntegrationHandler.php b/src/ComposerIntegration/ComposerIntegrationHandler.php index ce9543d..4c51f72 100644 --- a/src/ComposerIntegration/ComposerIntegrationHandler.php +++ b/src/ComposerIntegration/ComposerIntegrationHandler.php @@ -88,4 +88,46 @@ public function runInstall( ($this->vendorCleanup)($composer); } + + public function runUninstall( + Package $packageToRemove, + Composer $composer, + TargetPlatform $targetPlatform, + RequestedPackageAndVersion $requestedPackageAndVersionToRemove, + ): void { + // Write the new requirement to pie.json; because we later essentially just do a `composer install` using that file + $pieComposerJson = Platform::getPieJsonFilename($targetPlatform); + $pieJsonEditor = PieJsonEditor::fromTargetPlatform($targetPlatform); + $originalPieJsonContent = $pieJsonEditor->removeRequire($requestedPackageAndVersionToRemove->package); + + // Refresh the Composer instance so it re-reads the updated pie.json + $composer = PieComposerFactory::recreatePieComposer($this->container, $composer); + + $composerInstaller = PieComposerInstaller::createWithPhpBinary( + $targetPlatform->phpBinaryPath, + $packageToRemove->extensionName(), + $this->arrayCollectionIo, + $composer, + ); + $composerInstaller + ->setAllowedTypes(['php-ext', 'php-ext-zend']) + ->setInstall(true) + ->setIgnoredTypes([]) + ->setDryRun(false) + ->setDownloadOnly(false); + + if (file_exists(PieComposerFactory::getLockFile($pieComposerJson))) { + $composerInstaller->setUpdate(true); + $composerInstaller->setUpdateAllowList([$requestedPackageAndVersionToRemove->package]); + } + + $resultCode = $composerInstaller->run(); + + if ($resultCode !== Installer::ERROR_NONE) { + // Revert composer.json change + $pieJsonEditor->revert($originalPieJsonContent); + + throw ComposerRunFailed::fromExitCode($resultCode); + } + } } From 7ce8f44988ed0402557392e0fc2bb5a866021358 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 17 Feb 2025 22:09:10 +0000 Subject: [PATCH 11/25] Added Composer uninstall processor for PiePackageInstaller --- bin/pie | 2 + src/Command/UninstallCommand.php | 103 ++++++++++++++++++ .../PieComposerFactory.php | 1 + .../PiePackageInstaller.php | 42 +++++++ src/ComposerIntegration/UninstallProcess.php | 48 ++++++++ src/Container.php | 2 + 6 files changed, 198 insertions(+) create mode 100644 src/Command/UninstallCommand.php create mode 100644 src/ComposerIntegration/UninstallProcess.php diff --git a/bin/pie b/bin/pie index 2b907bb..caa0a2f 100755 --- a/bin/pie +++ b/bin/pie @@ -13,6 +13,7 @@ use Php\Pie\Command\RepositoryAddCommand; use Php\Pie\Command\RepositoryListCommand; use Php\Pie\Command\RepositoryRemoveCommand; use Php\Pie\Command\ShowCommand; +use Php\Pie\Command\UninstallCommand; use Php\Pie\Util\PieVersion; use Symfony\Component\Console\Application; use Symfony\Component\Console\CommandLoader\ContainerCommandLoader; @@ -37,6 +38,7 @@ $application->setCommandLoader(new ContainerCommandLoader( 'repository:list' => RepositoryListCommand::class, 'repository:add' => RepositoryAddCommand::class, 'repository:remove' => RepositoryRemoveCommand::class, + 'uninstall' => UninstallCommand::class, ] )); diff --git a/src/Command/UninstallCommand.php b/src/Command/UninstallCommand.php new file mode 100644 index 0000000..71f041d --- /dev/null +++ b/src/Command/UninstallCommand.php @@ -0,0 +1,103 @@ +addArgument( + self::ARG_PACKAGE_NAME, + InputArgument::REQUIRED, + 'The package name to remove, in the format {vendor/package}, for example `xdebug/xdebug`', + ); + + CommandHelper::configurePhpConfigOptions($this); + } + + public function execute(InputInterface $input, OutputInterface $output): int + { + $packageToRemove = (string) $input->getArgument(self::ARG_PACKAGE_NAME); + Assert::stringNotEmpty($packageToRemove); + $requestedPackageAndVersionToRemove = new RequestedPackageAndVersion($packageToRemove, null); + + $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); + + $piePackage = $this->findPiePackageByPackageName($packageToRemove, $targetPlatform); + + if ($piePackage === null) { + $output->writeln('No package found: ' . $packageToRemove . ''); + + return 1; + } + + $composer = PieComposerFactory::createPieComposer( + $this->container, + new PieComposerRequest( + $output, + $targetPlatform, + $requestedPackageAndVersionToRemove, + PieOperation::Uninstall, + [], // Configure options are not needed for uninstall + null, + true, + ), + ); + + $this->composerIntegrationHandler->runUninstall( + $piePackage, + $composer, + $targetPlatform, + $requestedPackageAndVersionToRemove, + ); + + return 0; + } + + private function findPiePackageByPackageName(string $packageToRemove, TargetPlatform $targetPlatform): Package|null + { + $piePackages = $this->installedPiePackages->allPiePackages($targetPlatform); + + foreach ($piePackages as $piePackage) { + if ($piePackage->name() === $packageToRemove) { + return $piePackage; + } + } + + return null; + } +} diff --git a/src/ComposerIntegration/PieComposerFactory.php b/src/ComposerIntegration/PieComposerFactory.php index d3ace61..ba9dcd2 100644 --- a/src/ComposerIntegration/PieComposerFactory.php +++ b/src/ComposerIntegration/PieComposerFactory.php @@ -39,6 +39,7 @@ protected function createDefaultInstallers(Installer\InstallationManager $im, Pa $type, $fs, $this->container->get(InstallAndBuildProcess::class), + $this->container->get(UninstallProcess::class), $this->composerRequest, ); }; diff --git a/src/ComposerIntegration/PiePackageInstaller.php b/src/ComposerIntegration/PiePackageInstaller.php index d2185cc..66cdc76 100644 --- a/src/ComposerIntegration/PiePackageInstaller.php +++ b/src/ComposerIntegration/PiePackageInstaller.php @@ -25,6 +25,7 @@ public function __construct( ExtensionType $type, Filesystem $filesystem, private readonly InstallAndBuildProcess $installAndBuildProcess, + private readonly UninstallProcess $uninstallProcess, private readonly PieComposerRequest $composerRequest, ) { parent::__construct($io, $composer, $type->value, $filesystem); @@ -71,4 +72,45 @@ public function install(InstalledRepositoryInterface $repo, PackageInterface $pa return null; }); } + + /** @inheritDoc */ + public function uninstall(InstalledRepositoryInterface $repo, PackageInterface $package) + { + $composerPackage = $package; + + // @todo check into why not being removed from `vendor/composer/installed.json` + return parent::uninstall($repo, $composerPackage) + ?->then(function () use ($composerPackage) { + $output = $this->composerRequest->pieOutput; + + if ($this->composerRequest->requestedPackage->package !== $composerPackage->getName()) { + $output->writeln( + sprintf( + 'Skipping %s uninstall request from Composer as it was not the expected PIE package %s', + $composerPackage->getName(), + $this->composerRequest->requestedPackage->package, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + + return null; + } + + if (! $composerPackage instanceof CompletePackage) { + $output->writeln(sprintf( + 'Not using PIE to install %s as it was not a Complete Package', + $composerPackage->getName(), + )); + + return null; + } + + ($this->uninstallProcess)( + $this->composerRequest, + $composerPackage, + ); + + return null; + }); + } } diff --git a/src/ComposerIntegration/UninstallProcess.php b/src/ComposerIntegration/UninstallProcess.php new file mode 100644 index 0000000..06f1d82 --- /dev/null +++ b/src/ComposerIntegration/UninstallProcess.php @@ -0,0 +1,48 @@ +pieOutput; + + $piePackage = Package::fromComposerCompletePackage($composerPackage); + + $affectedIniFiles = ($this->removeIniEntry)($piePackage, $composerRequest->targetPlatform); + + if (count($affectedIniFiles) === 1) { + $output->writeln(sprintf('INI file "%s" was updated to remove the extension.', reset($affectedIniFiles))); + } elseif (count($affectedIniFiles) === 0) { + $output->writeln('No INI files were updated to remove the extension.'); + } else { + $output->writeln('The following INI files were updated to remove the extnesion:'); + array_walk($affectedIniFiles, static fn (string $ini) => $output->writeln(' - ' . $ini)); + } + + ($this->uninstall)($piePackage); + } +} diff --git a/src/Container.php b/src/Container.php index e13ad63..351db89 100644 --- a/src/Container.php +++ b/src/Container.php @@ -17,6 +17,7 @@ use Php\Pie\Command\RepositoryListCommand; use Php\Pie\Command\RepositoryRemoveCommand; use Php\Pie\Command\ShowCommand; +use Php\Pie\Command\UninstallCommand; use Php\Pie\ComposerIntegration\MinimalHelperSet; use Php\Pie\ComposerIntegration\QuieterConsoleIO; use Php\Pie\DependencyResolver\DependencyResolver; @@ -54,6 +55,7 @@ public static function factory(): ContainerInterface $container->singleton(RepositoryListCommand::class); $container->singleton(RepositoryAddCommand::class); $container->singleton(RepositoryRemoveCommand::class); + $container->singleton(UninstallCommand::class); $container->singleton(QuieterConsoleIO::class, static function (ContainerInterface $container): QuieterConsoleIO { return new QuieterConsoleIO( From edb5448b5464290dee750fee58294e02fb1da72e Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 20 Feb 2025 20:21:58 +0000 Subject: [PATCH 12/25] Use Composer in InstalledPiePackages --- src/Command/ShowCommand.php | 15 ++++++++++- src/Command/UninstallCommand.php | 17 +++++++++--- src/Platform/InstalledPiePackages.php | 20 +++----------- .../Platform/InstalledPiePackagesTest.php | 26 ++++++++++++++++++- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/src/Command/ShowCommand.php b/src/Command/ShowCommand.php index 6986784..01e8c7f 100644 --- a/src/Command/ShowCommand.php +++ b/src/Command/ShowCommand.php @@ -5,13 +5,17 @@ namespace Php\Pie\Command; use Php\Pie\BinaryFile; +use Php\Pie\ComposerIntegration\PieComposerFactory; +use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; use Php\Pie\Installing\BinaryFileFailedVerification; use Php\Pie\Platform\InstalledPiePackages; use Php\Pie\Platform\OperatingSystem; +use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface; use function array_key_exists; @@ -31,6 +35,7 @@ final class ShowCommand extends Command { public function __construct( private readonly InstalledPiePackages $installedPiePackages, + private readonly ContainerInterface $container, ) { parent::__construct(); } @@ -46,7 +51,15 @@ public function execute(InputInterface $input, OutputInterface $output): int { $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); - $piePackages = $this->installedPiePackages->allPiePackages($targetPlatform); + $composer = PieComposerFactory::createPieComposer( + $this->container, + PieComposerRequest::noOperation( + new NullOutput(), + $targetPlatform, + ), + ); + + $piePackages = $this->installedPiePackages->allPiePackages($composer); $phpEnabledExtensions = $targetPlatform->phpBinaryPath->extensions(); $extensionPath = $targetPlatform->phpBinaryPath->extensionPath(); $extensionEnding = $targetPlatform->operatingSystem === OperatingSystem::Windows ? '.dll' : '.so'; diff --git a/src/Command/UninstallCommand.php b/src/Command/UninstallCommand.php index 71f041d..cb0c1dc 100644 --- a/src/Command/UninstallCommand.php +++ b/src/Command/UninstallCommand.php @@ -4,6 +4,7 @@ namespace Php\Pie\Command; +use Composer\Composer; use Php\Pie\ComposerIntegration\ComposerIntegrationHandler; use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; @@ -11,12 +12,12 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\DependencyResolver\RequestedPackageAndVersion; use Php\Pie\Platform\InstalledPiePackages; -use Php\Pie\Platform\TargetPlatform; use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface; use Webmozart\Assert\Assert; @@ -57,7 +58,15 @@ public function execute(InputInterface $input, OutputInterface $output): int $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); - $piePackage = $this->findPiePackageByPackageName($packageToRemove, $targetPlatform); + $composer = PieComposerFactory::createPieComposer( + $this->container, + PieComposerRequest::noOperation( + new NullOutput(), + $targetPlatform, + ), + ); + + $piePackage = $this->findPiePackageByPackageName($packageToRemove, $composer); if ($piePackage === null) { $output->writeln('No package found: ' . $packageToRemove . ''); @@ -88,9 +97,9 @@ public function execute(InputInterface $input, OutputInterface $output): int return 0; } - private function findPiePackageByPackageName(string $packageToRemove, TargetPlatform $targetPlatform): Package|null + private function findPiePackageByPackageName(string $packageToRemove, Composer $composer): Package|null { - $piePackages = $this->installedPiePackages->allPiePackages($targetPlatform); + $piePackages = $this->installedPiePackages->allPiePackages($composer); foreach ($piePackages as $piePackage) { if ($piePackage->name() === $packageToRemove) { diff --git a/src/Platform/InstalledPiePackages.php b/src/Platform/InstalledPiePackages.php index aa1e496..d303d9a 100644 --- a/src/Platform/InstalledPiePackages.php +++ b/src/Platform/InstalledPiePackages.php @@ -4,13 +4,10 @@ namespace Php\Pie\Platform; +use Composer\Composer; use Composer\Package\BasePackage; use Composer\Package\CompletePackageInterface; -use Php\Pie\ComposerIntegration\PieComposerFactory; -use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\DependencyResolver\Package; -use Psr\Container\ContainerInterface; -use Symfony\Component\Console\Output\NullOutput; use function array_combine; use function array_filter; @@ -23,31 +20,20 @@ */ class InstalledPiePackages { - /** @psalm-suppress PossiblyUnusedMethod no direct reference; used in service locator */ - public function __construct(private readonly ContainerInterface $container) - { - } - /** * Returns a list of PIE packages according to PIE; this does NOT check if * the extension is actually enabled in the target PHP. * * @return ListOfPiePackages */ - public function allPiePackages(TargetPlatform $targetPlatform): array + public function allPiePackages(Composer $composer): array { $composerInstalledPackages = array_map( static function (CompletePackageInterface $package): Package { return Package::fromComposerCompletePackage($package); }, array_filter( - PieComposerFactory::createPieComposer( - $this->container, - PieComposerRequest::noOperation( - new NullOutput(), - $targetPlatform, - ), - ) + $composer ->getRepositoryManager() ->getLocalRepository() ->getPackages(), diff --git a/test/unit/Platform/InstalledPiePackagesTest.php b/test/unit/Platform/InstalledPiePackagesTest.php index 628ca7d..22e6357 100644 --- a/test/unit/Platform/InstalledPiePackagesTest.php +++ b/test/unit/Platform/InstalledPiePackagesTest.php @@ -4,6 +4,10 @@ namespace Php\PieUnitTest\Platform; +use Composer\Composer; +use Composer\Package\CompletePackage; +use Composer\Repository\InstalledRepositoryInterface; +use Composer\Repository\RepositoryManager; use Php\Pie\Platform\InstalledPiePackages; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -13,6 +17,26 @@ final class InstalledPiePackagesTest extends TestCase { public function testAllPiePackages(): void { - self::fail('to be implemented'); // @todo implement this test + $localRepo = $this->createMock(InstalledRepositoryInterface::class); + $localRepo->method('getPackages')->willReturn([ + new CompletePackage('foo/bar1', '1.2.3.0', '1.2.3'), + new CompletePackage('foo/bar2', '1.2.3.0', '1.2.3'), + ]); + + $repoManager = $this->createMock(RepositoryManager::class); + $repoManager->method('getLocalRepository')->willReturn($localRepo); + + $composer = $this->createMock(Composer::class); + $composer->method('getRepositoryManager')->willReturn($repoManager); + + $packages = (new InstalledPiePackages())->allPiePackages($composer); + + self::assertArrayHasKey('bar1', $packages); + self::assertArrayHasKey('bar2', $packages); + + self::assertSame('bar1', $packages['bar1']->extensionName()->name()); + self::assertSame('foo/bar1', $packages['bar1']->name()); + self::assertSame('bar2', $packages['bar2']->extensionName()->name()); + self::assertSame('foo/bar2', $packages['bar2']->name()); } } From 9f420e81bd16906ae5661a676f3b2190a8d3bc23 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 24 Feb 2025 11:20:43 +0000 Subject: [PATCH 13/25] Fix uninstall and complete tests --- features/uninstall-extensions.feature | 6 + .../RemoveUnrelatedInstallOperations.php | 3 +- src/ComposerIntegration/UninstallProcess.php | 18 ++- .../Ini/RemoveIniEntryWithFileGetContents.php | 7 +- src/Installing/Uninstall.php | 3 +- src/Installing/UninstallUsingUnlink.php | 4 +- test/behaviour/CliContext.php | 21 ++++ .../RemoveIniEntryWithFileGetContentsTest.php | 112 +++++++++++++++++- .../Installing/UninstallUsingUnlinkTest.php | 57 ++++++++- 9 files changed, 219 insertions(+), 12 deletions(-) create mode 100644 features/uninstall-extensions.feature diff --git a/features/uninstall-extensions.feature b/features/uninstall-extensions.feature new file mode 100644 index 0000000..67d8c2d --- /dev/null +++ b/features/uninstall-extensions.feature @@ -0,0 +1,6 @@ +Feature: Extensions can be uninstalled with PIE + + Example: The latest version of an extension can be downloaded + Given an extension was previously installed + When I run a command to uninstall an extension + Then the extension should not be installed anymore diff --git a/src/ComposerIntegration/Listeners/RemoveUnrelatedInstallOperations.php b/src/ComposerIntegration/Listeners/RemoveUnrelatedInstallOperations.php index eb7a8a4..905a276 100644 --- a/src/ComposerIntegration/Listeners/RemoveUnrelatedInstallOperations.php +++ b/src/ComposerIntegration/Listeners/RemoveUnrelatedInstallOperations.php @@ -8,6 +8,7 @@ use Composer\Composer; use Composer\DependencyResolver\Operation\InstallOperation; use Composer\DependencyResolver\Operation\OperationInterface; +use Composer\DependencyResolver\Operation\UninstallOperation; use Composer\DependencyResolver\Transaction; use Composer\Installer\InstallerEvent; use Composer\Installer\InstallerEvents; @@ -49,7 +50,7 @@ public function __invoke(InstallerEvent $installerEvent): void $newOperations = array_filter( $installerEvent->getTransaction()?->getOperations() ?? [], function (OperationInterface $operation) use ($pieOutput): bool { - if (! $operation instanceof InstallOperation) { + if (! $operation instanceof InstallOperation && ! $operation instanceof UninstallOperation) { $pieOutput->writeln( sprintf( 'Unexpected operation during installer: %s', diff --git a/src/ComposerIntegration/UninstallProcess.php b/src/ComposerIntegration/UninstallProcess.php index 06f1d82..781fd0b 100644 --- a/src/ComposerIntegration/UninstallProcess.php +++ b/src/ComposerIntegration/UninstallProcess.php @@ -8,6 +8,7 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\Installing\Ini\RemoveIniEntry; use Php\Pie\Installing\Uninstall; +use Symfony\Component\Console\Output\OutputInterface; use function array_walk; use function count; @@ -35,14 +36,23 @@ public function __invoke( $affectedIniFiles = ($this->removeIniEntry)($piePackage, $composerRequest->targetPlatform); if (count($affectedIniFiles) === 1) { - $output->writeln(sprintf('INI file "%s" was updated to remove the extension.', reset($affectedIniFiles))); + $output->writeln( + sprintf('INI file "%s" was updated to remove the extension.', reset($affectedIniFiles)), + OutputInterface::VERBOSITY_VERBOSE, + ); } elseif (count($affectedIniFiles) === 0) { - $output->writeln('No INI files were updated to remove the extension.'); + $output->writeln( + 'No INI files were updated to remove the extension.', + OutputInterface::VERBOSITY_VERBOSE, + ); } else { - $output->writeln('The following INI files were updated to remove the extnesion:'); + $output->writeln( + 'The following INI files were updated to remove the extnesion:', + OutputInterface::VERBOSITY_VERBOSE, + ); array_walk($affectedIniFiles, static fn (string $ini) => $output->writeln(' - ' . $ini)); } - ($this->uninstall)($piePackage); + $output->writeln(sprintf('👋 Removed extension: %s', ($this->uninstall)($piePackage)->filePath)); } } diff --git a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php index df8579f..279a4d6 100644 --- a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php +++ b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php @@ -28,7 +28,12 @@ class RemoveIniEntryWithFileGetContents implements RemoveIniEntry /** @return list Returns a list of INI files that were updated to remove the extension */ public function __invoke(Package $package, TargetPlatform $targetPlatform): array { - $allIniFiles = [$targetPlatform->phpBinaryPath->loadedIniConfigurationFile()]; + $allIniFiles = []; + + $mainIni = $targetPlatform->phpBinaryPath->loadedIniConfigurationFile(); + if ($mainIni !== null) { + $allIniFiles[] = $mainIni; + } $additionalIniDirectory = $targetPlatform->phpBinaryPath->additionalIniDirectory(); if ($additionalIniDirectory !== null) { diff --git a/src/Installing/Uninstall.php b/src/Installing/Uninstall.php index 16ffac4..06584b0 100644 --- a/src/Installing/Uninstall.php +++ b/src/Installing/Uninstall.php @@ -4,10 +4,11 @@ namespace Php\Pie\Installing; +use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ interface Uninstall { - public function __invoke(Package $package): void; + public function __invoke(Package $package): BinaryFile; } diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 2811d50..76d77a5 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -14,7 +14,7 @@ /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ class UninstallUsingUnlink implements Uninstall { - public function __invoke(Package $package): void + public function __invoke(Package $package): BinaryFile { $pieMetadata = PieInstalledJsonMetadataKeys::pieMetadataFromComposerPackage($package->composerPackage()); @@ -40,6 +40,8 @@ public function __invoke(Package $package): void $expectedBinaryFile->verify(); unlink($expectedBinaryFile->filePath); + // @todo verify the unlink worked etc, maybe permissions failed + return $expectedBinaryFile; } } diff --git a/test/behaviour/CliContext.php b/test/behaviour/CliContext.php index 31b2f9a..8f111b0 100644 --- a/test/behaviour/CliContext.php +++ b/test/behaviour/CliContext.php @@ -114,11 +114,32 @@ public function theExtensionShouldHaveBeenBuiltWithOptions(): void } #[When('I run a command to install an extension')] + #[Given('an extension was previously installed')] public function iRunACommandToInstallAnExtension(): void { $this->runPieCommand(['install', 'asgrim/example-pie-extension']); } + #[When('I run a command to uninstall an extension')] + public function iRunACommandToUninstallAnExtension(): void + { + $this->runPieCommand(['uninstall', 'asgrim/example-pie-extension']); + } + + #[Then('the extension should not be installed anymore')] + public function theExtensionShouldNotBeInstalled(): void + { + $this->assertCommandSuccessful(); + + Assert::regex($this->output, '#👋 Removed extension: [-_a-zA-Z0-9/]+/example_pie_extension.so#'); + + $isExtEnabled = (new Process([self::PHP_BINARY, '-r', 'echo extension_loaded("example_pie_extension")?"yes":"no";'])) + ->mustRun() + ->getOutput(); + + Assert::same('no', $isExtEnabled); + } + #[Then('the extension should have been installed')] public function theExtensionShouldHaveBeenInstalled(): void { diff --git a/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php index f658f2f..3aae85b 100644 --- a/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php +++ b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php @@ -4,15 +4,123 @@ namespace Php\PieUnitTest\Installing\Ini; +use Composer\Package\CompletePackageInterface; +use Composer\Util\Filesystem; +use Php\Pie\DependencyResolver\Package; +use Php\Pie\ExtensionName; +use Php\Pie\ExtensionType; use Php\Pie\Installing\Ini\RemoveIniEntryWithFileGetContents; +use Php\Pie\Platform\Architecture; +use Php\Pie\Platform\OperatingSystem; +use Php\Pie\Platform\OperatingSystemFamily; +use Php\Pie\Platform\TargetPhp\PhpBinaryPath; +use Php\Pie\Platform\TargetPlatform; +use Php\Pie\Platform\ThreadSafetyMode; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Webmozart\Assert\Assert; + +use function file_get_contents; +use function file_put_contents; +use function mkdir; +use function sys_get_temp_dir; +use function uniqid; + +use const DIRECTORY_SEPARATOR; #[CoversClass(RemoveIniEntryWithFileGetContents::class)] final class RemoveIniEntryWithFileGetContentsTest extends TestCase { - public function testRelevantIniFilesHaveExtensionRemoved(): void + private const INI_WITH_COMMENTED_EXTS = ";extension=foobar\n;zend_extension=foobar\n"; + private const INI_WITH_ACTIVE_EXTS = "extension=foobar\nzend_extension=foobar\n"; + + private string $iniFilePath; + + public function setUp(): void { - self::fail('to be implemented'); // @todo + parent::setUp(); + + $this->iniFilePath = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_remove_ini_test', true); + mkdir($this->iniFilePath); + Assert::positiveInteger(file_put_contents( + $this->iniFilePath . DIRECTORY_SEPARATOR . 'with_commented_exts.ini', + self::INI_WITH_COMMENTED_EXTS, + )); + Assert::positiveInteger(file_put_contents( + $this->iniFilePath . DIRECTORY_SEPARATOR . 'with_active_exts.ini', + self::INI_WITH_ACTIVE_EXTS, + )); + } + + public function tearDown(): void + { + parent::tearDown(); + + (new Filesystem())->remove($this->iniFilePath); + } + + /** + * @return array + * + * @psalm-suppress PossiblyUnusedMethod https://github.com/psalm/psalm-plugin-phpunit/issues/131 + */ + public function extensionTypeProvider(): array + { + return [ + 'phpModule' => [ExtensionType::PhpModule, "; extension=foobar ; removed by PIE\nzend_extension=foobar\n"], + 'zendExtension' => [ExtensionType::ZendExtension, "extension=foobar\n; zend_extension=foobar ; removed by PIE\n"], + ]; + } + + #[DataProvider('extensionTypeProvider')] + public function testRelevantIniFilesHaveExtensionRemoved(ExtensionType $extensionType, string $expectedActiveContent): void + { + $phpBinaryPath = $this->createMock(PhpBinaryPath::class); + $phpBinaryPath + ->method('loadedIniConfigurationFile') + ->willReturn(null); + $phpBinaryPath + ->method('additionalIniDirectory') + ->willReturn($this->iniFilePath); + + $package = new Package( + $this->createMock(CompletePackageInterface::class), + $extensionType, + ExtensionName::normaliseFromString('foobar'), + 'foobar/foobar', + '1.2.3', + null, + ); + + $targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $phpBinaryPath, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $affectedFiles = (new RemoveIniEntryWithFileGetContents())( + $package, + $targetPlatform, + ); + + self::assertSame( + [$this->iniFilePath . DIRECTORY_SEPARATOR . 'with_active_exts.ini'], + $affectedFiles, + ); + + self::assertSame( + self::INI_WITH_COMMENTED_EXTS, + file_get_contents($this->iniFilePath . DIRECTORY_SEPARATOR . 'with_commented_exts.ini'), + ); + + self::assertSame( + $expectedActiveContent, + file_get_contents($this->iniFilePath . DIRECTORY_SEPARATOR . 'with_active_exts.ini'), + ); } } diff --git a/test/unit/Installing/UninstallUsingUnlinkTest.php b/test/unit/Installing/UninstallUsingUnlinkTest.php index e6a8e54..8b928d1 100644 --- a/test/unit/Installing/UninstallUsingUnlinkTest.php +++ b/test/unit/Installing/UninstallUsingUnlinkTest.php @@ -4,20 +4,73 @@ namespace Php\PieUnitTest\Installing; +use Composer\Package\CompletePackageInterface; +use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; +use Php\Pie\DependencyResolver\Package; +use Php\Pie\ExtensionName; +use Php\Pie\ExtensionType; +use Php\Pie\Installing\PackageMetadataMissing; use Php\Pie\Installing\UninstallUsingUnlink; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; +use function file_put_contents; +use function hash_file; +use function sys_get_temp_dir; +use function uniqid; + +use const DIRECTORY_SEPARATOR; + #[CoversClass(UninstallUsingUnlink::class)] final class UninstallUsingUnlinkTest extends TestCase { public function testMissingMetadataThrowsException(): void { - self::fail('to be implemented'); // @todo + $composerPackage = $this->createMock(CompletePackageInterface::class); + $composerPackage + ->method('getExtra') + ->willReturn([]); + + $package = new Package( + $composerPackage, + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foobar/foobar', + '1.2.3', + null, + ); + + $this->expectException(PackageMetadataMissing::class); + $this->expectExceptionMessage('PIE metadata was missing for package foobar/foobar. Missing metadata keys: pie-installed-binary, pie-installed-binary-checksum'); + (new UninstallUsingUnlink())($package); } public function testBinaryFileIsRemoved(): void { - self::fail('to be implemented'); // @todo + $testFilename = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_uninstall_binary_test_', true); + file_put_contents($testFilename, 'test content'); + $testHash = hash_file('sha256', $testFilename); + + $composerPackage = $this->createMock(CompletePackageInterface::class); + $composerPackage + ->method('getExtra') + ->willReturn([ + PieInstalledJsonMetadataKeys::InstalledBinary->value => $testFilename, + PieInstalledJsonMetadataKeys::BinaryChecksum->value => $testHash, + ]); + + $package = new Package( + $composerPackage, + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foobar/foobar', + '1.2.3', + null, + ); + + $uninstalled = (new UninstallUsingUnlink())($package); + + self::assertSame($testFilename, $uninstalled->filePath); + self::assertFileDoesNotExist($testFilename); } } From 78007337775438a11cd327ab3a5f74a80dd5e2f1 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 25 Feb 2025 09:11:37 +0000 Subject: [PATCH 14/25] Disable uninstall command for Windows See https://github.com/php/pie/issues/190 for details. --- features/uninstall-extensions.feature | 4 +++- src/Command/UninstallCommand.php | 11 +++++++++++ src/Installing/FailedToRemoveExtension.php | 21 +++++++++++++++++++++ src/Installing/UninstallUsingUnlink.php | 5 +++-- test/behaviour/CliContext.php | 10 +++++++--- 5 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 src/Installing/FailedToRemoveExtension.php diff --git a/features/uninstall-extensions.feature b/features/uninstall-extensions.feature index 67d8c2d..154c10c 100644 --- a/features/uninstall-extensions.feature +++ b/features/uninstall-extensions.feature @@ -1,6 +1,8 @@ Feature: Extensions can be uninstalled with PIE - Example: The latest version of an extension can be downloaded + # See https://github.com/php/pie/issues/190 for why this is non-Windows + @non-windows + Example: An extension can be uninstalled Given an extension was previously installed When I run a command to uninstall an extension Then the extension should not be installed anymore diff --git a/src/Command/UninstallCommand.php b/src/Command/UninstallCommand.php index cb0c1dc..82d00f9 100644 --- a/src/Command/UninstallCommand.php +++ b/src/Command/UninstallCommand.php @@ -5,6 +5,7 @@ namespace Php\Pie\Command; use Composer\Composer; +use Composer\Util\Platform; use Php\Pie\ComposerIntegration\ComposerIntegrationHandler; use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; @@ -52,6 +53,16 @@ public function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { + if (Platform::isWindows()) { + /** + * @todo add support for uninstalling in Windows - see + * {@link https://github.com/php/pie/issues/190} for details + */ + $output->writeln('Uninstalling extensions on Windows is not currently supported.'); + + return 1; + } + $packageToRemove = (string) $input->getArgument(self::ARG_PACKAGE_NAME); Assert::stringNotEmpty($packageToRemove); $requestedPackageAndVersionToRemove = new RequestedPackageAndVersion($packageToRemove, null); diff --git a/src/Installing/FailedToRemoveExtension.php b/src/Installing/FailedToRemoveExtension.php new file mode 100644 index 0000000..8b5abae --- /dev/null +++ b/src/Installing/FailedToRemoveExtension.php @@ -0,0 +1,21 @@ +filePath, + )); + } +} diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 76d77a5..0d9a923 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -39,9 +39,10 @@ public function __invoke(Package $package): BinaryFile $expectedBinaryFile->verify(); - unlink($expectedBinaryFile->filePath); + if (! unlink($expectedBinaryFile->filePath)) { + throw FailedToRemoveExtension::withFilename($expectedBinaryFile); + } - // @todo verify the unlink worked etc, maybe permissions failed return $expectedBinaryFile; } } diff --git a/test/behaviour/CliContext.php b/test/behaviour/CliContext.php index 8f111b0..78391cf 100644 --- a/test/behaviour/CliContext.php +++ b/test/behaviour/CliContext.php @@ -131,13 +131,17 @@ public function theExtensionShouldNotBeInstalled(): void { $this->assertCommandSuccessful(); - Assert::regex($this->output, '#👋 Removed extension: [-_a-zA-Z0-9/]+/example_pie_extension.so#'); + if (Platform::isWindows()) { + Assert::regex($this->output, '#👋 Removed extension: [-\\\_:.a-zA-Z0-9]+\\\php_example_pie_extension.dll#'); + } else { + Assert::regex($this->output, '#👋 Removed extension: [-_a-zA-Z0-9/]+/example_pie_extension.so#'); + } $isExtEnabled = (new Process([self::PHP_BINARY, '-r', 'echo extension_loaded("example_pie_extension")?"yes":"no";'])) ->mustRun() ->getOutput(); - Assert::same('no', $isExtEnabled); + Assert::same($isExtEnabled, 'no'); } #[Then('the extension should have been installed')] @@ -159,7 +163,7 @@ public function theExtensionShouldHaveBeenInstalled(): void ->mustRun() ->getOutput(); - Assert::same('yes', $isExtEnabled); + Assert::same($isExtEnabled, 'yes'); } #[Given('I have an invalid extension installed')] From cc3d53315609932f1803ef7559fae5c5e9e1f4ec Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 25 Feb 2025 10:45:17 +0000 Subject: [PATCH 15/25] Use sudo to remove the binary if no permissions given --- src/Command/UninstallCommand.php | 5 +++++ .../Ini/RemoveIniEntryWithFileGetContents.php | 6 ++++-- src/Installing/UninstallUsingUnlink.php | 13 ++++++++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/Command/UninstallCommand.php b/src/Command/UninstallCommand.php index 82d00f9..03f6622 100644 --- a/src/Command/UninstallCommand.php +++ b/src/Command/UninstallCommand.php @@ -13,6 +13,7 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\DependencyResolver\RequestedPackageAndVersion; use Php\Pie\Platform\InstalledPiePackages; +use Php\Pie\Platform\TargetPlatform; use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -63,6 +64,10 @@ public function execute(InputInterface $input, OutputInterface $output): int return 1; } + if (! TargetPlatform::isRunningAsRoot()) { + $output->writeln('This command may need elevated privileges, and may prompt you for your password.'); + } + $packageToRemove = (string) $input->getArgument(self::ARG_PACKAGE_NAME); Assert::stringNotEmpty($packageToRemove); $requestedPackageAndVersionToRemove = new RequestedPackageAndVersion($packageToRemove, null); diff --git a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php index 279a4d6..2ec292a 100644 --- a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php +++ b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php @@ -83,8 +83,10 @@ static function (string $iniFile) use (&$updatedIniFiles, $regex): void { return; } - // @todo verify it was written; permissions may have failed etc - file_put_contents($iniFile, $replacedContent); + if (! file_put_contents($iniFile, $replacedContent)) { + return; + } + $updatedIniFiles[] = $iniFile; }, ); diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 0d9a923..87d98e2 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -8,6 +8,7 @@ use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; use Php\Pie\DependencyResolver\Package; +use Php\Pie\Util\Process; use function array_key_exists; use function unlink; @@ -39,7 +40,17 @@ public function __invoke(Package $package): BinaryFile $expectedBinaryFile->verify(); - if (! unlink($expectedBinaryFile->filePath)) { + // If the target directory isn't writable, or a .so file already exists and isn't writable, try to use sudo + if (file_exists($expectedBinaryFile->filePath) && ! is_writable($expectedBinaryFile->filePath)) { + Process::run(['sudo', 'rm', $expectedBinaryFile->filePath]); + + // Removal worked, bail out + if (! file_exists($expectedBinaryFile->filePath)) { + return $expectedBinaryFile; + } + } + + if (file_exists($expectedBinaryFile->filePath) && ! unlink($expectedBinaryFile->filePath)) { throw FailedToRemoveExtension::withFilename($expectedBinaryFile); } From 393ead4e16eb58421d3b39fb5e72b50596f7be39 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 26 Feb 2025 07:35:55 +0000 Subject: [PATCH 16/25] Move BinaryFile into new File namespace --- src/Building/Build.php | 2 +- src/Building/UnixBuild.php | 2 +- src/Building/WindowsBuild.php | 2 +- src/Command/ShowCommand.php | 4 ++-- src/ComposerIntegration/InstalledJsonMetadata.php | 2 +- src/{ => File}/BinaryFile.php | 4 ++-- src/{Installing => File}/BinaryFileFailedVerification.php | 3 +-- src/Installing/FailedToRemoveExtension.php | 2 +- src/Installing/Ini/DockerPhpExtEnable.php | 2 +- src/Installing/Ini/OndrejPhpenmod.php | 2 +- src/Installing/Ini/PickBestSetupIniApproach.php | 2 +- src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php | 2 +- src/Installing/Ini/SetupIniApproach.php | 2 +- src/Installing/Ini/StandardAdditionalPhpIniDirectory.php | 2 +- src/Installing/Ini/StandardSinglePhpIni.php | 2 +- src/Installing/Install.php | 2 +- src/Installing/SetupIniFile.php | 2 +- src/Installing/Uninstall.php | 2 +- src/Installing/UninstallUsingUnlink.php | 6 ++++-- src/Installing/UnixInstall.php | 2 +- src/Installing/WindowsInstall.php | 2 +- .../ComposerIntegration/InstallAndBuildProcessTest.php | 2 +- .../ComposerIntegration/InstalledJsonMetadataTest.php | 2 +- test/unit/{ => File}/BinaryFileTest.php | 8 ++++---- test/unit/Installing/Ini/DockerPhpExtEnableTest.php | 2 +- test/unit/Installing/Ini/OndrejPhpenmodTest.php | 2 +- test/unit/Installing/Ini/PickBestSetupIniApproachTest.php | 2 +- .../Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php | 2 +- .../Ini/StandardAdditionalPhpIniDirectoryTest.php | 2 +- test/unit/Installing/Ini/StandardSinglePhpIniTest.php | 2 +- 30 files changed, 38 insertions(+), 37 deletions(-) rename src/{ => File}/BinaryFile.php (95%) rename src/{Installing => File}/BinaryFileFailedVerification.php (93%) rename test/unit/{ => File}/BinaryFileTest.php (90%) diff --git a/src/Building/Build.php b/src/Building/Build.php index 884b170..4ec0066 100644 --- a/src/Building/Build.php +++ b/src/Building/Build.php @@ -4,8 +4,8 @@ namespace Php\Pie\Building; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPhp\PhpizePath; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Building/UnixBuild.php b/src/Building/UnixBuild.php index 3130407..ec91385 100644 --- a/src/Building/UnixBuild.php +++ b/src/Building/UnixBuild.php @@ -4,8 +4,8 @@ namespace Php\Pie\Building; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPhp\PhpizePath; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; diff --git a/src/Building/WindowsBuild.php b/src/Building/WindowsBuild.php index 991ac77..66cbff4 100644 --- a/src/Building/WindowsBuild.php +++ b/src/Building/WindowsBuild.php @@ -4,8 +4,8 @@ namespace Php\Pie\Building; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPhp\PhpizePath; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Platform\WindowsExtensionAssetName; diff --git a/src/Command/ShowCommand.php b/src/Command/ShowCommand.php index 01e8c7f..6bf3a9a 100644 --- a/src/Command/ShowCommand.php +++ b/src/Command/ShowCommand.php @@ -4,11 +4,11 @@ namespace Php\Pie\Command; -use Php\Pie\BinaryFile; use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; -use Php\Pie\Installing\BinaryFileFailedVerification; +use Php\Pie\File\BinaryFile; +use Php\Pie\File\BinaryFileFailedVerification; use Php\Pie\Platform\InstalledPiePackages; use Php\Pie\Platform\OperatingSystem; use Psr\Container\ContainerInterface; diff --git a/src/ComposerIntegration/InstalledJsonMetadata.php b/src/ComposerIntegration/InstalledJsonMetadata.php index 61bc857..e91aaaa 100644 --- a/src/ComposerIntegration/InstalledJsonMetadata.php +++ b/src/ComposerIntegration/InstalledJsonMetadata.php @@ -7,8 +7,8 @@ use Composer\Package\CompletePackage; use Composer\Package\CompletePackageInterface; use Composer\PartialComposer; -use Php\Pie\BinaryFile; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys as MetadataKey; +use Php\Pie\File\BinaryFile; use Webmozart\Assert\Assert; use function array_merge; diff --git a/src/BinaryFile.php b/src/File/BinaryFile.php similarity index 95% rename from src/BinaryFile.php rename to src/File/BinaryFile.php index 50cac2e..db3ae94 100644 --- a/src/BinaryFile.php +++ b/src/File/BinaryFile.php @@ -2,9 +2,9 @@ declare(strict_types=1); -namespace Php\Pie; +namespace Php\Pie\File; -use Php\Pie\Installing\BinaryFileFailedVerification; +use Php\Pie\Util; use function file_exists; use function hash_file; diff --git a/src/Installing/BinaryFileFailedVerification.php b/src/File/BinaryFileFailedVerification.php similarity index 93% rename from src/Installing/BinaryFileFailedVerification.php rename to src/File/BinaryFileFailedVerification.php index dbd8f22..7dd141b 100644 --- a/src/Installing/BinaryFileFailedVerification.php +++ b/src/File/BinaryFileFailedVerification.php @@ -2,9 +2,8 @@ declare(strict_types=1); -namespace Php\Pie\Installing; +namespace Php\Pie\File; -use Php\Pie\BinaryFile; use RuntimeException; use function sprintf; diff --git a/src/Installing/FailedToRemoveExtension.php b/src/Installing/FailedToRemoveExtension.php index 8b5abae..f0f4afe 100644 --- a/src/Installing/FailedToRemoveExtension.php +++ b/src/Installing/FailedToRemoveExtension.php @@ -4,7 +4,7 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; +use Php\Pie\File\BinaryFile; use RuntimeException; use function sprintf; diff --git a/src/Installing/Ini/DockerPhpExtEnable.php b/src/Installing/Ini/DockerPhpExtEnable.php index 195b80b..588a5e0 100644 --- a/src/Installing/Ini/DockerPhpExtEnable.php +++ b/src/Installing/Ini/DockerPhpExtEnable.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; diff --git a/src/Installing/Ini/OndrejPhpenmod.php b/src/Installing/Ini/OndrejPhpenmod.php index 7c52d7d..92bb5eb 100644 --- a/src/Installing/Ini/OndrejPhpenmod.php +++ b/src/Installing/Ini/OndrejPhpenmod.php @@ -5,8 +5,8 @@ namespace Php\Pie\Installing\Ini; use Composer\Util\Platform; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Ini/PickBestSetupIniApproach.php b/src/Installing/Ini/PickBestSetupIniApproach.php index 83c719d..a122491 100644 --- a/src/Installing/Ini/PickBestSetupIniApproach.php +++ b/src/Installing/Ini/PickBestSetupIniApproach.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use ReflectionClass; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php b/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php index 9ae8c55..23c8b0c 100644 --- a/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php +++ b/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Ini/SetupIniApproach.php b/src/Installing/Ini/SetupIniApproach.php index 37870e9..0e003e0 100644 --- a/src/Installing/Ini/SetupIniApproach.php +++ b/src/Installing/Ini/SetupIniApproach.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php index af94dab..3202450 100644 --- a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php +++ b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Ini/StandardSinglePhpIni.php b/src/Installing/Ini/StandardSinglePhpIni.php index 0eb046e..decaf6d 100644 --- a/src/Installing/Ini/StandardSinglePhpIni.php +++ b/src/Installing/Ini/StandardSinglePhpIni.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing\Ini; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Install.php b/src/Installing/Install.php index 17982b4..0e1b3fe 100644 --- a/src/Installing/Install.php +++ b/src/Installing/Install.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/SetupIniFile.php b/src/Installing/SetupIniFile.php index 35b11f5..e169246 100644 --- a/src/Installing/SetupIniFile.php +++ b/src/Installing/SetupIniFile.php @@ -4,9 +4,9 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\SetupIniApproach; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; diff --git a/src/Installing/Uninstall.php b/src/Installing/Uninstall.php index 06584b0..1c72af8 100644 --- a/src/Installing/Uninstall.php +++ b/src/Installing/Uninstall.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; +use Php\Pie\File\BinaryFile; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ interface Uninstall diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 87d98e2..4e3699a 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -4,12 +4,14 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; use Php\Pie\DependencyResolver\Package; - +use Php\Pie\File\BinaryFile; use Php\Pie\Util\Process; + use function array_key_exists; +use function file_exists; +use function is_writable; use function unlink; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ diff --git a/src/Installing/UnixInstall.php b/src/Installing/UnixInstall.php index efe9bea..fa179ff 100644 --- a/src/Installing/UnixInstall.php +++ b/src/Installing/UnixInstall.php @@ -4,8 +4,8 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; use RuntimeException; diff --git a/src/Installing/WindowsInstall.php b/src/Installing/WindowsInstall.php index 29d4ef2..23c8f2a 100644 --- a/src/Installing/WindowsInstall.php +++ b/src/Installing/WindowsInstall.php @@ -4,9 +4,9 @@ namespace Php\Pie\Installing; -use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Platform\WindowsExtensionAssetName; use RecursiveDirectoryIterator; diff --git a/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php b/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php index a0137f5..5d3f5a9 100644 --- a/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php +++ b/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php @@ -6,13 +6,13 @@ use Composer\Package\CompletePackage; use Composer\PartialComposer; -use Php\Pie\BinaryFile; use Php\Pie\Building\Build; use Php\Pie\ComposerIntegration\InstallAndBuildProcess; use Php\Pie\ComposerIntegration\InstalledJsonMetadata; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieOperation; use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Install; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; diff --git a/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php b/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php index 48790e7..a4df418 100644 --- a/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php +++ b/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php @@ -8,11 +8,11 @@ use Composer\Package\CompletePackage; use Composer\Repository\InstalledArrayRepository; use Composer\Repository\RepositoryManager; -use Php\Pie\BinaryFile; use Php\Pie\ComposerIntegration\InstalledJsonMetadata; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieOperation; use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\File\BinaryFile; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; use Php\Pie\Platform\OperatingSystemFamily; diff --git a/test/unit/BinaryFileTest.php b/test/unit/File/BinaryFileTest.php similarity index 90% rename from test/unit/BinaryFileTest.php rename to test/unit/File/BinaryFileTest.php index 27400b2..0b7fc21 100644 --- a/test/unit/BinaryFileTest.php +++ b/test/unit/File/BinaryFileTest.php @@ -2,10 +2,10 @@ declare(strict_types=1); -namespace Php\PieUnitTest; +namespace Php\PieUnitTest\File; -use Php\Pie\BinaryFile; -use Php\Pie\Installing\BinaryFileFailedVerification; +use Php\Pie\File\BinaryFile; +use Php\Pie\File\BinaryFileFailedVerification; use Php\Pie\Util\FileNotFound; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -13,7 +13,7 @@ #[CoversClass(BinaryFile::class)] final class BinaryFileTest extends TestCase { - private const TEST_FILE = __DIR__ . '/../assets/test-zip.zip'; + private const TEST_FILE = __DIR__ . '/../../assets/test-zip.zip'; private const TEST_FILE_HASH = '64e40b4a66831437a3cc6b899ea71a36765ccb435f8831ab20d49f8ce3f806fa'; public function testVerifySucceedsWithGoodHash(): void diff --git a/test/unit/Installing/Ini/DockerPhpExtEnableTest.php b/test/unit/Installing/Ini/DockerPhpExtEnableTest.php index ab407af..e118096 100644 --- a/test/unit/Installing/Ini/DockerPhpExtEnableTest.php +++ b/test/unit/Installing/Ini/DockerPhpExtEnableTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackageInterface; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\DockerPhpExtEnable; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; diff --git a/test/unit/Installing/Ini/OndrejPhpenmodTest.php b/test/unit/Installing/Ini/OndrejPhpenmodTest.php index 527fe42..efa7686 100644 --- a/test/unit/Installing/Ini/OndrejPhpenmodTest.php +++ b/test/unit/Installing/Ini/OndrejPhpenmodTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackageInterface; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\CheckAndAddExtensionToIniIfNeeded; use Php\Pie\Installing\Ini\OndrejPhpenmod; use Php\Pie\Platform\Architecture; diff --git a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php index 1c35c02..0786ee7 100644 --- a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php +++ b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackage; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\PickBestSetupIniApproach; use Php\Pie\Installing\Ini\SetupIniApproach; use Php\Pie\Platform\Architecture; diff --git a/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php index 6435e6b..69b71db 100644 --- a/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php +++ b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackageInterface; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\PreCheckExtensionAlreadyLoaded; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; diff --git a/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php index 45a073d..8380d21 100644 --- a/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php +++ b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackageInterface; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\CheckAndAddExtensionToIniIfNeeded; use Php\Pie\Installing\Ini\StandardAdditionalPhpIniDirectory; use Php\Pie\Platform\Architecture; diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php index 21f4771..3d35fd0 100644 --- a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -5,11 +5,11 @@ namespace Php\PieUnitTest\Installing\Ini; use Composer\Package\CompletePackageInterface; -use Php\Pie\BinaryFile; use Php\Pie\DependencyResolver\Package; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\File\BinaryFile; use Php\Pie\Installing\Ini\CheckAndAddExtensionToIniIfNeeded; use Php\Pie\Installing\Ini\StandardSinglePhpIni; use Php\Pie\Platform\Architecture; From 80edcb17ef3fd08fafeb19944ed5108f693955de Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 26 Feb 2025 10:34:15 +0000 Subject: [PATCH 17/25] Util to capture and return errors running a function --- src/Util/CaptureErrors.php | 44 ++++++++++++++++++++++++++++ test/unit/Util/CaptureErrorsTest.php | 38 ++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 src/Util/CaptureErrors.php create mode 100644 test/unit/Util/CaptureErrorsTest.php diff --git a/src/Util/CaptureErrors.php b/src/Util/CaptureErrors.php new file mode 100644 index 0000000..4d706a4 --- /dev/null +++ b/src/Util/CaptureErrors.php @@ -0,0 +1,44 @@ + + */ +final class CaptureErrors +{ + /** + * @param callable():T $code + * @param CapturedErrorList $captured + * + * @return T + * + * @template T + */ + public static function for(callable $code, array &$captured): mixed + { + set_error_handler(static function (int $level, string $message, string $filename, int $line) use (&$captured): bool { + $captured[] = [ + 'level' => $level, + 'message' => $message, + 'filename' => $filename, + 'line' => $line, + ]; + + return true; + }); + + $returnValue = $code(); + + restore_error_handler(); + + return $returnValue; + } +} diff --git a/test/unit/Util/CaptureErrorsTest.php b/test/unit/Util/CaptureErrorsTest.php new file mode 100644 index 0000000..a1cddea --- /dev/null +++ b/test/unit/Util/CaptureErrorsTest.php @@ -0,0 +1,38 @@ + Date: Wed, 26 Feb 2025 10:43:09 +0000 Subject: [PATCH 18/25] Added Sudo locator --- src/File/Sudo.php | 48 +++++++++++++++++++++++++++++++ src/File/SudoNotFoundOnSystem.php | 15 ++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/File/Sudo.php create mode 100644 src/File/SudoNotFoundOnSystem.php diff --git a/src/File/Sudo.php b/src/File/Sudo.php new file mode 100644 index 0000000..b3041b4 --- /dev/null +++ b/src/File/Sudo.php @@ -0,0 +1,48 @@ +find('sudo'); + + if ($sudo === null || $sudo === '') { + throw SudoNotFoundOnSystem::new(); + } + + self::$memoizedSudo = $sudo; + } + + return self::$memoizedSudo; + } + + public static function exists(): bool + { + try { + self::find(); + + return true; + } catch (Throwable) { + return false; + } + } +} diff --git a/src/File/SudoNotFoundOnSystem.php b/src/File/SudoNotFoundOnSystem.php new file mode 100644 index 0000000..e3818aa --- /dev/null +++ b/src/File/SudoNotFoundOnSystem.php @@ -0,0 +1,15 @@ + Date: Wed, 26 Feb 2025 10:45:49 +0000 Subject: [PATCH 19/25] Added file util to write to a file with sudo --- src/File/FailedToWriteFile.php | 26 +++++++++++++ src/File/SudoFilePut.php | 61 ++++++++++++++++++++++++++++++ test/unit/File/SudoFilePutTest.php | 33 ++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 src/File/FailedToWriteFile.php create mode 100644 src/File/SudoFilePut.php create mode 100644 test/unit/File/SudoFilePutTest.php diff --git a/src/File/FailedToWriteFile.php b/src/File/FailedToWriteFile.php new file mode 100644 index 0000000..b856ea4 --- /dev/null +++ b/src/File/FailedToWriteFile.php @@ -0,0 +1,26 @@ + file_put_contents($filename, $content), + $capturedErrors, + ); + + if ($writeSuccessful === false) { + throw FailedToWriteFile::fromFilePutContentErrors($filename, $capturedErrors); + } + + if (! $didChangePermissions || ! Sudo::exists()) { + return; + } + + Process::run([Sudo::find(), 'chmod', $previousPermissions, $filename]); + } + + private static function attemptToMakeFileEditable(string $filename): bool + { + if (! Sudo::exists()) { + return false; + } + + if (! is_writable($filename)) { + try { + Process::run([Sudo::find(), 'chmod', '0777', $filename]); + + return true; + } catch (ProcessFailedException) { + return false; + } + } + + return false; + } +} diff --git a/test/unit/File/SudoFilePutTest.php b/test/unit/File/SudoFilePutTest.php new file mode 100644 index 0000000..7d909df --- /dev/null +++ b/test/unit/File/SudoFilePutTest.php @@ -0,0 +1,33 @@ + Date: Wed, 26 Feb 2025 11:01:03 +0000 Subject: [PATCH 20/25] Use sudo to write INI file to remove the INI entry --- src/Command/UninstallCommand.php | 2 +- src/ComposerIntegration/UninstallProcess.php | 2 +- src/Installing/Ini/RemoveIniEntry.php | 3 ++- .../Ini/RemoveIniEntryWithFileGetContents.php | 20 ++++++++++++++----- .../RemoveIniEntryWithFileGetContentsTest.php | 4 +++- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/Command/UninstallCommand.php b/src/Command/UninstallCommand.php index 03f6622..0e6997b 100644 --- a/src/Command/UninstallCommand.php +++ b/src/Command/UninstallCommand.php @@ -59,7 +59,7 @@ public function execute(InputInterface $input, OutputInterface $output): int * @todo add support for uninstalling in Windows - see * {@link https://github.com/php/pie/issues/190} for details */ - $output->writeln('Uninstalling extensions on Windows is not currently supported.'); + $output->writeln('Uninstalling extensions on Windows is not currently supported.'); return 1; } diff --git a/src/ComposerIntegration/UninstallProcess.php b/src/ComposerIntegration/UninstallProcess.php index 781fd0b..6f4d747 100644 --- a/src/ComposerIntegration/UninstallProcess.php +++ b/src/ComposerIntegration/UninstallProcess.php @@ -33,7 +33,7 @@ public function __invoke( $piePackage = Package::fromComposerCompletePackage($composerPackage); - $affectedIniFiles = ($this->removeIniEntry)($piePackage, $composerRequest->targetPlatform); + $affectedIniFiles = ($this->removeIniEntry)($piePackage, $composerRequest->targetPlatform, $output); if (count($affectedIniFiles) === 1) { $output->writeln( diff --git a/src/Installing/Ini/RemoveIniEntry.php b/src/Installing/Ini/RemoveIniEntry.php index fb5c7fd..347c916 100644 --- a/src/Installing/Ini/RemoveIniEntry.php +++ b/src/Installing/Ini/RemoveIniEntry.php @@ -6,10 +6,11 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\Platform\TargetPlatform; +use Symfony\Component\Console\Output\OutputInterface; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ interface RemoveIniEntry { /** @return list Returns a list of INI files that were updated to remove the extension */ - public function __invoke(Package $package, TargetPlatform $targetPlatform): array; + public function __invoke(Package $package, TargetPlatform $targetPlatform, OutputInterface $output): array; } diff --git a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php index 2ec292a..6b4235c 100644 --- a/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php +++ b/src/Installing/Ini/RemoveIniEntryWithFileGetContents.php @@ -6,7 +6,10 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\ExtensionType; +use Php\Pie\File\FailedToWriteFile; +use Php\Pie\File\SudoFilePut; use Php\Pie\Platform\TargetPlatform; +use Symfony\Component\Console\Output\OutputInterface; use function array_filter; use function array_map; @@ -14,7 +17,6 @@ use function array_walk; use function file_exists; use function file_get_contents; -use function file_put_contents; use function in_array; use function preg_replace; use function scandir; @@ -26,7 +28,7 @@ class RemoveIniEntryWithFileGetContents implements RemoveIniEntry { /** @return list Returns a list of INI files that were updated to remove the extension */ - public function __invoke(Package $package, TargetPlatform $targetPlatform): array + public function __invoke(Package $package, TargetPlatform $targetPlatform, OutputInterface $output): array { $allIniFiles = []; @@ -58,7 +60,7 @@ static function (string $path) use ($additionalIniDirectory): bool { } $regex = sprintf( - '/^(%s\w*=\w*%s)$/m', + '/^(%s\w*=\w*%s)/m', $package->extensionType() === ExtensionType::PhpModule ? 'extension' : 'zend_extension', $package->extensionName()->name(), ); @@ -66,7 +68,7 @@ static function (string $path) use ($additionalIniDirectory): bool { $updatedIniFiles = []; array_walk( $allIniFiles, - static function (string $iniFile) use (&$updatedIniFiles, $regex): void { + static function (string $iniFile) use (&$updatedIniFiles, $regex, $package, $output): void { $currentContent = file_get_contents($iniFile); if ($currentContent === false || $currentContent === '') { @@ -83,7 +85,15 @@ static function (string $iniFile) use (&$updatedIniFiles, $regex): void { return; } - if (! file_put_contents($iniFile, $replacedContent)) { + try { + SudoFilePut::contents($iniFile, $replacedContent); + } catch (FailedToWriteFile) { + $output->writeln(sprintf( + 'Failed to remove extension "%s" from INI file "%s"', + $package->extensionName()->name(), + $iniFile, + )); + return; } diff --git a/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php index 3aae85b..d08fb49 100644 --- a/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php +++ b/test/unit/Installing/Ini/RemoveIniEntryWithFileGetContentsTest.php @@ -19,6 +19,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Symfony\Component\Console\Output\OutputInterface; use Webmozart\Assert\Assert; use function file_get_contents; @@ -65,7 +66,7 @@ public function tearDown(): void * * @psalm-suppress PossiblyUnusedMethod https://github.com/psalm/psalm-plugin-phpunit/issues/131 */ - public function extensionTypeProvider(): array + public static function extensionTypeProvider(): array { return [ 'phpModule' => [ExtensionType::PhpModule, "; extension=foobar ; removed by PIE\nzend_extension=foobar\n"], @@ -106,6 +107,7 @@ public function testRelevantIniFilesHaveExtensionRemoved(ExtensionType $extensio $affectedFiles = (new RemoveIniEntryWithFileGetContents())( $package, $targetPlatform, + $this->createMock(OutputInterface::class), ); self::assertSame( From acaeac44a1ae8cbaacad06027af2703391c84e4d Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 26 Feb 2025 11:11:37 +0000 Subject: [PATCH 21/25] Only run SudoFilePutTest if allowed --- test/unit/File/SudoFilePutTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/File/SudoFilePutTest.php b/test/unit/File/SudoFilePutTest.php index 7d909df..6470764 100644 --- a/test/unit/File/SudoFilePutTest.php +++ b/test/unit/File/SudoFilePutTest.php @@ -4,6 +4,7 @@ namespace Php\PieUnitTest\File; +use Php\Pie\File\Sudo; use Php\Pie\File\SudoFilePut; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -21,6 +22,10 @@ final class SudoFilePutTest extends TestCase { public function testSudoFilePutContents(): void { + if (! Sudo::exists()) { + self::markTestSkipped('Cannot test sudo file_put_contents without sudo'); + } + $file = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_test_sudo_file_put_contents_', true); touch($file); chmod($file, 0000); From 1d3ac873a263a8354592f014de1a8c4e6ac0a91a Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 28 Feb 2025 07:55:06 +0000 Subject: [PATCH 22/25] Display pie.json path if in verbose mode --- src/Command/CommandHelper.php | 8 ++++++++ src/ComposerIntegration/PiePackageInstaller.php | 1 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index a456314..489b247 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -13,6 +13,7 @@ use InvalidArgumentException; use Php\Pie\DependencyResolver\Package; use Php\Pie\DependencyResolver\RequestedPackageAndVersion; +use Php\Pie\Platform as PiePlatform; use Php\Pie\Platform\OperatingSystem; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Php\Pie\Platform\TargetPhp\PhpizePath; @@ -168,6 +169,13 @@ public static function determineTargetPlatformFromInputs(InputInterface $input, $targetPlatform->architecture->name, $phpBinaryPath->phpBinaryPath, )); + $output->writeln( + sprintf( + 'Using pie.json: %s', + PiePlatform::getPieJsonFilename($targetPlatform), + ), + OutputInterface::VERBOSITY_VERBOSE, + ); return $targetPlatform; } diff --git a/src/ComposerIntegration/PiePackageInstaller.php b/src/ComposerIntegration/PiePackageInstaller.php index 66cdc76..db6b025 100644 --- a/src/ComposerIntegration/PiePackageInstaller.php +++ b/src/ComposerIntegration/PiePackageInstaller.php @@ -78,7 +78,6 @@ public function uninstall(InstalledRepositoryInterface $repo, PackageInterface $ { $composerPackage = $package; - // @todo check into why not being removed from `vendor/composer/installed.json` return parent::uninstall($repo, $composerPackage) ?->then(function () use ($composerPackage) { $output = $this->composerRequest->pieOutput; From 6e5375955bb3d5432f27780e4da656375cf4f172 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 28 Feb 2025 08:00:49 +0000 Subject: [PATCH 23/25] Use sudo helper in other sudo locations --- src/Installing/UninstallUsingUnlink.php | 5 +++-- src/Installing/UnixInstall.php | 10 +++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 4e3699a..0813a81 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -7,6 +7,7 @@ use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; use Php\Pie\DependencyResolver\Package; use Php\Pie\File\BinaryFile; +use Php\Pie\File\Sudo; use Php\Pie\Util\Process; use function array_key_exists; @@ -43,8 +44,8 @@ public function __invoke(Package $package): BinaryFile $expectedBinaryFile->verify(); // If the target directory isn't writable, or a .so file already exists and isn't writable, try to use sudo - if (file_exists($expectedBinaryFile->filePath) && ! is_writable($expectedBinaryFile->filePath)) { - Process::run(['sudo', 'rm', $expectedBinaryFile->filePath]); + if (file_exists($expectedBinaryFile->filePath) && ! is_writable($expectedBinaryFile->filePath) && Sudo::exists()) { + Process::run([Sudo::find(), 'rm', $expectedBinaryFile->filePath]); // Removal worked, bail out if (! file_exists($expectedBinaryFile->filePath)) { diff --git a/src/Installing/UnixInstall.php b/src/Installing/UnixInstall.php index fa179ff..571cc4d 100644 --- a/src/Installing/UnixInstall.php +++ b/src/Installing/UnixInstall.php @@ -6,6 +6,7 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\File\BinaryFile; +use Php\Pie\File\Sudo; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; use RuntimeException; @@ -44,14 +45,17 @@ public function __invoke( // If the target directory isn't writable, or a .so file already exists and isn't writable, try to use sudo if ( - ! is_writable($targetExtensionPath) - || (file_exists($expectedSharedObjectLocation) && ! is_writable($expectedSharedObjectLocation)) + Sudo::exists() + && ( + ! is_writable($targetExtensionPath) + || (file_exists($expectedSharedObjectLocation) && ! is_writable($expectedSharedObjectLocation)) + ) ) { $output->writeln(sprintf( 'Cannot write to %s, so using sudo to elevate privileges.', $targetExtensionPath, )); - array_unshift($makeInstallCommand, 'sudo'); + array_unshift($makeInstallCommand, Sudo::find()); } $makeInstallOutput = Process::run( From 08568138595836981071ada58b18dac3cbef732c Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 28 Feb 2025 09:30:27 +0000 Subject: [PATCH 24/25] Use sudo for INI additions and file unlinking --- src/File/FailedToUnlinkFile.php | 47 ++++++++++++++++ src/File/SudoUnlink.php | 53 +++++++++++++++++++ src/Installing/FailedToRemoveExtension.php | 14 +++-- .../Ini/AddExtensionToTheIniFile.php | 9 ++-- src/Installing/Ini/OndrejPhpenmod.php | 42 ++++++++++----- .../Ini/StandardAdditionalPhpIniDirectory.php | 25 ++++++--- src/Installing/UninstallUsingUnlink.php | 9 ++-- test/unit/File/SudoUnlinkTest.php | 37 +++++++++++++ .../StandardAdditionalPhpIniDirectoryTest.php | 2 +- 9 files changed, 205 insertions(+), 33 deletions(-) create mode 100644 src/File/FailedToUnlinkFile.php create mode 100644 src/File/SudoUnlink.php create mode 100644 test/unit/File/SudoUnlinkTest.php diff --git a/src/File/FailedToUnlinkFile.php b/src/File/FailedToUnlinkFile.php new file mode 100644 index 0000000..745ac02 --- /dev/null +++ b/src/File/FailedToUnlinkFile.php @@ -0,0 +1,47 @@ +getMessage(), + ), + previous: $processFailed, + ); + } +} diff --git a/src/File/SudoUnlink.php b/src/File/SudoUnlink.php new file mode 100644 index 0000000..6e7efb9 --- /dev/null +++ b/src/File/SudoUnlink.php @@ -0,0 +1,53 @@ + unlink($filename), + $capturedErrors, + ); + + if (! $unlinkSuccessful || file_exists($filename)) { + throw FailedToUnlinkFile::fromUnlinkErrors($filename, $capturedErrors); + } + + return; + } + + if (! Sudo::exists()) { + throw FailedToUnlinkFile::fromNoPermissions($filename); + } + + try { + Process::run([Sudo::find(), 'rm', $filename]); + } catch (ProcessFailedException $processFailedException) { + throw FailedToUnlinkFile::fromSudoRmProcessFailed($filename, $processFailedException); + } + + if (! file_exists($filename)) { + return; + } + + FailedToUnlinkFile::fromNoPermissions($filename); + } +} diff --git a/src/Installing/FailedToRemoveExtension.php b/src/Installing/FailedToRemoveExtension.php index f0f4afe..f5004db 100644 --- a/src/Installing/FailedToRemoveExtension.php +++ b/src/Installing/FailedToRemoveExtension.php @@ -6,16 +6,20 @@ use Php\Pie\File\BinaryFile; use RuntimeException; +use Throwable; use function sprintf; class FailedToRemoveExtension extends RuntimeException { - public static function withFilename(BinaryFile $extension): self + public static function withFilename(BinaryFile $extension, Throwable $previous): self { - return new self(sprintf( - 'Failed to remove extension file: %s', - $extension->filePath, - )); + return new self( + sprintf( + 'Failed to remove extension file: %s', + $extension->filePath, + ), + previous: $previous, + ); } } diff --git a/src/Installing/Ini/AddExtensionToTheIniFile.php b/src/Installing/Ini/AddExtensionToTheIniFile.php index f4cece3..94ba60d 100644 --- a/src/Installing/Ini/AddExtensionToTheIniFile.php +++ b/src/Installing/Ini/AddExtensionToTheIniFile.php @@ -6,12 +6,13 @@ use Php\Pie\DependencyResolver\Package; use Php\Pie\ExtensionType; +use Php\Pie\File\Sudo; +use Php\Pie\File\SudoFilePut; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Symfony\Component\Console\Output\OutputInterface; use Throwable; use function file_get_contents; -use function file_put_contents; use function is_string; use function is_writable; use function sprintf; @@ -29,7 +30,7 @@ public function __invoke( OutputInterface $output, callable|null $additionalEnableStep, ): bool { - if (! is_writable($ini)) { + if (! is_writable($ini) && ! Sudo::exists()) { $output->writeln( sprintf( 'PHP is configured to use %s, but it is not writable by PIE.', @@ -56,7 +57,7 @@ public function __invoke( } try { - file_put_contents( + SudoFilePut::contents( $ini, $originalIniContent . $this->iniFileContent($package), ); @@ -77,7 +78,7 @@ public function __invoke( return true; } catch (Throwable $anything) { - file_put_contents($ini, $originalIniContent); + SudoFilePut::contents($ini, $originalIniContent); $output->writeln(sprintf( 'Something went wrong enabling the %s extension: %s', diff --git a/src/Installing/Ini/OndrejPhpenmod.php b/src/Installing/Ini/OndrejPhpenmod.php index 92bb5eb..7d3865d 100644 --- a/src/Installing/Ini/OndrejPhpenmod.php +++ b/src/Installing/Ini/OndrejPhpenmod.php @@ -7,11 +7,14 @@ use Composer\Util\Platform; use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\File\BinaryFile; +use Php\Pie\File\Sudo; +use Php\Pie\File\SudoUnlink; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Process\Exception\ProcessFailedException; +use function array_unshift; use function file_exists; use function is_dir; use function is_writable; @@ -19,7 +22,6 @@ use function rtrim; use function sprintf; use function touch; -use function unlink; use const DIRECTORY_SEPARATOR; @@ -103,16 +105,21 @@ public function setup( return false; } + $needSudo = false; if (! is_writable($expectedModsAvailablePath)) { - $output->writeln( - sprintf( - 'Mods available path %s is not writable', - $expectedModsAvailablePath, - ), - OutputInterface::VERBOSITY_VERBOSE, - ); - - return false; + if (! Sudo::exists()) { + $output->writeln( + sprintf( + 'Mods available path %s is not writable', + $expectedModsAvailablePath, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + $needSudo = true; } $expectedIniFile = sprintf( @@ -140,16 +147,23 @@ public function setup( $targetPlatform, $downloadedPackage, $output, - static function () use ($phpenmodPath, $targetPlatform, $downloadedPackage, $output): bool { + static function () use ($needSudo, $phpenmodPath, $targetPlatform, $downloadedPackage, $output): bool { try { - Process::run([ + $processArgs = [ $phpenmodPath, '-v', $targetPlatform->phpBinaryPath->majorMinorVersion(), '-s', 'ALL', $downloadedPackage->package->extensionName()->name(), - ]); + ]; + + if ($needSudo && Sudo::exists()) { + $output->writeln('Using sudo to elevate privileges for phpenmod', OutputInterface::VERBOSITY_VERBOSE); + array_unshift($processArgs, Sudo::find()); + } + + Process::run($processArgs); return true; } catch (ProcessFailedException $processFailedException) { @@ -170,7 +184,7 @@ static function () use ($phpenmodPath, $targetPlatform, $downloadedPackage, $out ); if (! $addingExtensionWasSuccessful && $pieCreatedTheIniFile) { - unlink($expectedIniFile); + SudoUnlink::singleFile($expectedIniFile); } return $addingExtensionWasSuccessful; diff --git a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php index 3202450..b274436 100644 --- a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php +++ b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php @@ -6,6 +6,9 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\File\BinaryFile; +use Php\Pie\File\Sudo; +use Php\Pie\File\SudoFilePut; +use Php\Pie\File\SudoUnlink; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; @@ -13,8 +16,6 @@ use function is_writable; use function rtrim; use function sprintf; -use function touch; -use function unlink; use const DIRECTORY_SEPARATOR; @@ -44,10 +45,22 @@ public function setup( return false; } - if (! file_exists($additionalIniFilesPath) || ! is_writable($additionalIniFilesPath)) { + if (! file_exists($additionalIniFilesPath)) { $output->writeln( sprintf( - 'PHP is configured to use additional INI file path %s, but it did not exist, or is not writable by PIE.', + 'PHP is configured to use additional INI file path %s, but it did not exist.', + $additionalIniFilesPath, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + if (! is_writable($additionalIniFilesPath) && ! Sudo::exists()) { + $output->writeln( + sprintf( + 'PHP is configured to use additional INI file path %s, but it was not writable by PIE.', $additionalIniFilesPath, ), OutputInterface::VERBOSITY_VERBOSE, @@ -74,7 +87,7 @@ public function setup( OutputInterface::VERBOSITY_VERY_VERBOSE, ); $pieCreatedTheIniFile = true; - touch($expectedIniFile); + SudoFilePut::contents($expectedIniFile, ''); } $addingExtensionWasSuccessful = ($this->checkAndAddExtensionToIniIfNeeded)( @@ -86,7 +99,7 @@ public function setup( ); if (! $addingExtensionWasSuccessful && $pieCreatedTheIniFile) { - unlink($expectedIniFile); + SudoUnlink::singleFile($expectedIniFile); } return $addingExtensionWasSuccessful; diff --git a/src/Installing/UninstallUsingUnlink.php b/src/Installing/UninstallUsingUnlink.php index 0813a81..ea11f9d 100644 --- a/src/Installing/UninstallUsingUnlink.php +++ b/src/Installing/UninstallUsingUnlink.php @@ -7,13 +7,14 @@ use Php\Pie\ComposerIntegration\PieInstalledJsonMetadataKeys; use Php\Pie\DependencyResolver\Package; use Php\Pie\File\BinaryFile; +use Php\Pie\File\FailedToUnlinkFile; use Php\Pie\File\Sudo; +use Php\Pie\File\SudoUnlink; use Php\Pie\Util\Process; use function array_key_exists; use function file_exists; use function is_writable; -use function unlink; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ class UninstallUsingUnlink implements Uninstall @@ -53,8 +54,10 @@ public function __invoke(Package $package): BinaryFile } } - if (file_exists($expectedBinaryFile->filePath) && ! unlink($expectedBinaryFile->filePath)) { - throw FailedToRemoveExtension::withFilename($expectedBinaryFile); + try { + SudoUnlink::singleFile($expectedBinaryFile->filePath); + } catch (FailedToUnlinkFile $failedToUnlinkFile) { + throw FailedToRemoveExtension::withFilename($expectedBinaryFile, $failedToUnlinkFile); } return $expectedBinaryFile; diff --git a/test/unit/File/SudoUnlinkTest.php b/test/unit/File/SudoUnlinkTest.php new file mode 100644 index 0000000..77fa044 --- /dev/null +++ b/test/unit/File/SudoUnlinkTest.php @@ -0,0 +1,37 @@ +output, )); self::assertStringContainsString( - 'PHP is configured to use additional INI file path /path/to/something/does/not/exist, but it did not exist, or is not writable by PIE.', + 'PHP is configured to use additional INI file path /path/to/something/does/not/exist, but it did not exist', $this->output->fetch(), ); } From 7d4586689bb9c3a494c4ff86f804c8719beb4b05 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 28 Feb 2025 09:59:50 +0000 Subject: [PATCH 25/25] Only try to chmod file in SudoFilePut if it already exists --- phpunit.xml.dist | 1 + src/File/SudoFilePut.php | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 2808b46..430ba14 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -8,6 +8,7 @@ requireCoverageMetadata="true" beStrictAboutOutputDuringTests="true" displayDetailsOnSkippedTests="true" + displayDetailsOnTestsThatTriggerWarnings="true" failOnRisky="true" failOnWarning="true"> diff --git a/src/File/SudoFilePut.php b/src/File/SudoFilePut.php index 49a7504..32cdcc1 100644 --- a/src/File/SudoFilePut.php +++ b/src/File/SudoFilePut.php @@ -8,8 +8,10 @@ use Php\Pie\Util\Process; use Symfony\Component\Process\Exception\ProcessFailedException; +use function file_exists; use function file_put_contents; use function fileperms; +use function is_string; use function is_writable; use function sprintf; use function substr; @@ -19,9 +21,12 @@ final class SudoFilePut { public static function contents(string $filename, string $content): void { - $previousPermissions = substr(sprintf('%o', fileperms($filename)), -4); + $didChangePermissions = false; + if (file_exists($filename)) { + $previousPermissions = substr(sprintf('%o', fileperms($filename)), -4); - $didChangePermissions = self::attemptToMakeFileEditable($filename); + $didChangePermissions = self::attemptToMakeFileEditable($filename); + } $capturedErrors = []; $writeSuccessful = CaptureErrors::for( @@ -33,7 +38,7 @@ public static function contents(string $filename, string $content): void throw FailedToWriteFile::fromFilePutContentErrors($filename, $capturedErrors); } - if (! $didChangePermissions || ! Sudo::exists()) { + if (! isset($previousPermissions) || ! is_string($previousPermissions) || ! $didChangePermissions || ! Sudo::exists()) { return; }