From b5e03d7e7dbf0bc4153f3bc76d7c9e406b63836d Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 5 Dec 2024 08:48:27 +0000 Subject: [PATCH 01/18] Extract setup of PHP INI files into new component --- src/Container.php | 8 + .../Ini/PickBestSetupIniApproach.php | 79 +++++++ src/Installing/Ini/SetupIniApproach.php | 31 +++ src/Installing/SetupIniFile.php | 47 +++++ src/Installing/UnixInstall.php | 27 +-- src/Installing/WindowsInstall.php | 15 +- .../Installing/UnixInstallTest.php | 4 +- .../Installing/WindowsInstallTest.php | 4 +- .../Ini/PickBestSetupIniApproachTest.php | 193 ++++++++++++++++++ 9 files changed, 392 insertions(+), 16 deletions(-) create mode 100644 src/Installing/Ini/PickBestSetupIniApproach.php create mode 100644 src/Installing/Ini/SetupIniApproach.php create mode 100644 src/Installing/SetupIniFile.php create mode 100644 test/unit/Installing/Ini/PickBestSetupIniApproachTest.php diff --git a/src/Container.php b/src/Container.php index 263fd418..975a8b28 100644 --- a/src/Container.php +++ b/src/Container.php @@ -19,6 +19,7 @@ use Php\Pie\DependencyResolver\ResolveDependencyWithComposer; use Php\Pie\Downloading\GithubPackageReleaseAssets; use Php\Pie\Downloading\PackageReleaseAssets; +use Php\Pie\Installing\Ini; use Php\Pie\Installing\Install; use Php\Pie\Installing\UnixInstall; use Php\Pie\Installing\WindowsInstall; @@ -71,6 +72,13 @@ static function (ContainerInterface $container): Build { }, ); + $container->singleton( + Ini\SetupIniApproach::class, + static function (): Ini\SetupIniApproach { + return new Ini\PickBestSetupIniApproach([]); + }, + ); + $container->singleton( Install::class, static function (ContainerInterface $container): Install { diff --git a/src/Installing/Ini/PickBestSetupIniApproach.php b/src/Installing/Ini/PickBestSetupIniApproach.php new file mode 100644 index 00000000..83c719d3 --- /dev/null +++ b/src/Installing/Ini/PickBestSetupIniApproach.php @@ -0,0 +1,79 @@ +|null */ + private array|null $memoizedApproachesThatCanBeUsed = null; + + /** @param list $possibleApproaches */ + public function __construct( + private readonly array $possibleApproaches, + ) { + } + + /** @return list */ + private function approachesThatCanBeUsed(TargetPlatform $targetPlatform): array + { + if ($this->memoizedApproachesThatCanBeUsed === null) { + $this->memoizedApproachesThatCanBeUsed = array_values(array_filter( + $this->possibleApproaches, + static fn (SetupIniApproach $approach) => $approach->canBeUsed($targetPlatform), + )); + } + + return $this->memoizedApproachesThatCanBeUsed; + } + + public function canBeUsed(TargetPlatform $targetPlatform): bool + { + return count($this->approachesThatCanBeUsed($targetPlatform)) > 0; + } + + public function setup( + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + BinaryFile $binaryFile, + OutputInterface $output, + ): bool { + $approaches = $this->approachesThatCanBeUsed($targetPlatform); + + if (count($approaches) === 0) { + $output->writeln('No INI setup approaches can be used on this platform.', OutputInterface::VERBOSITY_VERBOSE); + + return false; + } + + foreach ($approaches as $approach) { + $output->writeln( + sprintf( + 'Trying to enable extension using %s', + (new ReflectionClass($approach))->getShortName(), + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + if ($approach->setup($targetPlatform, $downloadedPackage, $binaryFile, $output)) { + return true; + } + } + + $output->writeln('None of the INI setup approaches succeeded.', OutputInterface::VERBOSITY_VERBOSE); + + return false; + } +} diff --git a/src/Installing/Ini/SetupIniApproach.php b/src/Installing/Ini/SetupIniApproach.php new file mode 100644 index 00000000..37870e90 --- /dev/null +++ b/src/Installing/Ini/SetupIniApproach.php @@ -0,0 +1,31 @@ +setupIniApproach->canBeUsed($targetPlatform) + && $this->setupIniApproach->setup($targetPlatform, $downloadedPackage, $binaryFile, $output) + ) { + $output->writeln(sprintf( + '✅ Extension is enabled and loaded in %s', + $targetPlatform->phpBinaryPath->phpBinaryPath, + )); + } else { + $output->writeln('⚠️ Extension has NOT been automatically enabled.'); + $output->writeln(sprintf( + 'You must now add "%s=%s" to your php.ini', + $downloadedPackage->package->extensionType === ExtensionType::PhpModule ? 'extension' : 'zend_extension', + $downloadedPackage->package->extensionName->name(), + )); + } + } +} diff --git a/src/Installing/UnixInstall.php b/src/Installing/UnixInstall.php index 6411e6b5..bda19108 100644 --- a/src/Installing/UnixInstall.php +++ b/src/Installing/UnixInstall.php @@ -6,7 +6,6 @@ use Php\Pie\BinaryFile; use Php\Pie\Downloading\DownloadedPackage; -use Php\Pie\ExtensionType; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Util\Process; use RuntimeException; @@ -22,6 +21,10 @@ final class UnixInstall implements Install { private const MAKE_INSTALL_TIMEOUT_SECS = 60; // 1 minute + public function __construct(private readonly SetupIniFile $setupIniFile) + { + } + public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): BinaryFile { $targetExtensionPath = $targetPlatform->phpBinaryPath->extensionPath(); @@ -63,17 +66,15 @@ public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $t $output->writeln('Install complete: ' . $expectedSharedObjectLocation); - /** - * @link https://github.com/php/pie/issues/20 - * - * @todo this should be improved in future to try to automatically set up the ext - */ - $output->writeln(sprintf( - 'You must now add "%s=%s" to your php.ini', - $downloadedPackage->package->extensionType === ExtensionType::PhpModule ? 'extension' : 'zend_extension', - $downloadedPackage->package->extensionName->name(), - )); - - return BinaryFile::fromFileWithSha256Checksum($expectedSharedObjectLocation); + $binaryFile = BinaryFile::fromFileWithSha256Checksum($expectedSharedObjectLocation); + + ($this->setupIniFile)( + $targetPlatform, + $downloadedPackage, + $binaryFile, + $output, + ); + + return $binaryFile; } } diff --git a/src/Installing/WindowsInstall.php b/src/Installing/WindowsInstall.php index a9f6c3c1..4a993383 100644 --- a/src/Installing/WindowsInstall.php +++ b/src/Installing/WindowsInstall.php @@ -31,6 +31,10 @@ /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ final class WindowsInstall implements Install { + public function __construct(private readonly SetupIniFile $setupIniFile) + { + } + public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): BinaryFile { $extractedSourcePath = $downloadedPackage->extractedSourcePath; @@ -82,7 +86,16 @@ public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $t $downloadedPackage->package->extensionName->name(), )); - return BinaryFile::fromFileWithSha256Checksum($destinationDllName); + $binaryFile = BinaryFile::fromFileWithSha256Checksum($destinationDllName); + + ($this->setupIniFile)( + $targetPlatform, + $downloadedPackage, + $binaryFile, + $output, + ); + + return $binaryFile; } /** diff --git a/test/integration/Installing/UnixInstallTest.php b/test/integration/Installing/UnixInstallTest.php index 049a7af4..159844d2 100644 --- a/test/integration/Installing/UnixInstallTest.php +++ b/test/integration/Installing/UnixInstallTest.php @@ -12,6 +12,8 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\Installing\Ini\PickBestSetupIniApproach; +use Php\Pie\Installing\SetupIniFile; use Php\Pie\Installing\UnixInstall; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Php\Pie\Platform\TargetPlatform; @@ -105,7 +107,7 @@ public function testUnixInstallCanInstallExtension(string $phpConfig): void null, ); - $installedSharedObject = (new UnixInstall())->__invoke( + $installedSharedObject = (new UnixInstall(new SetupIniFile(new PickBestSetupIniApproach([]))))->__invoke( $downloadedPackage, $targetPlatform, $output, diff --git a/test/integration/Installing/WindowsInstallTest.php b/test/integration/Installing/WindowsInstallTest.php index b8664728..b9a469f9 100644 --- a/test/integration/Installing/WindowsInstallTest.php +++ b/test/integration/Installing/WindowsInstallTest.php @@ -9,6 +9,8 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; +use Php\Pie\Installing\Ini\PickBestSetupIniApproach; +use Php\Pie\Installing\SetupIniFile; use Php\Pie\Installing\WindowsInstall; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; @@ -73,7 +75,7 @@ public function testWindowsInstallCanInstallExtension(): void $phpPath = dirname($targetPlatform->phpBinaryPath->phpBinaryPath); $extensionPath = $targetPlatform->phpBinaryPath->extensionPath(); - $installer = new WindowsInstall(); + $installer = new WindowsInstall(new SetupIniFile(new PickBestSetupIniApproach([]))); $installedDll = $installer->__invoke($downloadedPackage, $targetPlatform, $output); self::assertSame($extensionPath . '\php_pie_test_ext.dll', $installedDll->filePath); diff --git a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php new file mode 100644 index 00000000..0e0499f0 --- /dev/null +++ b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php @@ -0,0 +1,193 @@ +canBeUsed($this->targetPlatform())); + } + + public function testCannotBeUsedWithAnyApproaches(): void + { + $one = $this->createMock(SetupIniApproach::class); + $one->expects(self::once())->method('canBeUsed')->willReturn(false); + $two = $this->createMock(SetupIniApproach::class); + $two->expects(self::once())->method('canBeUsed')->willReturn(false); + + self::assertFalse((new PickBestSetupIniApproach([$one, $two]))->canBeUsed($this->targetPlatform())); + } + + public function testCanBeUsedWithApproachOne(): void + { + $one = $this->createMock(SetupIniApproach::class); + $one->expects(self::once())->method('canBeUsed')->willReturn(false); + $two = $this->createMock(SetupIniApproach::class); + $two->expects(self::once())->method('canBeUsed')->willReturn(true); + + self::assertTrue((new PickBestSetupIniApproach([$one, $two]))->canBeUsed($this->targetPlatform())); + } + + public function testCanBeUsedWithApproachTwo(): void + { + $one = $this->createMock(SetupIniApproach::class); + $one->expects(self::once())->method('canBeUsed')->willReturn(true); + $two = $this->createMock(SetupIniApproach::class); + $two->expects(self::once())->method('canBeUsed')->willReturn(false); + + self::assertTrue((new PickBestSetupIniApproach([$one, $two]))->canBeUsed($this->targetPlatform())); + } + + public function testCanBeUsedWithAllApproaches(): void + { + $one = $this->createMock(SetupIniApproach::class); + $one->expects(self::once())->method('canBeUsed')->willReturn(true); + $two = $this->createMock(SetupIniApproach::class); + $two->expects(self::once())->method('canBeUsed')->willReturn(true); + + self::assertTrue((new PickBestSetupIniApproach([$one, $two]))->canBeUsed($this->targetPlatform())); + } + + public function testVerboseMessageIsEmittedSettingUpWithoutAnyApproaches(): void + { + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + self::assertFalse((new PickBestSetupIniApproach([]))->setup( + $this->targetPlatform(), + DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foo'), + 'test-vendor/test-package', + '1.2.3', + 'https://test-uri/', + [], + true, + true, + null, + ), + '/path/to/extracted/source', + ), + new BinaryFile('/path/to/extracted/source/module/foo.so', 'some-checksum'), + $output, + )); + + $outputString = $output->fetch(); + self::assertStringContainsString( + 'No INI setup approaches can be used on this platform.', + $outputString, + ); + } + + public function testWorkingApproachIsUsed(): void + { + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $one = $this->createMock(SetupIniApproach::class); + $one->method('canBeUsed')->willReturn(true); + $one->expects(self::once())->method('setup')->willReturn(false); + $two = $this->createMock(SetupIniApproach::class); + $two->method('canBeUsed')->willReturn(true); + $two->expects(self::once())->method('setup')->willReturn(true); + + self::assertTrue((new PickBestSetupIniApproach([$one, $two]))->setup( + $this->targetPlatform(), + DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foo'), + 'test-vendor/test-package', + '1.2.3', + 'https://test-uri/', + [], + true, + true, + null, + ), + '/path/to/extracted/source', + ), + new BinaryFile('/path/to/extracted/source/module/foo.so', 'some-checksum'), + $output, + )); + + $outputString = $output->fetch(); + self::assertStringContainsString( + 'Trying to enable extension using MockObject_SetupIniApproach', + $outputString, + ); + } + + public function testSetupFailsWhenNoApproachesWork(): void + { + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $one = $this->createMock(SetupIniApproach::class); + $one->method('canBeUsed')->willReturn(true); + $one->expects(self::once())->method('setup')->willReturn(false); + $two = $this->createMock(SetupIniApproach::class); + $two->method('canBeUsed')->willReturn(true); + $two->expects(self::once())->method('setup')->willReturn(false); + + self::assertFalse((new PickBestSetupIniApproach([$one, $two]))->setup( + $this->targetPlatform(), + DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foo'), + 'test-vendor/test-package', + '1.2.3', + 'https://test-uri/', + [], + true, + true, + null, + ), + '/path/to/extracted/source', + ), + new BinaryFile('/path/to/extracted/source/module/foo.so', 'some-checksum'), + $output, + )); + + $outputString = $output->fetch(); + self::assertStringContainsString( + 'None of the INI setup approaches succeeded.', + $outputString, + ); + } +} From fa1b2d3730e0457576b5256109f63c52d6d64a91 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 6 Dec 2024 09:05:13 +0000 Subject: [PATCH 02/18] Add assertion to PhpBinaryPath to ensure an ext is loaded in runtime --- .../Exception/ExtensionIsNotLoaded.php | 23 +++++++++++++ src/Platform/TargetPhp/PhpBinaryPath.php | 26 +++++++++++++++ .../Platform/TargetPhp/PhpBinaryPathTest.php | 33 +++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 src/Platform/TargetPhp/Exception/ExtensionIsNotLoaded.php diff --git a/src/Platform/TargetPhp/Exception/ExtensionIsNotLoaded.php b/src/Platform/TargetPhp/Exception/ExtensionIsNotLoaded.php new file mode 100644 index 00000000..7955186f --- /dev/null +++ b/src/Platform/TargetPhp/Exception/ExtensionIsNotLoaded.php @@ -0,0 +1,23 @@ +name(), + $php->phpBinaryPath, + )); + } +} diff --git a/src/Platform/TargetPhp/PhpBinaryPath.php b/src/Platform/TargetPhp/PhpBinaryPath.php index 7589b2fa..63c133e6 100644 --- a/src/Platform/TargetPhp/PhpBinaryPath.php +++ b/src/Platform/TargetPhp/PhpBinaryPath.php @@ -6,22 +6,26 @@ use Composer\Semver\VersionParser; use Composer\Util\Platform; +use Php\Pie\ExtensionName; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; use Php\Pie\Platform\OperatingSystemFamily; use Php\Pie\Util\Process; use RuntimeException; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Process\PhpExecutableFinder; use Webmozart\Assert\Assert; use function array_combine; use function array_key_exists; +use function array_keys; use function array_map; use function assert; use function dirname; use function explode; use function file_exists; use function implode; +use function in_array; use function is_dir; use function is_executable; use function preg_match; @@ -112,6 +116,28 @@ public function extensionPath(): string throw new RuntimeException('Could not determine extension path for ' . $this->phpBinaryPath); } + public function assertExtensionIsLoadedInRuntime(ExtensionName $extension, OutputInterface|null $output = null): void + { + if (! in_array($extension->name(), array_keys($this->extensions()))) { + throw Exception\ExtensionIsNotLoaded::fromExpectedExtension( + $this, + $extension, + ); + } + + if ($output === null) { + return; + } + + $output->writeln( + sprintf( + 'Successfully asserted that extension %s is loaded in runtime.', + $extension->name(), + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + } + /** * Returns a map where the key is the name of the extension and the value is the version ('0' if not defined) * diff --git a/test/unit/Platform/TargetPhp/PhpBinaryPathTest.php b/test/unit/Platform/TargetPhp/PhpBinaryPathTest.php index 39ffdaa3..5e3bd10f 100644 --- a/test/unit/Platform/TargetPhp/PhpBinaryPathTest.php +++ b/test/unit/Platform/TargetPhp/PhpBinaryPathTest.php @@ -5,22 +5,27 @@ namespace Php\PieUnitTest\Platform\TargetPhp; use Composer\Util\Platform; +use Php\Pie\ExtensionName; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; use Php\Pie\Platform\OperatingSystemFamily; +use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPhp\Exception\InvalidPhpBinaryPath; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Symfony\Component\Console\Output\BufferedOutput; use Symfony\Component\Process\PhpExecutableFinder; use function array_column; use function array_combine; use function array_filter; +use function array_key_exists; use function array_map; use function array_unique; use function assert; +use function count; use function defined; use function dirname; use function file_exists; @@ -271,4 +276,32 @@ public function testDifferentVersionsOfPhp(string $phpPath): void self::assertGreaterThan(0, $php->phpIntSize()); self::assertNotEmpty($php->phpinfo()); } + + public function testAssertExtensionIsLoaded(): void + { + $php = PhpBinaryPath::fromCurrentProcess(); + $loadedExtensions = $php->extensions(); + + if (! count($loadedExtensions) || ! array_key_exists('Core', $loadedExtensions)) { + self::fail('Core extension is not loaded, this is quite unexpected...'); + } + + $output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + $php->assertExtensionIsLoadedInRuntime(ExtensionName::normaliseFromString('Core'), $output); + + self::assertStringContainsString( + 'Successfully asserted that extension Core is loaded in runtime.', + $output->fetch(), + ); + } + + public function testAssertExtensionFailsWhenNotLoaded(): void + { + $php = PhpBinaryPath::fromCurrentProcess(); + + $this->expectException(ExtensionIsNotLoaded::class); + $php->assertExtensionIsLoadedInRuntime(ExtensionName::normaliseFromString( + 'hopefully_this_extension_name_is_not_real_otherwise_this_test_will_fail', + )); + } } From bd32d3f3abbe6243a1ca016f2a1fd26714ef1fa6 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 6 Dec 2024 09:17:58 +0000 Subject: [PATCH 03/18] Added method to check if an extension is already in an INI file --- .../Ini/IsExtensionAlreadyInTheIniFile.php | 52 +++++++++++++++++++ .../with_commented_extension.ini | 1 + .../example_ini_files/with_extension.ini | 1 + .../example_ini_files/with_zend_extension.ini | 1 + .../example_ini_files/without_extension.ini | 1 + .../IsExtensionAlreadyInTheIniFileTest.php | 49 +++++++++++++++++ 6 files changed, 105 insertions(+) create mode 100644 src/Installing/Ini/IsExtensionAlreadyInTheIniFile.php create mode 100644 test/assets/example_ini_files/with_commented_extension.ini create mode 100644 test/assets/example_ini_files/with_extension.ini create mode 100644 test/assets/example_ini_files/with_zend_extension.ini create mode 100644 test/assets/example_ini_files/without_extension.ini create mode 100644 test/unit/Installing/Ini/IsExtensionAlreadyInTheIniFileTest.php diff --git a/src/Installing/Ini/IsExtensionAlreadyInTheIniFile.php b/src/Installing/Ini/IsExtensionAlreadyInTheIniFile.php new file mode 100644 index 00000000..cfe2c8bf --- /dev/null +++ b/src/Installing/Ini/IsExtensionAlreadyInTheIniFile.php @@ -0,0 +1,52 @@ +readIniFile($iniFilePath); + + return in_array($extensionName->name(), array_merge($loadedExts['extensions'], $loadedExts['zend_extensions'])); + } + + /** @return array{extensions: list, zend_extensions: list} */ + private function readIniFile(string $iniFilePath): array + { + $iniFileContentLines = file($iniFilePath); + + $extensions = []; + $zendExtensions = []; + foreach ($iniFileContentLines as $line) { + $lineIni = parse_ini_string($line); + + if (array_key_exists('extension', $lineIni) && is_string($lineIni['extension']) && $lineIni['extension'] !== '') { + $extensions[] = $lineIni['extension']; + } + + if (! array_key_exists('zend_extension', $lineIni) || ! is_string($lineIni['zend_extension']) || $lineIni['zend_extension'] === '') { + continue; + } + + $zendExtensions[] = $lineIni['zend_extension']; + } + + return [ + 'extensions' => $extensions, + 'zend_extensions' => $zendExtensions, + ]; + } +} diff --git a/test/assets/example_ini_files/with_commented_extension.ini b/test/assets/example_ini_files/with_commented_extension.ini new file mode 100644 index 00000000..2beb79d2 --- /dev/null +++ b/test/assets/example_ini_files/with_commented_extension.ini @@ -0,0 +1 @@ +;extension=foobar diff --git a/test/assets/example_ini_files/with_extension.ini b/test/assets/example_ini_files/with_extension.ini new file mode 100644 index 00000000..40e7397e --- /dev/null +++ b/test/assets/example_ini_files/with_extension.ini @@ -0,0 +1 @@ +extension=foobar diff --git a/test/assets/example_ini_files/with_zend_extension.ini b/test/assets/example_ini_files/with_zend_extension.ini new file mode 100644 index 00000000..d2b87581 --- /dev/null +++ b/test/assets/example_ini_files/with_zend_extension.ini @@ -0,0 +1 @@ +zend_extension=foobar diff --git a/test/assets/example_ini_files/without_extension.ini b/test/assets/example_ini_files/without_extension.ini new file mode 100644 index 00000000..131f894d --- /dev/null +++ b/test/assets/example_ini_files/without_extension.ini @@ -0,0 +1 @@ +html_errors=1 diff --git a/test/unit/Installing/Ini/IsExtensionAlreadyInTheIniFileTest.php b/test/unit/Installing/Ini/IsExtensionAlreadyInTheIniFileTest.php new file mode 100644 index 00000000..18f1a293 --- /dev/null +++ b/test/unit/Installing/Ini/IsExtensionAlreadyInTheIniFileTest.php @@ -0,0 +1,49 @@ + Date: Mon, 9 Dec 2024 09:01:41 +0000 Subject: [PATCH 04/18] Added helper to add extension to an INI file --- .../Ini/AddExtensionToTheIniFile.php | 85 ++++++++ .../Ini/AddExtensionToTheIniFileTest.php | 203 ++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100644 src/Installing/Ini/AddExtensionToTheIniFile.php create mode 100644 test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php diff --git a/src/Installing/Ini/AddExtensionToTheIniFile.php b/src/Installing/Ini/AddExtensionToTheIniFile.php new file mode 100644 index 00000000..7c0264d6 --- /dev/null +++ b/src/Installing/Ini/AddExtensionToTheIniFile.php @@ -0,0 +1,85 @@ +writeln( + sprintf( + 'PHP is configured to use %s, but it is not writable by PIE.', + $ini, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + $originalIniContent = file_get_contents($ini); + + if (! is_string($originalIniContent)) { + $output->writeln( + sprintf( + 'Tried making a backup of %s but could not read it, aborting enablement of extension', + $ini, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + try { + file_put_contents( + $ini, + $originalIniContent . PHP_EOL + . '; PIE automatically added this to enable the ' . $package->name . ' extension' . PHP_EOL + . ($package->extensionType === ExtensionType::PhpModule ? 'extension' : 'zend_extension') + . '=' + . $package->extensionName->name() . PHP_EOL, + ); + $output->writeln( + sprintf( + 'Enabled extension %s in the INI file %s', + $package->extensionName->name(), + $ini, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + $phpBinaryPath->assertExtensionIsLoadedInRuntime($package->extensionName, $output); + + return true; + } catch (Throwable $anything) { + file_put_contents($ini, $originalIniContent); + + $output->writeln(sprintf( + 'Something went wrong enabling the %s extension: %s', + $package->extensionName->name(), + $anything->getMessage(), + )); + + return false; + } + } +} diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php new file mode 100644 index 00000000..bf5bf9ca --- /dev/null +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -0,0 +1,203 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + } + + public function testReturnsFalseWhenFileIsNotWritable(): void + { + $unwritableFilename = tempnam(sys_get_temp_dir(), 'PIE_unwritable_ini_file'); + touch($unwritableFilename); + chmod($unwritableFilename, 000); + + try { + self::assertFalse((new AddExtensionToTheIniFile())( + $unwritableFilename, + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.0.0', + null, + [], + true, + true, + null, + ), + $this->mockPhpBinary, + $this->output, + )); + + self::assertStringContainsString( + sprintf('PHP is configured to use %s, but it is not writable by PIE.', $unwritableFilename), + $this->output->fetch(), + ); + } finally { + chmod($unwritableFilename, 644); + unlink($unwritableFilename); + } + } + + public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void + { + $unreadableIniFile = tempnam(sys_get_temp_dir(), 'PIE_unreadable_ini_file'); + touch($unreadableIniFile); + chmod($unreadableIniFile, 222); + + try { + self::assertFalse((new AddExtensionToTheIniFile())( + $unreadableIniFile, + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.0.0', + null, + [], + true, + true, + null, + ), + $this->mockPhpBinary, + $this->output, + )); + + self::assertStringContainsString( + sprintf('Tried making a backup of %s but could not read it, aborting enablement of extension', $unreadableIniFile), + $this->output->fetch(), + ); + } finally { + chmod($unreadableIniFile, 644); + unlink($unreadableIniFile); + } + } + + public function testReturnsFalseWhenExtensionWasAddedButPhpRuntimeDidNotLoadExtension(): void + { + $extensionName = ExtensionName::normaliseFromString('foobar'); + $originalIniContent = "; some comment\nerror_reporting=E_ALL\n"; + + $iniFile = tempnam(sys_get_temp_dir(), 'PIE_ini_file'); + file_put_contents($iniFile, $originalIniContent); + + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->willThrowException( + ExtensionIsNotLoaded::fromExpectedExtension($this->mockPhpBinary, $extensionName), + ); + + try { + self::assertFalse((new AddExtensionToTheIniFile())( + $iniFile, + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + $extensionName, + 'foo/bar', + '1.0.0', + null, + [], + true, + true, + null, + ), + $this->mockPhpBinary, + $this->output, + )); + + self::assertStringContainsString( + 'Something went wrong enabling the foobar extension: Expected extension foobar to be loaded in PHP /path/to/php, but it was not detected.', + $this->output->fetch(), + ); + + // Ensure the original INI file content was restored + self::assertSame($originalIniContent, file_get_contents($iniFile)); + } finally { + unlink($iniFile); + } + } + + public function testReturnsTrueWhenExtensionAdded(): void + { + $iniFile = tempnam(sys_get_temp_dir(), 'PIE_ini_file'); + touch($iniFile); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime'); + + try { + self::assertTrue((new AddExtensionToTheIniFile())( + $iniFile, + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.0.0', + null, + [], + true, + true, + null, + ), + $this->mockPhpBinary, + $this->output, + )); + + $iniContent = file_get_contents($iniFile); + self::assertSame("\n; PIE automatically added this to enable the foo/bar extension\nextension=foobar\n", $iniContent); + + self::assertStringContainsString( + sprintf('Enabled extension foobar in the INI file %s', $iniFile), + $this->output->fetch(), + ); + } finally { + unlink($iniFile); + } + } +} From 75bdc189aa77da04b5056a3dc17060dffc54f43b Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 9 Dec 2024 10:10:41 +0000 Subject: [PATCH 05/18] Added standard single php.ini approach --- src/Container.php | 6 +- src/Installing/Ini/StandardSinglePhpIni.php | 98 ++++++ .../Ini/StandardSinglePhpIniTest.php | 309 ++++++++++++++++++ 3 files changed, 411 insertions(+), 2 deletions(-) create mode 100644 src/Installing/Ini/StandardSinglePhpIni.php create mode 100644 test/unit/Installing/Ini/StandardSinglePhpIniTest.php diff --git a/src/Container.php b/src/Container.php index 975a8b28..2c07ec9a 100644 --- a/src/Container.php +++ b/src/Container.php @@ -74,8 +74,10 @@ static function (ContainerInterface $container): Build { $container->singleton( Ini\SetupIniApproach::class, - static function (): Ini\SetupIniApproach { - return new Ini\PickBestSetupIniApproach([]); + static function (ContainerInterface $container): Ini\SetupIniApproach { + return new Ini\PickBestSetupIniApproach([ + $container->get(Ini\StandardSinglePhpIni::class), + ]); }, ); diff --git a/src/Installing/Ini/StandardSinglePhpIni.php b/src/Installing/Ini/StandardSinglePhpIni.php new file mode 100644 index 00000000..bbec74d9 --- /dev/null +++ b/src/Installing/Ini/StandardSinglePhpIni.php @@ -0,0 +1,98 @@ +extractPhpIniFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()) !== null; + } + + public function setup( + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + BinaryFile $binaryFile, + OutputInterface $output, + ): bool { + $ini = $this->extractPhpIniFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()); + + /** In practice, this shouldn't happen since {@see canBeUsed()} checks this */ + if ($ini === null) { + return false; + } + + if (! file_exists($ini) || ! is_readable($ini)) { + $output->writeln( + sprintf( + 'PHP is configured to use %s, but it did not exist, or is not readable by PIE.', + $ini, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + if (($this->isExtensionAlreadyInTheIniFile)($ini, $downloadedPackage->package->extensionName)) { + $output->writeln( + sprintf( + 'Extension is already enabled in the INI file %s', + $ini, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + try { + $targetPlatform->phpBinaryPath->assertExtensionIsLoadedInRuntime($downloadedPackage->package->extensionName, $output); + + return true; + } catch (Throwable $anything) { + $output->writeln(sprintf( + 'Something went wrong verifying the %s extension is enabled: %s', + $downloadedPackage->package->extensionName->name(), + $anything->getMessage(), + )); + + return false; + } + } + + return ($this->addExtensionToTheIniFile)($ini, $downloadedPackage->package, $targetPlatform->phpBinaryPath, $output); + } + + private function extractPhpIniFromPhpInfo(string $phpinfoString): string|null + { + if ( + preg_match('/Loaded Configuration File([ =>\t]*)(.*)/', $phpinfoString, $m) + && array_key_exists(2, $m) + && $m[2] !== '' + && $m[2] !== '(none)' + ) { + return $m[2]; + } + + return null; + } +} diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php new file mode 100644 index 00000000..7ccf4ef4 --- /dev/null +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -0,0 +1,309 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->isExtensionAlreadyInTheIniFile = $this->createMock(IsExtensionAlreadyInTheIniFile::class); + $this->addExtensionToTheIniFile = $this->createMock(AddExtensionToTheIniFile::class); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + ), + '/path/to/extracted/source', + ); + + $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); + + $this->standardSinglePhpIni = new StandardSinglePhpIni( + $this->isExtensionAlreadyInTheIniFile, + $this->addExtensionToTheIniFile, + ); + } + + public function testCannotBeUsedWithNoDefinedPhpIni(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => (none)'); + + self::assertFalse($this->standardSinglePhpIni->canBeUsed($this->targetPlatform)); + } + + public function testCanBeUsedWithDefinedPhpIni(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => /path/to/php.ini'); + + self::assertTrue($this->standardSinglePhpIni->canBeUsed($this->targetPlatform)); + } + + public function testSetupReturnsWhenCannotBeUsed(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => (none)'); + + self::assertFalse($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } + + public function testSetupWhenIniFileDoesNotExist(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => /path/to/non/existent/php.ini'); + + $this->isExtensionAlreadyInTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + + self::assertStringContainsString( + 'PHP is configured to use /path/to/non/existent/php.ini, but it did not exist, or is not readable by PIE.', + $this->output->fetch(), + ); + } + + public function testExtensionIsAlreadyEnabledButExtensionDoesNotLoad(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(true); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output) + ->willThrowException(ExtensionIsNotLoaded::fromExpectedExtension( + $this->mockPhpBinary, + $this->downloadedPackage->package->extensionName, + )); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + + $output = $this->output->fetch(); + self::assertStringContainsString( + 'Extension is already enabled in the INI file', + $output, + ); + self::assertStringContainsString( + 'Something went wrong verifying the foobar extension is enabled: Expected extension foobar to be loaded in PHP /path/to/php, but it was not detected.', + $output, + ); + } + + public function testExtensionIsAlreadyEnabledAndExtensionLoaded(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(true); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertTrue($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + + $output = $this->output->fetch(); + self::assertStringContainsString( + 'Extension is already enabled in the INI file', + $output, + ); + } + + public function testExtensionIsNotYetAdded(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(false); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with( + self::INI_FILE, + $this->downloadedPackage->package, + $this->targetPlatform->phpBinaryPath, + $this->output, + ) + ->willReturn(true); + + self::assertTrue($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } + + public function testExtensionIsNotYetAddedButFailsToBeAdded(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(false); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with( + self::INI_FILE, + $this->downloadedPackage->package, + $this->targetPlatform->phpBinaryPath, + $this->output, + ) + ->willReturn(false); + + self::assertFalse($this->standardSinglePhpIni->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } +} From dd7af3e2aead4c3d0aeda2faaea6749315184010 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 11 Dec 2024 11:51:28 +0000 Subject: [PATCH 06/18] Added --skip-enable-extension option to avoid auto enabling of extensions in php.ini --- src/Command/BuildCommand.php | 2 ++ src/Command/CommandHelper.php | 12 ++++++++++++ src/Command/DownloadCommand.php | 1 + src/Command/InfoCommand.php | 1 + src/Command/InstallCommand.php | 2 ++ src/ComposerIntegration/InstallAndBuildProcess.php | 1 + src/ComposerIntegration/PieComposerRequest.php | 1 + src/Installing/Install.php | 1 + src/Installing/SetupIniFile.php | 8 +++++++- src/Installing/UnixInstall.php | 9 +++++++-- src/Installing/WindowsInstall.php | 9 +++++++-- test/integration/Command/InstallCommandTest.php | 11 +++++++++-- .../ResolveDependencyWithComposerTest.php | 1 + test/integration/Installing/UnixInstallTest.php | 1 + test/integration/Installing/WindowsInstallTest.php | 2 +- .../InstallAndBuildProcessTest.php | 3 +++ .../InstalledJsonMetadataTest.php | 2 ++ .../OverrideWindowsUrlInstallListenerTest.php | 3 +++ .../Installing/Ini/AddExtensionToTheIniFileTest.php | 9 +++++++++ 19 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/Command/BuildCommand.php b/src/Command/BuildCommand.php index 46bd27e0..0860038e 100644 --- a/src/Command/BuildCommand.php +++ b/src/Command/BuildCommand.php @@ -54,6 +54,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Resolve, [], // Configure options are not needed for resolve only null, + false, // setting up INI not needed for build ), ); @@ -74,6 +75,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Build, $configureOptionsValues, CommandHelper::determinePhpizePathFromInputs($input), + false, // setting up INI not needed for build ), ); diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index 68b55d2c..8e25165b 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -38,6 +38,7 @@ final class CommandHelper private const OPTION_WITH_PHP_PATH = 'with-php-path'; private const OPTION_WITH_PHPIZE_PATH = 'with-phpize-path'; private const OPTION_MAKE_PARALLEL_JOBS = 'make-parallel-jobs'; + private const OPTION_SKIP_ENABLE_EXTENSION = 'skip-enable-extension'; /** @psalm-suppress UnusedConstructor */ private function __construct() @@ -79,6 +80,12 @@ public static function configureDownloadBuildInstallOptions(Command $command): v 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, + InputOption::VALUE_NONE, + 'Specify this to skip attempting to enable the extension in php.ini', + ); self::configurePhpConfigOptions($command); @@ -154,6 +161,11 @@ public static function determineTargetPlatformFromInputs(InputInterface $input, return $targetPlatform; } + public static function determineAttemptToSetupIniFile(InputInterface $input): bool + { + return ! $input->hasOption(self::OPTION_SKIP_ENABLE_EXTENSION) || ! $input->getOption(self::OPTION_SKIP_ENABLE_EXTENSION); + } + public static function determinePhpizePathFromInputs(InputInterface $input): PhpizePath|null { if ($input->hasOption(self::OPTION_WITH_PHPIZE_PATH)) { diff --git a/src/Command/DownloadCommand.php b/src/Command/DownloadCommand.php index a6c237d4..f6616290 100644 --- a/src/Command/DownloadCommand.php +++ b/src/Command/DownloadCommand.php @@ -56,6 +56,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Download, [], // Configure options are not needed for download only null, + false, // setting up INI not needed for download ), ); diff --git a/src/Command/InfoCommand.php b/src/Command/InfoCommand.php index 0d3a6cc3..9f1f806f 100644 --- a/src/Command/InfoCommand.php +++ b/src/Command/InfoCommand.php @@ -54,6 +54,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Resolve, [], // Configure options are not needed for resolve only null, + false, // setting up INI not needed for info ), ); diff --git a/src/Command/InstallCommand.php b/src/Command/InstallCommand.php index ca293165..aca34af2 100644 --- a/src/Command/InstallCommand.php +++ b/src/Command/InstallCommand.php @@ -59,6 +59,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Resolve, [], // Configure options are not needed for resolve only null, + false, // setting up INI not needed for resolve step ), ); @@ -79,6 +80,7 @@ public function execute(InputInterface $input, OutputInterface $output): int PieOperation::Install, $configureOptionsValues, CommandHelper::determinePhpizePathFromInputs($input), + CommandHelper::determineAttemptToSetupIniFile($input), ), ); diff --git a/src/ComposerIntegration/InstallAndBuildProcess.php b/src/ComposerIntegration/InstallAndBuildProcess.php index c6944720..39e62fee 100644 --- a/src/ComposerIntegration/InstallAndBuildProcess.php +++ b/src/ComposerIntegration/InstallAndBuildProcess.php @@ -77,6 +77,7 @@ public function __invoke( $downloadedPackage, $composerRequest->targetPlatform, $output, + $composerRequest->attemptToSetupIniFile, ), ); } diff --git a/src/ComposerIntegration/PieComposerRequest.php b/src/ComposerIntegration/PieComposerRequest.php index 536a3471..2679bdd9 100644 --- a/src/ComposerIntegration/PieComposerRequest.php +++ b/src/ComposerIntegration/PieComposerRequest.php @@ -24,6 +24,7 @@ public function __construct( public readonly PieOperation $operation, public readonly array $configureOptions, public readonly PhpizePath|null $phpizePath, + public readonly bool $attemptToSetupIniFile, ) { } } diff --git a/src/Installing/Install.php b/src/Installing/Install.php index 71e2849d..17982b41 100644 --- a/src/Installing/Install.php +++ b/src/Installing/Install.php @@ -20,5 +20,6 @@ public function __invoke( DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output, + bool $attemptToSetupIniFile, ): BinaryFile; } diff --git a/src/Installing/SetupIniFile.php b/src/Installing/SetupIniFile.php index dbe00f2b..d6ca58bb 100644 --- a/src/Installing/SetupIniFile.php +++ b/src/Installing/SetupIniFile.php @@ -26,9 +26,11 @@ public function __invoke( DownloadedPackage $downloadedPackage, BinaryFile $binaryFile, OutputInterface $output, + bool $attemptToSetupIniFile, ): void { if ( - $this->setupIniApproach->canBeUsed($targetPlatform) + $attemptToSetupIniFile + && $this->setupIniApproach->canBeUsed($targetPlatform) && $this->setupIniApproach->setup($targetPlatform, $downloadedPackage, $binaryFile, $output) ) { $output->writeln(sprintf( @@ -36,6 +38,10 @@ public function __invoke( $targetPlatform->phpBinaryPath->phpBinaryPath, )); } else { + if (! $attemptToSetupIniFile) { + $output->writeln('Automatic extension enabling was skipped.', OutputInterface::VERBOSITY_VERBOSE); + } + $output->writeln('⚠️ Extension has NOT been automatically enabled.'); $output->writeln(sprintf( 'You must now add "%s=%s" to your php.ini', diff --git a/src/Installing/UnixInstall.php b/src/Installing/UnixInstall.php index bda19108..da831eed 100644 --- a/src/Installing/UnixInstall.php +++ b/src/Installing/UnixInstall.php @@ -25,8 +25,12 @@ public function __construct(private readonly SetupIniFile $setupIniFile) { } - public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): BinaryFile - { + public function __invoke( + DownloadedPackage $downloadedPackage, + TargetPlatform $targetPlatform, + OutputInterface $output, + bool $attemptToSetupIniFile, + ): BinaryFile { $targetExtensionPath = $targetPlatform->phpBinaryPath->extensionPath(); $sharedObjectName = $downloadedPackage->package->extensionName->name() . '.so'; @@ -73,6 +77,7 @@ public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $t $downloadedPackage, $binaryFile, $output, + $attemptToSetupIniFile, ); return $binaryFile; diff --git a/src/Installing/WindowsInstall.php b/src/Installing/WindowsInstall.php index 4a993383..7ff5ef1d 100644 --- a/src/Installing/WindowsInstall.php +++ b/src/Installing/WindowsInstall.php @@ -35,8 +35,12 @@ public function __construct(private readonly SetupIniFile $setupIniFile) { } - public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): BinaryFile - { + public function __invoke( + DownloadedPackage $downloadedPackage, + TargetPlatform $targetPlatform, + OutputInterface $output, + bool $attemptToSetupIniFile, + ): BinaryFile { $extractedSourcePath = $downloadedPackage->extractedSourcePath; $sourceDllName = WindowsExtensionAssetName::determineDllName($targetPlatform, $downloadedPackage); $sourcePdbName = str_replace('.dll', '.pdb', $sourceDllName); @@ -93,6 +97,7 @@ public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $t $downloadedPackage, $binaryFile, $output, + $attemptToSetupIniFile, ); return $binaryFile; diff --git a/test/integration/Command/InstallCommandTest.php b/test/integration/Command/InstallCommandTest.php index c6ed049a..3257ae95 100644 --- a/test/integration/Command/InstallCommandTest.php +++ b/test/integration/Command/InstallCommandTest.php @@ -79,7 +79,11 @@ public function testInstallCommandWillInstallCompatibleExtensionNonWindows(strin } $this->commandTester->execute( - ['requested-package-and-version' => self::TEST_PACKAGE, '--with-php-config' => $phpConfigPath], + [ + 'requested-package-and-version' => self::TEST_PACKAGE, + '--with-php-config' => $phpConfigPath, + '--skip-enable-extension' => true, + ], ['verbosity' => BufferedOutput::VERBOSITY_VERY_VERBOSE], ); @@ -112,7 +116,10 @@ public function testInstallCommandWillInstallCompatibleExtensionNonWindows(strin #[RequiresOperatingSystemFamily('Windows')] public function testInstallCommandWillInstallCompatibleExtensionWindows(): void { - $this->commandTester->execute(['requested-package-and-version' => self::TEST_PACKAGE]); + $this->commandTester->execute([ + 'requested-package-and-version' => self::TEST_PACKAGE, + '--skip-enable-extension' => true, + ]); $this->commandTester->assertCommandIsSuccessful(); diff --git a/test/integration/DependencyResolver/ResolveDependencyWithComposerTest.php b/test/integration/DependencyResolver/ResolveDependencyWithComposerTest.php index c708dff1..1477b39c 100644 --- a/test/integration/DependencyResolver/ResolveDependencyWithComposerTest.php +++ b/test/integration/DependencyResolver/ResolveDependencyWithComposerTest.php @@ -90,6 +90,7 @@ public function testDependenciesAreResolvedToExpectedVersions( PieOperation::Resolve, [], null, + false, ), ), $targetPlatform, diff --git a/test/integration/Installing/UnixInstallTest.php b/test/integration/Installing/UnixInstallTest.php index 159844d2..100f1877 100644 --- a/test/integration/Installing/UnixInstallTest.php +++ b/test/integration/Installing/UnixInstallTest.php @@ -111,6 +111,7 @@ public function testUnixInstallCanInstallExtension(string $phpConfig): void $downloadedPackage, $targetPlatform, $output, + true, ); $outputString = $output->fetch(); diff --git a/test/integration/Installing/WindowsInstallTest.php b/test/integration/Installing/WindowsInstallTest.php index b9a469f9..7192617f 100644 --- a/test/integration/Installing/WindowsInstallTest.php +++ b/test/integration/Installing/WindowsInstallTest.php @@ -77,7 +77,7 @@ public function testWindowsInstallCanInstallExtension(): void $installer = new WindowsInstall(new SetupIniFile(new PickBestSetupIniApproach([]))); - $installedDll = $installer->__invoke($downloadedPackage, $targetPlatform, $output); + $installedDll = $installer->__invoke($downloadedPackage, $targetPlatform, $output, true); self::assertSame($extensionPath . '\php_pie_test_ext.dll', $installedDll->filePath); $outputString = $output->fetch(); diff --git a/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php b/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php index 1b186ad5..a0137f5b 100644 --- a/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php +++ b/test/unit/ComposerIntegration/InstallAndBuildProcessTest.php @@ -68,6 +68,7 @@ public function testDownloadWithoutBuildAndInstall(): void PieOperation::Download, ['--foo', '--bar="yes"'], null, + false, ); $composerPackage = new CompletePackage('foo/bar', '1.2.3.0', '1.2.3'); $installPath = '/path/to/install'; @@ -109,6 +110,7 @@ public function testDownloadAndBuildWithoutInstall(): void PieOperation::Build, ['--foo', '--bar="yes"'], null, + false, ); $composerPackage = new CompletePackage('foo/bar', '1.2.3.0', '1.2.3'); $installPath = '/path/to/install'; @@ -153,6 +155,7 @@ public function testDownloadBuildAndInstall(): void PieOperation::Install, ['--foo', '--bar="yes"'], null, + false, ); $composerPackage = new CompletePackage('foo/bar', '1.2.3.0', '1.2.3'); $installPath = '/path/to/install'; diff --git a/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php b/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php index 7151bb35..48790e74 100644 --- a/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php +++ b/test/unit/ComposerIntegration/InstalledJsonMetadataTest.php @@ -64,6 +64,7 @@ public function testMetadataForDownloads(): void PieOperation::Build, ['--foo', '--bar="yes"'], null, + false, ), clone $package, ); @@ -102,6 +103,7 @@ public function testMetadataForBuilds(): void PieOperation::Build, ['--foo', '--bar="yes"'], new PhpizePath('/path/to/phpize'), + false, ), clone $package, new BinaryFile('/path/to/built', 'sha256-checksum-value'), diff --git a/test/unit/ComposerIntegration/OverrideWindowsUrlInstallListenerTest.php b/test/unit/ComposerIntegration/OverrideWindowsUrlInstallListenerTest.php index 4022d462..8e9d2697 100644 --- a/test/unit/ComposerIntegration/OverrideWindowsUrlInstallListenerTest.php +++ b/test/unit/ComposerIntegration/OverrideWindowsUrlInstallListenerTest.php @@ -80,6 +80,7 @@ public function testEventListenerRegistration(): void PieOperation::Install, [], null, + false, ), ); } @@ -125,6 +126,7 @@ public function testWindowsUrlInstallerDoesNotRunOnNonWindows(): void PieOperation::Install, [], null, + false, ), ))($installerEvent); @@ -182,6 +184,7 @@ public function testDistUrlIsUpdatedForWindowsInstallers(): void PieOperation::Install, [], null, + false, ), ))($installerEvent); diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index bf5bf9ca..b7bd0d6e 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -11,6 +11,7 @@ use Php\Pie\Installing\Ini\AddExtensionToTheIniFile; use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; +use Php\Pie\Platform\TargetPlatform; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -41,6 +42,10 @@ public function setUp(): void public function testReturnsFalseWhenFileIsNotWritable(): void { + if (TargetPlatform::isRunningAsRoot()) { + self::markTestSkipped('Test cannot be run as root, as root can always write files'); + } + $unwritableFilename = tempnam(sys_get_temp_dir(), 'PIE_unwritable_ini_file'); touch($unwritableFilename); chmod($unwritableFilename, 000); @@ -76,6 +81,10 @@ public function testReturnsFalseWhenFileIsNotWritable(): void public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void { + if (TargetPlatform::isRunningAsRoot()) { + self::markTestSkipped('Test cannot be run as root, as root can always read files'); + } + $unreadableIniFile = tempnam(sys_get_temp_dir(), 'PIE_unreadable_ini_file'); touch($unreadableIniFile); chmod($unreadableIniFile, 222); From 80ba60b7efb1c5ad9fb9cce5b90308a5717eca07 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 11 Dec 2024 13:52:52 +0000 Subject: [PATCH 07/18] Ensure new TargetPlatform and Package constructor params are used after rebase --- test/behaviour/CliContext.php | 2 +- test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php | 8 ++++++++ test/unit/Installing/Ini/PickBestSetupIniApproachTest.php | 8 ++++++++ test/unit/Installing/Ini/StandardSinglePhpIniTest.php | 4 ++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/test/behaviour/CliContext.php b/test/behaviour/CliContext.php index 16076023..b1b3bff4 100644 --- a/test/behaviour/CliContext.php +++ b/test/behaviour/CliContext.php @@ -37,7 +37,7 @@ public function iRunACommandToDownloadSpecificVersionOfAnExtension(string $versi /** @param list $command */ public function runPieCommand(array $command): void { - $pieCommand = array_merge(['php', ...$this->phpArguments, 'bin/pie'], $command); + $pieCommand = array_merge(['php', ...$this->phpArguments, 'bin/pie'], $command, ['--skip-enable-extension']); $proc = (new Process($pieCommand))->mustRun(); diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index b7bd0d6e..7aa377d2 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -64,6 +64,8 @@ public function testReturnsFalseWhenFileIsNotWritable(): void true, true, null, + null, + null, ), $this->mockPhpBinary, $this->output, @@ -103,6 +105,8 @@ public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void true, true, null, + null, + null, ), $this->mockPhpBinary, $this->output, @@ -153,6 +157,8 @@ public function testReturnsFalseWhenExtensionWasAddedButPhpRuntimeDidNotLoadExte true, true, null, + null, + null, ), $this->mockPhpBinary, $this->output, @@ -193,6 +199,8 @@ public function testReturnsTrueWhenExtensionAdded(): void true, true, null, + null, + null, ), $this->mockPhpBinary, $this->output, diff --git a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php index 0e0499f0..ea1576ab 100644 --- a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php +++ b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php @@ -14,6 +14,7 @@ use Php\Pie\Installing\Ini\SetupIniApproach; 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; @@ -28,6 +29,7 @@ private function targetPlatform(): TargetPlatform { return new TargetPlatform( OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, PhpBinaryPath::fromCurrentProcess(), Architecture::x86_64, ThreadSafetyMode::ThreadSafe, @@ -99,6 +101,8 @@ public function testVerboseMessageIsEmittedSettingUpWithoutAnyApproaches(): void true, true, null, + null, + null, ), '/path/to/extracted/source', ), @@ -138,6 +142,8 @@ public function testWorkingApproachIsUsed(): void true, true, null, + null, + null, ), '/path/to/extracted/source', ), @@ -177,6 +183,8 @@ public function testSetupFailsWhenNoApproachesWork(): void true, true, null, + null, + null, ), '/path/to/extracted/source', ), diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php index 7ccf4ef4..ea3a3797 100644 --- a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -15,6 +15,7 @@ use Php\Pie\Installing\Ini\StandardSinglePhpIni; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; +use Php\Pie\Platform\OperatingSystemFamily; use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Php\Pie\Platform\TargetPlatform; @@ -57,6 +58,7 @@ public function setUp(): void $this->targetPlatform = new TargetPlatform( OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, $this->mockPhpBinary, Architecture::x86_64, ThreadSafetyMode::ThreadSafe, @@ -76,6 +78,8 @@ public function setUp(): void true, true, null, + null, + null, ), '/path/to/extracted/source', ); From f34e2a528952195e87c3c0bc32f6793bd9eee8f1 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 12 Dec 2024 08:24:22 +0000 Subject: [PATCH 08/18] Check if ext is already loaded before trying to add to any INI files --- src/Container.php | 1 + .../Ini/PreCheckExtensionAlreadyLoaded.php | 38 ++++++ .../PreCheckExtensionAlreadyLoadedTest.php | 123 ++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php create mode 100644 test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php diff --git a/src/Container.php b/src/Container.php index 2c07ec9a..6f17e5f7 100644 --- a/src/Container.php +++ b/src/Container.php @@ -76,6 +76,7 @@ static function (ContainerInterface $container): Build { Ini\SetupIniApproach::class, static function (ContainerInterface $container): Ini\SetupIniApproach { return new Ini\PickBestSetupIniApproach([ + $container->get(Ini\PreCheckExtensionAlreadyLoaded::class), $container->get(Ini\StandardSinglePhpIni::class), ]); }, diff --git a/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php b/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php new file mode 100644 index 00000000..89f36819 --- /dev/null +++ b/src/Installing/Ini/PreCheckExtensionAlreadyLoaded.php @@ -0,0 +1,38 @@ +phpBinaryPath->assertExtensionIsLoadedInRuntime( + $downloadedPackage->package->extensionName, + $output, + ); + + return true; + } catch (ExtensionIsNotLoaded) { + return false; + } + } +} diff --git a/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php new file mode 100644 index 00000000..ca3a9f2a --- /dev/null +++ b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php @@ -0,0 +1,123 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + null, + null, + ), + '/path/to/extracted/source', + ); + + $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); + + $this->preCheckExtensionAlreadyLoaded = new PreCheckExtensionAlreadyLoaded(); + } + + public function testCanBeUsed(): void + { + self::assertTrue($this->preCheckExtensionAlreadyLoaded->canBeUsed( + $this->targetPlatform, + )); + } + + public function testSetupReturnsTrueWhenExtAlreadyRuntimeLoaded(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output); + + self::assertTrue($this->preCheckExtensionAlreadyLoaded->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } + + public function testSetupReturnsFalseWhenExtIsNotRuntimeLoaded(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output) + ->willThrowException(ExtensionIsNotLoaded::fromExpectedExtension( + $this->mockPhpBinary, + $this->downloadedPackage->package->extensionName, + )); + + self::assertFalse($this->preCheckExtensionAlreadyLoaded->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } +} From 38f2dfd4e8373d32aef863e881d856795c1281d5 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 12 Dec 2024 10:40:18 +0000 Subject: [PATCH 09/18] Added priority property to Package --- src/DependencyResolver/Package.php | 2 ++ test/integration/Building/UnixBuildTest.php | 5 +++++ .../Downloading/GithubPackageReleaseAssetsTest.php | 1 + test/integration/Installing/UnixInstallTest.php | 1 + test/integration/Installing/WindowsInstallTest.php | 1 + test/unit/Command/CommandHelperTest.php | 1 + test/unit/DependencyResolver/PackageTest.php | 1 + test/unit/Downloading/DownloadedPackageTest.php | 2 ++ .../Downloading/Exception/CouldNotFindReleaseAssetTest.php | 2 ++ test/unit/Downloading/GithubPackageReleaseAssetsTest.php | 3 +++ test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php | 4 ++++ test/unit/Installing/Ini/PickBestSetupIniApproachTest.php | 3 +++ .../Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php | 1 + test/unit/Installing/Ini/StandardSinglePhpIniTest.php | 1 + test/unit/Platform/WindowsExtensionAssetNameTest.php | 1 + 15 files changed, 29 insertions(+) diff --git a/src/DependencyResolver/Package.php b/src/DependencyResolver/Package.php index b4d3f7c7..d09a8a0d 100644 --- a/src/DependencyResolver/Package.php +++ b/src/DependencyResolver/Package.php @@ -47,6 +47,7 @@ public function __construct( public readonly string|null $buildPath, public readonly array|null $compatibleOsFamilies, public readonly array|null $incompatibleOsFamilies, + public readonly int $priority, ) { } @@ -93,6 +94,7 @@ public static function fromComposerCompletePackage(CompletePackageInterface $com $buildPath, self::convertInputStringsToOperatingSystemFamilies($compatibleOsFamilies), self::convertInputStringsToOperatingSystemFamilies($incompatibleOsFamilies), + $phpExtOptions['priority'] ?? 80, ); } diff --git a/test/integration/Building/UnixBuildTest.php b/test/integration/Building/UnixBuildTest.php index e8e8c61c..976ca7a0 100644 --- a/test/integration/Building/UnixBuildTest.php +++ b/test/integration/Building/UnixBuildTest.php @@ -50,6 +50,7 @@ public function testUnixBuildCanBuildExtension(): void null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); @@ -105,6 +106,7 @@ public function testUnixBuildWillThrowExceptionWhenExpectedBinaryNameMismatches( null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); @@ -148,6 +150,7 @@ public function testUnixBuildCanBuildExtensionWithBuildPath(): void 'pie_test_ext', null, null, + 99, ), dirname(self::TEST_EXTENSION_PATH), ); @@ -207,6 +210,7 @@ public function testCleanupDoesNotCleanWhenConfigureIsMissing(): void null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); @@ -251,6 +255,7 @@ public function testVerboseOutputShowsCleanupMessages(): void null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); diff --git a/test/integration/Downloading/GithubPackageReleaseAssetsTest.php b/test/integration/Downloading/GithubPackageReleaseAssetsTest.php index df71559c..53cd9b9f 100644 --- a/test/integration/Downloading/GithubPackageReleaseAssetsTest.php +++ b/test/integration/Downloading/GithubPackageReleaseAssetsTest.php @@ -56,6 +56,7 @@ public function testDeterminingReleaseAssetUrlForWindows(): void null, null, null, + 99, ); $io = $this->createMock(IOInterface::class); diff --git a/test/integration/Installing/UnixInstallTest.php b/test/integration/Installing/UnixInstallTest.php index 100f1877..ca28ee0d 100644 --- a/test/integration/Installing/UnixInstallTest.php +++ b/test/integration/Installing/UnixInstallTest.php @@ -95,6 +95,7 @@ public function testUnixInstallCanInstallExtension(string $phpConfig): void null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); diff --git a/test/integration/Installing/WindowsInstallTest.php b/test/integration/Installing/WindowsInstallTest.php index 7192617f..4d043415 100644 --- a/test/integration/Installing/WindowsInstallTest.php +++ b/test/integration/Installing/WindowsInstallTest.php @@ -59,6 +59,7 @@ public function testWindowsInstallCanInstallExtension(): void null, null, null, + 99, ), self::TEST_EXTENSION_PATH, ); diff --git a/test/unit/Command/CommandHelperTest.php b/test/unit/Command/CommandHelperTest.php index 04da3504..34183746 100644 --- a/test/unit/Command/CommandHelperTest.php +++ b/test/unit/Command/CommandHelperTest.php @@ -112,6 +112,7 @@ public function testProcessingConfigureOptionsFromInput(): void null, null, null, + 99, ); $inputDefinition = new InputDefinition(); $inputDefinition->addOption(new InputOption('with-stuff', null, InputOption::VALUE_REQUIRED)); diff --git a/test/unit/DependencyResolver/PackageTest.php b/test/unit/DependencyResolver/PackageTest.php index 79736362..fc99924a 100644 --- a/test/unit/DependencyResolver/PackageTest.php +++ b/test/unit/DependencyResolver/PackageTest.php @@ -140,6 +140,7 @@ public function testGithubOrgAndRepo(string $composerPackageName, string|null $d null, null, null, + 99, ); self::assertSame($expectedGithubOrgAndRepo, $package->githubOrgAndRepository()); diff --git a/test/unit/Downloading/DownloadedPackageTest.php b/test/unit/Downloading/DownloadedPackageTest.php index 8cdc941c..c4edf0b2 100644 --- a/test/unit/Downloading/DownloadedPackageTest.php +++ b/test/unit/Downloading/DownloadedPackageTest.php @@ -35,6 +35,7 @@ public function testFromPackageAndExtractedPath(): void null, null, null, + 99, ); $extractedSourcePath = uniqid('/path/to/downloaded/package', true); @@ -60,6 +61,7 @@ public function testFromPackageAndExtractedPathWithBuildPath(): void 'Downloading', null, null, + 99, ); $extractedSourcePath = realpath(__DIR__ . '/../'); diff --git a/test/unit/Downloading/Exception/CouldNotFindReleaseAssetTest.php b/test/unit/Downloading/Exception/CouldNotFindReleaseAssetTest.php index 3eb2e69a..e25bcecb 100644 --- a/test/unit/Downloading/Exception/CouldNotFindReleaseAssetTest.php +++ b/test/unit/Downloading/Exception/CouldNotFindReleaseAssetTest.php @@ -36,6 +36,7 @@ public function testForPackage(): void null, null, null, + 99, ); $exception = CouldNotFindReleaseAsset::forPackage($package, ['something.zip', 'something2.zip']); @@ -58,6 +59,7 @@ public function testForPackageWithMissingTag(): void null, null, null, + 99, ); $exception = CouldNotFindReleaseAsset::forPackageWithMissingTag($package); diff --git a/test/unit/Downloading/GithubPackageReleaseAssetsTest.php b/test/unit/Downloading/GithubPackageReleaseAssetsTest.php index b618b927..34d8e25a 100644 --- a/test/unit/Downloading/GithubPackageReleaseAssetsTest.php +++ b/test/unit/Downloading/GithubPackageReleaseAssetsTest.php @@ -84,6 +84,7 @@ public function testUrlIsReturnedWhenFindingWindowsDownloadUrl(): void null, null, null, + 99, ); $releaseAssets = new GithubPackageReleaseAssets('https://test-github-api-base-url.thephp.foundation'); @@ -146,6 +147,7 @@ public function testUrlIsReturnedWhenFindingWindowsDownloadUrlWithCompilerAndThr null, null, null, + 99, ); $releaseAssets = new GithubPackageReleaseAssets('https://test-github-api-base-url.thephp.foundation'); @@ -189,6 +191,7 @@ public function testFindWindowsDownloadUrlForPackageThrowsExceptionWhenAssetNotF null, null, null, + 99, ); $releaseAssets = new GithubPackageReleaseAssets('https://test-github-api-base-url.thephp.foundation'); diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index 7aa377d2..58c17eeb 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -66,6 +66,7 @@ public function testReturnsFalseWhenFileIsNotWritable(): void null, null, null, + 99, ), $this->mockPhpBinary, $this->output, @@ -107,6 +108,7 @@ public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void null, null, null, + 99, ), $this->mockPhpBinary, $this->output, @@ -159,6 +161,7 @@ public function testReturnsFalseWhenExtensionWasAddedButPhpRuntimeDidNotLoadExte null, null, null, + 99, ), $this->mockPhpBinary, $this->output, @@ -201,6 +204,7 @@ public function testReturnsTrueWhenExtensionAdded(): void null, null, null, + 99, ), $this->mockPhpBinary, $this->output, diff --git a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php index ea1576ab..07d4bd68 100644 --- a/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php +++ b/test/unit/Installing/Ini/PickBestSetupIniApproachTest.php @@ -103,6 +103,7 @@ public function testVerboseMessageIsEmittedSettingUpWithoutAnyApproaches(): void null, null, null, + 99, ), '/path/to/extracted/source', ), @@ -144,6 +145,7 @@ public function testWorkingApproachIsUsed(): void null, null, null, + 99, ), '/path/to/extracted/source', ), @@ -185,6 +187,7 @@ public function testSetupFailsWhenNoApproachesWork(): void null, null, null, + 99, ), '/path/to/extracted/source', ), diff --git a/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php index ca3a9f2a..de1795f3 100644 --- a/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php +++ b/test/unit/Installing/Ini/PreCheckExtensionAlreadyLoadedTest.php @@ -71,6 +71,7 @@ public function setUp(): void null, null, null, + 99, ), '/path/to/extracted/source', ); diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php index ea3a3797..fb630638 100644 --- a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -80,6 +80,7 @@ public function setUp(): void null, null, null, + 99, ), '/path/to/extracted/source', ); diff --git a/test/unit/Platform/WindowsExtensionAssetNameTest.php b/test/unit/Platform/WindowsExtensionAssetNameTest.php index f152af9b..75e72a6b 100644 --- a/test/unit/Platform/WindowsExtensionAssetNameTest.php +++ b/test/unit/Platform/WindowsExtensionAssetNameTest.php @@ -55,6 +55,7 @@ public function setUp(): void null, null, null, + 99, ); } From 9aacbdcfc250273ba4db29de8c5a72aa8593115c Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 12 Dec 2024 10:57:33 +0000 Subject: [PATCH 10/18] Extract checking and adding ext to a given INI file into a separate component --- .../Ini/CheckAndAddExtensionToIniIfNeeded.php | 70 +++++ src/Installing/Ini/StandardSinglePhpIni.php | 51 +--- .../CheckAndAddExtensionToIniIfNeededTest.php | 250 ++++++++++++++++++ .../Ini/StandardSinglePhpIniTest.php | 161 +---------- 4 files changed, 342 insertions(+), 190 deletions(-) create mode 100644 src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php create mode 100644 test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php diff --git a/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php b/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php new file mode 100644 index 00000000..e14d4401 --- /dev/null +++ b/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php @@ -0,0 +1,70 @@ +writeln( + sprintf( + 'PHP is configured to use %s, but it did not exist, or is not readable by PIE.', + $iniFile, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + if (($this->isExtensionAlreadyInTheIniFile)($iniFile, $downloadedPackage->package->extensionName)) { + $output->writeln( + sprintf( + 'Extension is already enabled in the INI file %s', + $iniFile, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + try { + $targetPlatform->phpBinaryPath->assertExtensionIsLoadedInRuntime($downloadedPackage->package->extensionName, $output); + + return true; + } catch (Throwable $anything) { + $output->writeln(sprintf( + 'Something went wrong verifying the %s extension is enabled: %s', + $downloadedPackage->package->extensionName->name(), + $anything->getMessage(), + )); + + return false; + } + } + + return ($this->addExtensionToTheIniFile)($iniFile, $downloadedPackage->package, $targetPlatform->phpBinaryPath, $output); + } +} diff --git a/src/Installing/Ini/StandardSinglePhpIni.php b/src/Installing/Ini/StandardSinglePhpIni.php index bbec74d9..2f3e8d03 100644 --- a/src/Installing/Ini/StandardSinglePhpIni.php +++ b/src/Installing/Ini/StandardSinglePhpIni.php @@ -8,20 +8,15 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; -use Throwable; use function array_key_exists; -use function file_exists; -use function is_readable; use function preg_match; -use function sprintf; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ final class StandardSinglePhpIni implements SetupIniApproach { public function __construct( - private readonly IsExtensionAlreadyInTheIniFile $isExtensionAlreadyInTheIniFile, - private readonly AddExtensionToTheIniFile $addExtensionToTheIniFile, + private readonly CheckAndAddExtensionToIniIfNeeded $checkAndAddExtensionToIniIfNeeded, ) { } @@ -43,45 +38,15 @@ public function setup( return false; } - if (! file_exists($ini) || ! is_readable($ini)) { - $output->writeln( - sprintf( - 'PHP is configured to use %s, but it did not exist, or is not readable by PIE.', - $ini, - ), - OutputInterface::VERBOSITY_VERBOSE, - ); - - return false; - } - - if (($this->isExtensionAlreadyInTheIniFile)($ini, $downloadedPackage->package->extensionName)) { - $output->writeln( - sprintf( - 'Extension is already enabled in the INI file %s', - $ini, - ), - OutputInterface::VERBOSITY_VERBOSE, - ); - - try { - $targetPlatform->phpBinaryPath->assertExtensionIsLoadedInRuntime($downloadedPackage->package->extensionName, $output); - - return true; - } catch (Throwable $anything) { - $output->writeln(sprintf( - 'Something went wrong verifying the %s extension is enabled: %s', - $downloadedPackage->package->extensionName->name(), - $anything->getMessage(), - )); - - return false; - } - } - - return ($this->addExtensionToTheIniFile)($ini, $downloadedPackage->package, $targetPlatform->phpBinaryPath, $output); + return ($this->checkAndAddExtensionToIniIfNeeded)( + $ini, + $targetPlatform, + $downloadedPackage, + $output, + ); } + /** @return non-empty-string|null */ private function extractPhpIniFromPhpInfo(string $phpinfoString): string|null { if ( diff --git a/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php b/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php new file mode 100644 index 00000000..7f6a949b --- /dev/null +++ b/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php @@ -0,0 +1,250 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->isExtensionAlreadyInTheIniFile = $this->createMock(IsExtensionAlreadyInTheIniFile::class); + $this->addExtensionToTheIniFile = $this->createMock(AddExtensionToTheIniFile::class); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + null, + null, + 66, + ), + '/path/to/extracted/source', + ); + + $this->checkAndAddExtensionToIniIfNeeded = new CheckAndAddExtensionToIniIfNeeded( + $this->isExtensionAlreadyInTheIniFile, + $this->addExtensionToTheIniFile, + ); + } + + public function testReturnsFalseWhenIniFileDoesNotExist(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse($this->checkAndAddExtensionToIniIfNeeded->__invoke( + '/path/to/non/existent/php.ini', + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + )); + + self::assertStringContainsString( + 'PHP is configured to use /path/to/non/existent/php.ini, but it did not exist, or is not readable by PIE.', + $this->output->fetch(), + ); + } + + public function testExtensionIsAlreadyEnabledButExtensionDoesNotLoad(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(true); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output) + ->willThrowException(ExtensionIsNotLoaded::fromExpectedExtension( + $this->mockPhpBinary, + $this->downloadedPackage->package->extensionName, + )); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse($this->checkAndAddExtensionToIniIfNeeded->__invoke( + self::INI_FILE, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + )); + + $output = $this->output->fetch(); + self::assertStringContainsString( + 'Extension is already enabled in the INI file', + $output, + ); + self::assertStringContainsString( + 'Something went wrong verifying the foobar extension is enabled: Expected extension foobar to be loaded in PHP /path/to/php, but it was not detected.', + $output, + ); + } + + public function testExtensionIsAlreadyEnabledAndExtensionLoaded(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(true); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + self::assertTrue($this->checkAndAddExtensionToIniIfNeeded->__invoke( + self::INI_FILE, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + )); + + $output = $this->output->fetch(); + self::assertStringContainsString( + 'Extension is already enabled in the INI file', + $output, + ); + } + + public function testExtensionIsNotYetAdded(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(false); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with( + self::INI_FILE, + $this->downloadedPackage->package, + $this->targetPlatform->phpBinaryPath, + $this->output, + ) + ->willReturn(true); + + self::assertTrue($this->checkAndAddExtensionToIniIfNeeded->__invoke( + self::INI_FILE, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + )); + } + + public function testExtensionIsNotYetAddedButFailsToBeAdded(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(false); + + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + $this->addExtensionToTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with( + self::INI_FILE, + $this->downloadedPackage->package, + $this->targetPlatform->phpBinaryPath, + $this->output, + ) + ->willReturn(false); + + self::assertFalse($this->checkAndAddExtensionToIniIfNeeded->__invoke( + self::INI_FILE, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + )); + } +} diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php index fb630638..2fcc85f0 100644 --- a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -10,13 +10,11 @@ use Php\Pie\Downloading\DownloadedPackage; use Php\Pie\ExtensionName; use Php\Pie\ExtensionType; -use Php\Pie\Installing\Ini\AddExtensionToTheIniFile; -use Php\Pie\Installing\Ini\IsExtensionAlreadyInTheIniFile; +use Php\Pie\Installing\Ini\CheckAndAddExtensionToIniIfNeeded; use Php\Pie\Installing\Ini\StandardSinglePhpIni; use Php\Pie\Platform\Architecture; use Php\Pie\Platform\OperatingSystem; use Php\Pie\Platform\OperatingSystemFamily; -use Php\Pie\Platform\TargetPhp\Exception\ExtensionIsNotLoaded; use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Php\Pie\Platform\TargetPlatform; use Php\Pie\Platform\ThreadSafetyMode; @@ -32,8 +30,7 @@ final class StandardSinglePhpIniTest extends TestCase private BufferedOutput $output; private PhpBinaryPath&MockObject $mockPhpBinary; - private IsExtensionAlreadyInTheIniFile&MockObject $isExtensionAlreadyInTheIniFile; - private AddExtensionToTheIniFile&MockObject $addExtensionToTheIniFile; + private CheckAndAddExtensionToIniIfNeeded&MockObject $checkAndAddExtensionToIniIfNeeded; private TargetPlatform $targetPlatform; private DownloadedPackage $downloadedPackage; private BinaryFile $binaryFile; @@ -53,8 +50,7 @@ public function setUp(): void (fn () => $this->phpBinaryPath = '/path/to/php') ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); - $this->isExtensionAlreadyInTheIniFile = $this->createMock(IsExtensionAlreadyInTheIniFile::class); - $this->addExtensionToTheIniFile = $this->createMock(AddExtensionToTheIniFile::class); + $this->checkAndAddExtensionToIniIfNeeded = $this->createMock(CheckAndAddExtensionToIniIfNeeded::class); $this->targetPlatform = new TargetPlatform( OperatingSystem::NonWindows, @@ -88,8 +84,7 @@ public function setUp(): void $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); $this->standardSinglePhpIni = new StandardSinglePhpIni( - $this->isExtensionAlreadyInTheIniFile, - $this->addExtensionToTheIniFile, + $this->checkAndAddExtensionToIniIfNeeded, ); } @@ -113,76 +108,14 @@ public function testCanBeUsedWithDefinedPhpIni(): void self::assertTrue($this->standardSinglePhpIni->canBeUsed($this->targetPlatform)); } - public function testSetupReturnsWhenCannotBeUsed(): void + public function testSetupReturnsWhenIniFileIsNotSet(): void { $this->mockPhpBinary ->expects(self::once()) ->method('phpinfo') ->willReturn('Loaded Configuration File => (none)'); - self::assertFalse($this->standardSinglePhpIni->setup( - $this->targetPlatform, - $this->downloadedPackage, - $this->binaryFile, - $this->output, - )); - } - - public function testSetupWhenIniFileDoesNotExist(): void - { - $this->mockPhpBinary - ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => /path/to/non/existent/php.ini'); - - $this->isExtensionAlreadyInTheIniFile - ->expects(self::never()) - ->method('__invoke'); - - $this->mockPhpBinary - ->expects(self::never()) - ->method('assertExtensionIsLoadedInRuntime'); - - $this->addExtensionToTheIniFile - ->expects(self::never()) - ->method('__invoke'); - - self::assertFalse($this->standardSinglePhpIni->setup( - $this->targetPlatform, - $this->downloadedPackage, - $this->binaryFile, - $this->output, - )); - - self::assertStringContainsString( - 'PHP is configured to use /path/to/non/existent/php.ini, but it did not exist, or is not readable by PIE.', - $this->output->fetch(), - ); - } - - public function testExtensionIsAlreadyEnabledButExtensionDoesNotLoad(): void - { - $this->mockPhpBinary - ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => ' . self::INI_FILE); - - $this->isExtensionAlreadyInTheIniFile - ->expects(self::once()) - ->method('__invoke') - ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) - ->willReturn(true); - - $this->mockPhpBinary - ->expects(self::once()) - ->method('assertExtensionIsLoadedInRuntime') - ->with($this->downloadedPackage->package->extensionName, $this->output) - ->willThrowException(ExtensionIsNotLoaded::fromExpectedExtension( - $this->mockPhpBinary, - $this->downloadedPackage->package->extensionName, - )); - - $this->addExtensionToTheIniFile + $this->checkAndAddExtensionToIniIfNeeded ->expects(self::never()) ->method('__invoke'); @@ -192,78 +125,22 @@ public function testExtensionIsAlreadyEnabledButExtensionDoesNotLoad(): void $this->binaryFile, $this->output, )); - - $output = $this->output->fetch(); - self::assertStringContainsString( - 'Extension is already enabled in the INI file', - $output, - ); - self::assertStringContainsString( - 'Something went wrong verifying the foobar extension is enabled: Expected extension foobar to be loaded in PHP /path/to/php, but it was not detected.', - $output, - ); - } - - public function testExtensionIsAlreadyEnabledAndExtensionLoaded(): void - { - $this->mockPhpBinary - ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => ' . self::INI_FILE); - - $this->isExtensionAlreadyInTheIniFile - ->expects(self::once()) - ->method('__invoke') - ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) - ->willReturn(true); - - $this->mockPhpBinary - ->expects(self::once()) - ->method('assertExtensionIsLoadedInRuntime') - ->with($this->downloadedPackage->package->extensionName, $this->output); - - $this->addExtensionToTheIniFile - ->expects(self::never()) - ->method('__invoke'); - - self::assertTrue($this->standardSinglePhpIni->setup( - $this->targetPlatform, - $this->downloadedPackage, - $this->binaryFile, - $this->output, - )); - - $output = $this->output->fetch(); - self::assertStringContainsString( - 'Extension is already enabled in the INI file', - $output, - ); } - public function testExtensionIsNotYetAdded(): void + public function testReturnsTrueWhenCheckAndAddExtensionIsInvoked(): void { $this->mockPhpBinary ->expects(self::once()) ->method('phpinfo') ->willReturn('Loaded Configuration File => ' . self::INI_FILE); - $this->isExtensionAlreadyInTheIniFile - ->expects(self::once()) - ->method('__invoke') - ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) - ->willReturn(false); - - $this->mockPhpBinary - ->expects(self::never()) - ->method('assertExtensionIsLoadedInRuntime'); - - $this->addExtensionToTheIniFile + $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) ->method('__invoke') ->with( self::INI_FILE, - $this->downloadedPackage->package, - $this->targetPlatform->phpBinaryPath, + $this->targetPlatform, + $this->downloadedPackage, $this->output, ) ->willReturn(true); @@ -276,30 +153,20 @@ public function testExtensionIsNotYetAdded(): void )); } - public function testExtensionIsNotYetAddedButFailsToBeAdded(): void + public function testReturnsFalseWhenCheckAndAddExtensionIsInvoked(): void { $this->mockPhpBinary ->expects(self::once()) ->method('phpinfo') ->willReturn('Loaded Configuration File => ' . self::INI_FILE); - $this->isExtensionAlreadyInTheIniFile - ->expects(self::once()) - ->method('__invoke') - ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) - ->willReturn(false); - - $this->mockPhpBinary - ->expects(self::never()) - ->method('assertExtensionIsLoadedInRuntime'); - - $this->addExtensionToTheIniFile + $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) ->method('__invoke') ->with( self::INI_FILE, - $this->downloadedPackage->package, - $this->targetPlatform->phpBinaryPath, + $this->targetPlatform, + $this->downloadedPackage, $this->output, ) ->willReturn(false); From d87ca3c1b177d0a13895bb9efe06333fa53d2b09 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 12 Dec 2024 12:28:49 +0000 Subject: [PATCH 11/18] Added processing of the 'additional .ini files' directory to enable extensions --- src/Container.php | 1 + .../Ini/StandardAdditionalPhpIniDirectory.php | 96 +++++++ .../StandardAdditionalPhpIniDirectoryTest.php | 244 ++++++++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 src/Installing/Ini/StandardAdditionalPhpIniDirectory.php create mode 100644 test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php diff --git a/src/Container.php b/src/Container.php index 6f17e5f7..34b0b8eb 100644 --- a/src/Container.php +++ b/src/Container.php @@ -77,6 +77,7 @@ static function (ContainerInterface $container): Build { static function (ContainerInterface $container): Ini\SetupIniApproach { return new Ini\PickBestSetupIniApproach([ $container->get(Ini\PreCheckExtensionAlreadyLoaded::class), + $container->get(Ini\StandardAdditionalPhpIniDirectory::class), $container->get(Ini\StandardSinglePhpIni::class), ]); }, diff --git a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php new file mode 100644 index 00000000..56265729 --- /dev/null +++ b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php @@ -0,0 +1,96 @@ +extractIniDirectoryFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()) !== null; + } + + public function setup( + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + BinaryFile $binaryFile, + OutputInterface $output, + ): bool { + $phpinfo = $targetPlatform->phpBinaryPath->phpinfo(); + + $additionalIniFilesPath = $this->extractIniDirectoryFromPhpInfo($phpinfo); + if ($additionalIniFilesPath === null) { + return false; + } + + $expectedIniFile = sprintf( + '%s%s%d-%s.ini', + rtrim($additionalIniFilesPath, DIRECTORY_SEPARATOR), + DIRECTORY_SEPARATOR, + $downloadedPackage->package->priority, + $downloadedPackage->package->extensionName->name(), + ); + + $pieCreatedTheIniFile = false; + if (! file_exists($expectedIniFile)) { + $output->writeln( + sprintf( + 'Creating new INI file based on extension priority: %s', + $expectedIniFile, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + $pieCreatedTheIniFile = true; + touch($expectedIniFile); + } + + $addingExtensionWasSuccessful = ($this->checkAndAddExtensionToIniIfNeeded)( + $expectedIniFile, + $targetPlatform, + $downloadedPackage, + $output, + ); + + if (! $addingExtensionWasSuccessful && $pieCreatedTheIniFile) { + unlink($expectedIniFile); + } + + return $addingExtensionWasSuccessful; + } + + private function extractIniDirectoryFromPhpInfo(string $phpinfoString): string|null + { + if ( + preg_match('/Scan this dir for additional \.ini files([ =>\t]*)(.*)/', $phpinfoString, $m) + && array_key_exists(2, $m) + && $m[2] !== '' + && $m[2] !== '(none)' + ) { + return $m[2]; + } + + return null; + } +} diff --git a/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php new file mode 100644 index 00000000..d069a26c --- /dev/null +++ b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php @@ -0,0 +1,244 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->checkAndAddExtensionToIniIfNeeded = $this->createMock(CheckAndAddExtensionToIniIfNeeded::class); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + null, + null, + 99, + ), + '/path/to/extracted/source', + ); + + $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); + + $this->standardAdditionalPhpIniDirectory = new StandardAdditionalPhpIniDirectory( + $this->checkAndAddExtensionToIniIfNeeded, + ); + } + + public function testCannotBeUsedWithNoDefinedAdditionalPhpIniDirectory(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => (none)'); + + self::assertFalse($this->standardAdditionalPhpIniDirectory->canBeUsed($this->targetPlatform)); + } + + public function testCanBeUsedWithDefinedAdditionalPhpIniDirectory(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => /path/to/the/php.d'); + + self::assertTrue($this->standardAdditionalPhpIniDirectory->canBeUsed($this->targetPlatform)); + } + + public function testSetupReturnsWhenAdditionalPhpIniDirectoryIsNotSet(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => (none)'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse($this->standardAdditionalPhpIniDirectory->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + } + + public function testReturnsTrueWhenCheckAndAddExtensionIsInvoked(): void + { + $additionalPhpIniDirectory = tempnam(sys_get_temp_dir(), 'pie_additional_php_ini_path'); + unlink($additionalPhpIniDirectory); + mkdir($additionalPhpIniDirectory, recursive: true); + + $expectedIniFile = $additionalPhpIniDirectory . DIRECTORY_SEPARATOR . '99-foobar.ini'; + + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + ) + ->willReturn(true); + + self::assertTrue($this->standardAdditionalPhpIniDirectory->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + self::assertFileExists($expectedIniFile); + + unlink($expectedIniFile); + rmdir($additionalPhpIniDirectory); + } + + public function testReturnsFalseAndRemovesPieCreatedIniFileWhenCheckAndAddExtensionIsInvoked(): void + { + $additionalPhpIniDirectory = tempnam(sys_get_temp_dir(), 'pie_additional_php_ini_path'); + unlink($additionalPhpIniDirectory); + mkdir($additionalPhpIniDirectory, recursive: true); + + $expectedIniFile = $additionalPhpIniDirectory . DIRECTORY_SEPARATOR . '99-foobar.ini'; + + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + ) + ->willReturn(false); + + self::assertFalse($this->standardAdditionalPhpIniDirectory->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + self::assertFileDoesNotExist($expectedIniFile); + + rmdir($additionalPhpIniDirectory); + } + + public function testReturnsFalseAndLeavesNonPieCreatedIniFileWhenCheckAndAddExtensionIsInvoked(): void + { + $additionalPhpIniDirectory = tempnam(sys_get_temp_dir(), 'pie_additional_php_ini_path'); + unlink($additionalPhpIniDirectory); + mkdir($additionalPhpIniDirectory, recursive: true); + + $expectedIniFile = $additionalPhpIniDirectory . DIRECTORY_SEPARATOR . '99-foobar.ini'; + touch($expectedIniFile); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('phpinfo') + ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + ) + ->willReturn(false); + + self::assertFalse($this->standardAdditionalPhpIniDirectory->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + )); + self::assertFileExists($expectedIniFile); + + unlink($expectedIniFile); + rmdir($additionalPhpIniDirectory); + } +} From f331578f1ffb1f79dfd56d64337ec631011f7ea1 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Thu, 12 Dec 2024 12:46:49 +0000 Subject: [PATCH 12/18] Ensure AddExtensionToTheIniFile tests are Windows friendly --- .../unit/Installing/Ini/AddExtensionToTheIniFileTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index 58c17eeb..5da906e9 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -13,6 +13,7 @@ use Php\Pie\Platform\TargetPhp\PhpBinaryPath; use Php\Pie\Platform\TargetPlatform; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\RequiresOperatingSystemFamily; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Output\BufferedOutput; @@ -26,6 +27,8 @@ use function touch; use function unlink; +use const PHP_EOL; + #[CoversClass(AddExtensionToTheIniFile::class)] final class AddExtensionToTheIniFileTest extends TestCase { @@ -82,6 +85,7 @@ public function testReturnsFalseWhenFileIsNotWritable(): void } } + #[RequiresOperatingSystemFamily('Linux')] public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void { if (TargetPlatform::isRunningAsRoot()) { @@ -211,7 +215,10 @@ public function testReturnsTrueWhenExtensionAdded(): void )); $iniContent = file_get_contents($iniFile); - self::assertSame("\n; PIE automatically added this to enable the foo/bar extension\nextension=foobar\n", $iniContent); + self::assertSame( + PHP_EOL . '; PIE automatically added this to enable the foo/bar extension' . PHP_EOL . 'extension=foobar' . PHP_EOL, + $iniContent, + ); self::assertStringContainsString( sprintf('Enabled extension foobar in the INI file %s', $iniFile), From 294a529d774f9ca12aaf4991d2d2361d4f7ddb32 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 17 Dec 2024 20:17:42 +0000 Subject: [PATCH 13/18] Added docker-php-ext-enable INI method --- src/Container.php | 1 + src/Installing/Ini/DockerPhpExtEnable.php | 89 +++++++++ test/assets/docker-php-ext-enable/bad | 4 + test/assets/docker-php-ext-enable/good | 4 + .../Installing/Ini/DockerPhpExtEnableTest.php | 176 ++++++++++++++++++ 5 files changed, 274 insertions(+) create mode 100644 src/Installing/Ini/DockerPhpExtEnable.php create mode 100755 test/assets/docker-php-ext-enable/bad create mode 100755 test/assets/docker-php-ext-enable/good create mode 100644 test/unit/Installing/Ini/DockerPhpExtEnableTest.php diff --git a/src/Container.php b/src/Container.php index 34b0b8eb..a2b5adf6 100644 --- a/src/Container.php +++ b/src/Container.php @@ -77,6 +77,7 @@ static function (ContainerInterface $container): Build { static function (ContainerInterface $container): Ini\SetupIniApproach { return new Ini\PickBestSetupIniApproach([ $container->get(Ini\PreCheckExtensionAlreadyLoaded::class), + $container->get(Ini\DockerPhpExtEnable::class), $container->get(Ini\StandardAdditionalPhpIniDirectory::class), $container->get(Ini\StandardSinglePhpIni::class), ]); diff --git a/src/Installing/Ini/DockerPhpExtEnable.php b/src/Installing/Ini/DockerPhpExtEnable.php new file mode 100644 index 00000000..2878af4d --- /dev/null +++ b/src/Installing/Ini/DockerPhpExtEnable.php @@ -0,0 +1,89 @@ +dockerPhpExtEnablePath() !== null; + } + + public function setup( + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + BinaryFile $binaryFile, + OutputInterface $output, + ): bool { + $dockerPhpExtEnable = $this->dockerPhpExtEnablePath(); + + if ($dockerPhpExtEnable === null) { + return false; + } + + try { + $enableOutput = Process::run([$dockerPhpExtEnable, $downloadedPackage->package->extensionName->name()]); + } catch (ProcessFailedException $processFailed) { + $output->writeln( + sprintf( + 'Could not enable extension %s using %s. Exception was: %s', + $downloadedPackage->package->extensionName->name(), + $this->dockerPhpExtEnableName, + $processFailed->getMessage(), + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + try { + $targetPlatform->phpBinaryPath->assertExtensionIsLoadedInRuntime( + $downloadedPackage->package->extensionName, + $output, + ); + + return true; + } catch (ExtensionIsNotLoaded) { + $output->writeln( + sprintf( + 'Asserting that extension %s was enabled using %s failed. Output was: %s', + $downloadedPackage->package->extensionName->name(), + $this->dockerPhpExtEnableName, + $enableOutput !== '' ? $enableOutput : '(empty)', + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + } + + private function dockerPhpExtEnablePath(): string|null + { + try { + return Process::run(['which', $this->dockerPhpExtEnableName]); + } catch (ProcessFailedException) { + return null; + } + } +} diff --git a/test/assets/docker-php-ext-enable/bad b/test/assets/docker-php-ext-enable/bad new file mode 100755 index 00000000..33514cfc --- /dev/null +++ b/test/assets/docker-php-ext-enable/bad @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +echo "something bad happened" +exit 1 diff --git a/test/assets/docker-php-ext-enable/good b/test/assets/docker-php-ext-enable/good new file mode 100755 index 00000000..5e2e73e3 --- /dev/null +++ b/test/assets/docker-php-ext-enable/good @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +echo "hi" +exit 0 diff --git a/test/unit/Installing/Ini/DockerPhpExtEnableTest.php b/test/unit/Installing/Ini/DockerPhpExtEnableTest.php new file mode 100644 index 00000000..410190a0 --- /dev/null +++ b/test/unit/Installing/Ini/DockerPhpExtEnableTest.php @@ -0,0 +1,176 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + null, + null, + 99, + ), + '/path/to/extracted/source', + ); + + $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); + } + + public function testCannotBeUsedWhenDockerPhpExtEnableIsNotInPath(): void + { + self::assertFalse( + (new DockerPhpExtEnable(self::NON_EXISTENT_DOCKER_PHP_EXT_ENABLE)) + ->canBeUsed($this->targetPlatform), + ); + } + + public function testCanBeUsedWhenDockerPhpExtEnableIsInPath(): void + { + self::assertTrue( + (new DockerPhpExtEnable(self::GOOD_DOCKER_PHP_EXT_ENABLE)) + ->canBeUsed($this->targetPlatform), + ); + } + + public function testSetupReturnsFalseWhenWhenDockerPhpExtEnableIsNotInPath(): void + { + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + self::assertFalse( + (new DockerPhpExtEnable(self::NON_EXISTENT_DOCKER_PHP_EXT_ENABLE)) + ->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } + + public function testReturnsTrueWhenDockerPhpExtEnableSuccessfullyEnablesExtension(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output); + + self::assertTrue( + (new DockerPhpExtEnable(self::GOOD_DOCKER_PHP_EXT_ENABLE)) + ->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } + + public function testReturnsFalseWhenDockerPhpExtEnableFailsToBeRun(): void + { + $this->mockPhpBinary + ->expects(self::never()) + ->method('assertExtensionIsLoadedInRuntime'); + + self::assertFalse( + (new DockerPhpExtEnable(self::BAD_DOCKER_PHP_EXT_ENABLE)) + ->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } + + public function testReturnsFalseWhenDockerPhpExtEnableFailsToAssertExtensionWasEnabled(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output) + ->willThrowException(ExtensionIsNotLoaded::fromExpectedExtension( + $this->mockPhpBinary, + $this->downloadedPackage->package->extensionName, + )); + + self::assertFalse( + (new DockerPhpExtEnable(self::GOOD_DOCKER_PHP_EXT_ENABLE)) + ->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } +} From e4a56e3040e2c7d6e9cfa520823c98375fa654d1 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Tue, 17 Dec 2024 21:51:23 +0000 Subject: [PATCH 14/18] Extract ini file and directory functions into PhpBinaryPath --- .../Ini/StandardAdditionalPhpIniDirectory.php | 22 ++------------ src/Installing/Ini/StandardSinglePhpIni.php | 22 ++------------ src/Platform/TargetPhp/PhpBinaryPath.php | 30 +++++++++++++++++++ .../StandardAdditionalPhpIniDirectoryTest.php | 24 +++++++-------- .../Ini/StandardSinglePhpIniTest.php | 20 ++++++------- 5 files changed, 57 insertions(+), 61 deletions(-) diff --git a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php index 56265729..060f1d69 100644 --- a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php +++ b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php @@ -9,9 +9,7 @@ use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; -use function array_key_exists; use function file_exists; -use function preg_match; use function rtrim; use function sprintf; use function touch; @@ -29,7 +27,7 @@ public function __construct( public function canBeUsed(TargetPlatform $targetPlatform): bool { - return $this->extractIniDirectoryFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()) !== null; + return $targetPlatform->phpBinaryPath->additionalIniDirectory() !== null; } public function setup( @@ -38,9 +36,9 @@ public function setup( BinaryFile $binaryFile, OutputInterface $output, ): bool { - $phpinfo = $targetPlatform->phpBinaryPath->phpinfo(); + $additionalIniFilesPath = $targetPlatform->phpBinaryPath->additionalIniDirectory(); - $additionalIniFilesPath = $this->extractIniDirectoryFromPhpInfo($phpinfo); + /** In practice, this shouldn't happen since {@see canBeUsed()} checks this */ if ($additionalIniFilesPath === null) { return false; } @@ -79,18 +77,4 @@ public function setup( return $addingExtensionWasSuccessful; } - - private function extractIniDirectoryFromPhpInfo(string $phpinfoString): string|null - { - if ( - preg_match('/Scan this dir for additional \.ini files([ =>\t]*)(.*)/', $phpinfoString, $m) - && array_key_exists(2, $m) - && $m[2] !== '' - && $m[2] !== '(none)' - ) { - return $m[2]; - } - - return null; - } } diff --git a/src/Installing/Ini/StandardSinglePhpIni.php b/src/Installing/Ini/StandardSinglePhpIni.php index 2f3e8d03..67602c6a 100644 --- a/src/Installing/Ini/StandardSinglePhpIni.php +++ b/src/Installing/Ini/StandardSinglePhpIni.php @@ -9,9 +9,6 @@ use Php\Pie\Platform\TargetPlatform; use Symfony\Component\Console\Output\OutputInterface; -use function array_key_exists; -use function preg_match; - /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ final class StandardSinglePhpIni implements SetupIniApproach { @@ -22,7 +19,7 @@ public function __construct( public function canBeUsed(TargetPlatform $targetPlatform): bool { - return $this->extractPhpIniFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()) !== null; + return $targetPlatform->phpBinaryPath->loadedIniConfigurationFile() !== null; } public function setup( @@ -31,7 +28,7 @@ public function setup( BinaryFile $binaryFile, OutputInterface $output, ): bool { - $ini = $this->extractPhpIniFromPhpInfo($targetPlatform->phpBinaryPath->phpinfo()); + $ini = $targetPlatform->phpBinaryPath->loadedIniConfigurationFile(); /** In practice, this shouldn't happen since {@see canBeUsed()} checks this */ if ($ini === null) { @@ -45,19 +42,4 @@ public function setup( $output, ); } - - /** @return non-empty-string|null */ - private function extractPhpIniFromPhpInfo(string $phpinfoString): string|null - { - if ( - preg_match('/Loaded Configuration File([ =>\t]*)(.*)/', $phpinfoString, $m) - && array_key_exists(2, $m) - && $m[2] !== '' - && $m[2] !== '(none)' - ) { - return $m[2]; - } - - return null; - } } diff --git a/src/Platform/TargetPhp/PhpBinaryPath.php b/src/Platform/TargetPhp/PhpBinaryPath.php index 63c133e6..f421b65a 100644 --- a/src/Platform/TargetPhp/PhpBinaryPath.php +++ b/src/Platform/TargetPhp/PhpBinaryPath.php @@ -138,6 +138,36 @@ public function assertExtensionIsLoadedInRuntime(ExtensionName $extension, Outpu ); } + /** @return non-empty-string|null */ + public function additionalIniDirectory(): string|null + { + if ( + preg_match('/Scan this dir for additional \.ini files([ =>\t]*)(.*)/', $this->phpinfo(), $m) + && array_key_exists(2, $m) + && $m[2] !== '' + && $m[2] !== '(none)' + ) { + return $m[2]; + } + + return null; + } + + /** @return non-empty-string|null */ + public function loadedIniConfigurationFile(): string|null + { + if ( + preg_match('/Loaded Configuration File([ =>\t]*)(.*)/', $this->phpinfo(), $m) + && array_key_exists(2, $m) + && $m[2] !== '' + && $m[2] !== '(none)' + ) { + return $m[2]; + } + + return null; + } + /** * Returns a map where the key is the name of the extension and the value is the version ('0' if not defined) * diff --git a/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php index d069a26c..404a1d04 100644 --- a/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php +++ b/test/unit/Installing/Ini/StandardAdditionalPhpIniDirectoryTest.php @@ -99,8 +99,8 @@ public function testCannotBeUsedWithNoDefinedAdditionalPhpIniDirectory(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => (none)'); + ->method('additionalIniDirectory') + ->willReturn(null); self::assertFalse($this->standardAdditionalPhpIniDirectory->canBeUsed($this->targetPlatform)); } @@ -109,8 +109,8 @@ public function testCanBeUsedWithDefinedAdditionalPhpIniDirectory(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => /path/to/the/php.d'); + ->method('additionalIniDirectory') + ->willReturn('/path/to/the/php.d'); self::assertTrue($this->standardAdditionalPhpIniDirectory->canBeUsed($this->targetPlatform)); } @@ -119,8 +119,8 @@ public function testSetupReturnsWhenAdditionalPhpIniDirectoryIsNotSet(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => (none)'); + ->method('additionalIniDirectory') + ->willReturn(null); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::never()) @@ -144,8 +144,8 @@ public function testReturnsTrueWhenCheckAndAddExtensionIsInvoked(): void $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + ->method('additionalIniDirectory') + ->willReturn($additionalPhpIniDirectory); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) @@ -180,8 +180,8 @@ public function testReturnsFalseAndRemovesPieCreatedIniFileWhenCheckAndAddExtens $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + ->method('additionalIniDirectory') + ->willReturn($additionalPhpIniDirectory); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) @@ -216,8 +216,8 @@ public function testReturnsFalseAndLeavesNonPieCreatedIniFileWhenCheckAndAddExte $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Scan this dir for additional .ini files => ' . $additionalPhpIniDirectory); + ->method('additionalIniDirectory') + ->willReturn($additionalPhpIniDirectory); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) diff --git a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php index 2fcc85f0..34b19daa 100644 --- a/test/unit/Installing/Ini/StandardSinglePhpIniTest.php +++ b/test/unit/Installing/Ini/StandardSinglePhpIniTest.php @@ -92,8 +92,8 @@ public function testCannotBeUsedWithNoDefinedPhpIni(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => (none)'); + ->method('loadedIniConfigurationFile') + ->willReturn(null); self::assertFalse($this->standardSinglePhpIni->canBeUsed($this->targetPlatform)); } @@ -102,8 +102,8 @@ public function testCanBeUsedWithDefinedPhpIni(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => /path/to/php.ini'); + ->method('loadedIniConfigurationFile') + ->willReturn('/path/to/php.ini'); self::assertTrue($this->standardSinglePhpIni->canBeUsed($this->targetPlatform)); } @@ -112,8 +112,8 @@ public function testSetupReturnsWhenIniFileIsNotSet(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => (none)'); + ->method('loadedIniConfigurationFile') + ->willReturn(null); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::never()) @@ -131,8 +131,8 @@ public function testReturnsTrueWhenCheckAndAddExtensionIsInvoked(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + ->method('loadedIniConfigurationFile') + ->willReturn(self::INI_FILE); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) @@ -157,8 +157,8 @@ public function testReturnsFalseWhenCheckAndAddExtensionIsInvoked(): void { $this->mockPhpBinary ->expects(self::once()) - ->method('phpinfo') - ->willReturn('Loaded Configuration File => ' . self::INI_FILE); + ->method('loadedIniConfigurationFile') + ->willReturn(self::INI_FILE); $this->checkAndAddExtensionToIniIfNeeded ->expects(self::once()) From acf9e28cf0e8ab4e2638cbee57e3ace3fb75964f Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 18 Dec 2024 10:41:12 +0000 Subject: [PATCH 15/18] Added priority comment into generated INI content --- src/Installing/Ini/AddExtensionToTheIniFile.php | 17 ++++++++++++----- .../Ini/AddExtensionToTheIniFileTest.php | 4 +++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Installing/Ini/AddExtensionToTheIniFile.php b/src/Installing/Ini/AddExtensionToTheIniFile.php index 7c0264d6..b6d80c2d 100644 --- a/src/Installing/Ini/AddExtensionToTheIniFile.php +++ b/src/Installing/Ini/AddExtensionToTheIniFile.php @@ -52,11 +52,7 @@ public function __invoke(string $ini, Package $package, PhpBinaryPath $phpBinary try { file_put_contents( $ini, - $originalIniContent . PHP_EOL - . '; PIE automatically added this to enable the ' . $package->name . ' extension' . PHP_EOL - . ($package->extensionType === ExtensionType::PhpModule ? 'extension' : 'zend_extension') - . '=' - . $package->extensionName->name() . PHP_EOL, + $originalIniContent . $this->iniFileContent($package), ); $output->writeln( sprintf( @@ -82,4 +78,15 @@ public function __invoke(string $ini, Package $package, PhpBinaryPath $phpBinary return false; } } + + /** @return non-empty-string */ + private function iniFileContent(Package $package): string + { + return PHP_EOL + . '; PIE automatically added this to enable the ' . $package->name . ' extension' . PHP_EOL + . '; priority=' . $package->priority . PHP_EOL + . ($package->extensionType === ExtensionType::PhpModule ? 'extension' : 'zend_extension') + . '=' + . $package->extensionName->name() . PHP_EOL; + } } diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index 5da906e9..22824b10 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -216,7 +216,9 @@ public function testReturnsTrueWhenExtensionAdded(): void $iniContent = file_get_contents($iniFile); self::assertSame( - PHP_EOL . '; PIE automatically added this to enable the foo/bar extension' . PHP_EOL . 'extension=foobar' . PHP_EOL, + PHP_EOL . '; PIE automatically added this to enable the foo/bar extension' . PHP_EOL + . '; priority=99' . PHP_EOL + . 'extension=foobar' . PHP_EOL, $iniContent, ); From 18274409dbb61cde0dde4505323ad4c5a485b383 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 18 Dec 2024 11:06:11 +0000 Subject: [PATCH 16/18] Add ability for additional step before checking exts are enabled --- .../Ini/AddExtensionToTheIniFile.php | 14 ++++- .../Ini/CheckAndAddExtensionToIniIfNeeded.php | 18 +++++- .../Ini/StandardAdditionalPhpIniDirectory.php | 1 + src/Installing/Ini/StandardSinglePhpIni.php | 1 + .../Ini/AddExtensionToTheIniFileTest.php | 60 +++++++++++++++++++ .../CheckAndAddExtensionToIniIfNeededTest.php | 44 ++++++++++++++ 6 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/Installing/Ini/AddExtensionToTheIniFile.php b/src/Installing/Ini/AddExtensionToTheIniFile.php index b6d80c2d..73469e9e 100644 --- a/src/Installing/Ini/AddExtensionToTheIniFile.php +++ b/src/Installing/Ini/AddExtensionToTheIniFile.php @@ -21,8 +21,14 @@ /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ class AddExtensionToTheIniFile { - public function __invoke(string $ini, Package $package, PhpBinaryPath $phpBinaryPath, OutputInterface $output): bool - { + /** @param callable():bool|null $additionalEnableStep */ + public function __invoke( + string $ini, + Package $package, + PhpBinaryPath $phpBinaryPath, + OutputInterface $output, + callable|null $additionalEnableStep, + ): bool { if (! is_writable($ini)) { $output->writeln( sprintf( @@ -63,6 +69,10 @@ public function __invoke(string $ini, Package $package, PhpBinaryPath $phpBinary OutputInterface::VERBOSITY_VERBOSE, ); + if ($additionalEnableStep !== null && ! $additionalEnableStep()) { + return false; + } + $phpBinaryPath->assertExtensionIsLoadedInRuntime($package->extensionName, $output); return true; diff --git a/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php b/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php index e14d4401..eec39db0 100644 --- a/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php +++ b/src/Installing/Ini/CheckAndAddExtensionToIniIfNeeded.php @@ -22,12 +22,16 @@ public function __construct( ) { } - /** @param non-empty-string $iniFile */ + /** + * @param non-empty-string $iniFile + * @param callable():bool|null $additionalEnableStep + */ public function __invoke( string $iniFile, TargetPlatform $targetPlatform, DownloadedPackage $downloadedPackage, OutputInterface $output, + callable|null $additionalEnableStep, ): bool { if (! file_exists($iniFile) || ! is_readable($iniFile)) { $output->writeln( @@ -50,6 +54,10 @@ public function __invoke( OutputInterface::VERBOSITY_VERBOSE, ); + if ($additionalEnableStep !== null && ! $additionalEnableStep()) { + return false; + } + try { $targetPlatform->phpBinaryPath->assertExtensionIsLoadedInRuntime($downloadedPackage->package->extensionName, $output); @@ -65,6 +73,12 @@ public function __invoke( } } - return ($this->addExtensionToTheIniFile)($iniFile, $downloadedPackage->package, $targetPlatform->phpBinaryPath, $output); + return ($this->addExtensionToTheIniFile)( + $iniFile, + $downloadedPackage->package, + $targetPlatform->phpBinaryPath, + $output, + $additionalEnableStep, + ); } } diff --git a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php index 060f1d69..a627b902 100644 --- a/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php +++ b/src/Installing/Ini/StandardAdditionalPhpIniDirectory.php @@ -69,6 +69,7 @@ public function setup( $targetPlatform, $downloadedPackage, $output, + null, ); if (! $addingExtensionWasSuccessful && $pieCreatedTheIniFile) { diff --git a/src/Installing/Ini/StandardSinglePhpIni.php b/src/Installing/Ini/StandardSinglePhpIni.php index 67602c6a..0eb046e8 100644 --- a/src/Installing/Ini/StandardSinglePhpIni.php +++ b/src/Installing/Ini/StandardSinglePhpIni.php @@ -40,6 +40,7 @@ public function setup( $targetPlatform, $downloadedPackage, $output, + null, ); } } diff --git a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php index 22824b10..3f6f3f2e 100644 --- a/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php +++ b/test/unit/Installing/Ini/AddExtensionToTheIniFileTest.php @@ -73,6 +73,7 @@ public function testReturnsFalseWhenFileIsNotWritable(): void ), $this->mockPhpBinary, $this->output, + null, )); self::assertStringContainsString( @@ -116,6 +117,7 @@ public function testReturnsFalseWhenExistingIniCouldNotBeRead(): void ), $this->mockPhpBinary, $this->output, + null, )); self::assertStringContainsString( @@ -169,6 +171,7 @@ public function testReturnsFalseWhenExtensionWasAddedButPhpRuntimeDidNotLoadExte ), $this->mockPhpBinary, $this->output, + null, )); self::assertStringContainsString( @@ -212,6 +215,7 @@ public function testReturnsTrueWhenExtensionAdded(): void ), $this->mockPhpBinary, $this->output, + null, )); $iniContent = file_get_contents($iniFile); @@ -230,4 +234,60 @@ public function testReturnsTrueWhenExtensionAdded(): void unlink($iniFile); } } + + public function testReturnsTrueWhenExtensionAddedWithAdditionalStep(): void + { + $iniFile = tempnam(sys_get_temp_dir(), 'PIE_ini_file'); + touch($iniFile); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime'); + + try { + $additionalStepInvoked = false; + self::assertTrue((new AddExtensionToTheIniFile())( + $iniFile, + new Package( + $this->createMock(CompletePackage::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.0.0', + null, + [], + true, + true, + null, + null, + null, + 99, + ), + $this->mockPhpBinary, + $this->output, + static function () use (&$additionalStepInvoked): bool { + $additionalStepInvoked = true; + + return true; + }, + )); + + self::assertTrue($additionalStepInvoked, 'Failed asserting that the additional step was invoked'); + + $iniContent = file_get_contents($iniFile); + self::assertSame( + PHP_EOL . '; PIE automatically added this to enable the foo/bar extension' . PHP_EOL + . '; priority=99' . PHP_EOL + . 'extension=foobar' . PHP_EOL, + $iniContent, + ); + + self::assertStringContainsString( + sprintf('Enabled extension foobar in the INI file %s', $iniFile), + $this->output->fetch(), + ); + } finally { + unlink($iniFile); + } + } } diff --git a/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php b/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php index 7f6a949b..ac31ecc2 100644 --- a/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php +++ b/test/unit/Installing/Ini/CheckAndAddExtensionToIniIfNeededTest.php @@ -108,6 +108,7 @@ public function testReturnsFalseWhenIniFileDoesNotExist(): void $this->targetPlatform, $this->downloadedPackage, $this->output, + null, )); self::assertStringContainsString( @@ -142,6 +143,7 @@ public function testExtensionIsAlreadyEnabledButExtensionDoesNotLoad(): void $this->targetPlatform, $this->downloadedPackage, $this->output, + null, )); $output = $this->output->fetch(); @@ -177,6 +179,7 @@ public function testExtensionIsAlreadyEnabledAndExtensionLoaded(): void $this->targetPlatform, $this->downloadedPackage, $this->output, + null, )); $output = $this->output->fetch(); @@ -186,6 +189,45 @@ public function testExtensionIsAlreadyEnabledAndExtensionLoaded(): void ); } + public function testExtensionIsAlreadyEnabledWithAdditionalStepAndExtensionLoaded(): void + { + $this->isExtensionAlreadyInTheIniFile + ->expects(self::once()) + ->method('__invoke') + ->with(self::INI_FILE, $this->downloadedPackage->package->extensionName) + ->willReturn(true); + + $this->mockPhpBinary + ->expects(self::once()) + ->method('assertExtensionIsLoadedInRuntime') + ->with($this->downloadedPackage->package->extensionName, $this->output); + + $this->addExtensionToTheIniFile + ->expects(self::never()) + ->method('__invoke'); + + $additionalStepInvoked = false; + self::assertTrue($this->checkAndAddExtensionToIniIfNeeded->__invoke( + self::INI_FILE, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + static function () use (&$additionalStepInvoked): bool { + $additionalStepInvoked = true; + + return true; + }, + )); + + self::assertTrue($additionalStepInvoked, 'Failed asserting that the additional step was invoked'); + + $output = $this->output->fetch(); + self::assertStringContainsString( + 'Extension is already enabled in the INI file', + $output, + ); + } + public function testExtensionIsNotYetAdded(): void { $this->isExtensionAlreadyInTheIniFile @@ -214,6 +256,7 @@ public function testExtensionIsNotYetAdded(): void $this->targetPlatform, $this->downloadedPackage, $this->output, + null, )); } @@ -245,6 +288,7 @@ public function testExtensionIsNotYetAddedButFailsToBeAdded(): void $this->targetPlatform, $this->downloadedPackage, $this->output, + null, )); } } From 5329ebe46dc0e1359c17565c3d233ae75f9cafe6 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Fri, 20 Dec 2024 10:53:17 +0000 Subject: [PATCH 17/18] Added INI enable approach for phpenmod --- src/Container.php | 1 + src/Installing/Ini/OndrejPhpenmod.php | 194 ++++++++ test/assets/phpenmod/bad | 4 + test/assets/phpenmod/good | 4 + .../Installing/Ini/OndrejPhpenmodTest.php | 465 ++++++++++++++++++ 5 files changed, 668 insertions(+) create mode 100644 src/Installing/Ini/OndrejPhpenmod.php create mode 100755 test/assets/phpenmod/bad create mode 100755 test/assets/phpenmod/good create mode 100644 test/unit/Installing/Ini/OndrejPhpenmodTest.php diff --git a/src/Container.php b/src/Container.php index a2b5adf6..1c181a9a 100644 --- a/src/Container.php +++ b/src/Container.php @@ -77,6 +77,7 @@ static function (ContainerInterface $container): Build { static function (ContainerInterface $container): Ini\SetupIniApproach { return new Ini\PickBestSetupIniApproach([ $container->get(Ini\PreCheckExtensionAlreadyLoaded::class), + $container->get(Ini\OndrejPhpenmod::class), $container->get(Ini\DockerPhpExtEnable::class), $container->get(Ini\StandardAdditionalPhpIniDirectory::class), $container->get(Ini\StandardSinglePhpIni::class), diff --git a/src/Installing/Ini/OndrejPhpenmod.php b/src/Installing/Ini/OndrejPhpenmod.php new file mode 100644 index 00000000..051dfb65 --- /dev/null +++ b/src/Installing/Ini/OndrejPhpenmod.php @@ -0,0 +1,194 @@ +phpenmodPath() !== null; + } + + public function setup( + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + BinaryFile $binaryFile, + OutputInterface $output, + ): bool { + $phpenmodPath = $this->phpenmodPath(); + + /** In practice, this shouldn't happen since {@see canBeUsed()} checks this */ + if ($phpenmodPath === null) { + return false; + } + + // the Ondrej repo uses an additional php.ini directory, if this isn't set, we may not actually be using Ondrej repo for this particular PHP install + $additionalPhpIniPath = $targetPlatform->phpBinaryPath->additionalIniDirectory(); + + if ($additionalPhpIniPath === null) { + $output->writeln( + 'Additional INI file path was not set - may not be Ondrej PHP repo', + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + // Cursory check for the expected PHP INI directory; this is another indication we're using the Ondrej repo + if (preg_match('#/etc/php/\d\.\d/[a-z-_]+/conf.d#', $additionalPhpIniPath) !== 1) { + $output->writeln( + sprintf( + 'Warning: additional INI file path was not in the expected format (/etc/php/{version}/{sapi}/conf.d). Path was: %s', + $additionalPhpIniPath, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + } + + $expectedModsAvailablePath = sprintf($this->modsAvailablePath, $targetPlatform->phpBinaryPath->majorMinorVersion()); + + if (! file_exists($expectedModsAvailablePath)) { + $output->writeln( + sprintf( + 'Mods available path %s does not exist', + $expectedModsAvailablePath, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + if (! is_dir($expectedModsAvailablePath)) { + $output->writeln( + sprintf( + 'Mods available path %s is not a directory', + $expectedModsAvailablePath, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + if (! is_writable($expectedModsAvailablePath)) { + $output->writeln( + sprintf( + 'Mods available path %s is not writable', + $expectedModsAvailablePath, + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + + $expectedIniFile = sprintf( + '%s%s%s.ini', + rtrim($expectedModsAvailablePath, DIRECTORY_SEPARATOR), + DIRECTORY_SEPARATOR, + $downloadedPackage->package->extensionName->name(), + ); + + $pieCreatedTheIniFile = false; + if (! file_exists($expectedIniFile)) { + $output->writeln( + sprintf( + 'Creating new INI file based on extension priority: %s', + $expectedIniFile, + ), + OutputInterface::VERBOSITY_VERY_VERBOSE, + ); + $pieCreatedTheIniFile = true; + touch($expectedIniFile); + } + + $addingExtensionWasSuccessful = ($this->checkAndAddExtensionToIniIfNeeded)( + $expectedIniFile, + $targetPlatform, + $downloadedPackage, + $output, + static function () use ($phpenmodPath, $targetPlatform, $downloadedPackage, $output): bool { + try { + Process::run([ + $phpenmodPath, + '-v', + $targetPlatform->phpBinaryPath->majorMinorVersion(), + '-s', + 'ALL', + $downloadedPackage->package->extensionName->name(), + ]); + + return true; + } catch (ProcessFailedException $processFailedException) { + $output->writeln( + sprintf( + 'Failed to use %s to enable %s for PHP %s: %s', + $phpenmodPath, + $downloadedPackage->package->extensionName->name(), + $targetPlatform->phpBinaryPath->majorMinorVersion(), + $processFailedException->getMessage(), + ), + OutputInterface::VERBOSITY_VERBOSE, + ); + + return false; + } + }, + ); + + if (! $addingExtensionWasSuccessful && $pieCreatedTheIniFile) { + unlink($expectedIniFile); + } + + return $addingExtensionWasSuccessful; + } + + /** @return non-empty-string|null */ + private function phpenmodPath(): string|null + { + if (Platform::isWindows()) { + return null; + } + + try { + $phpenmodPath = Process::run(['which', $this->phpenmod]); + + return $phpenmodPath !== '' ? $phpenmodPath : null; + } catch (ProcessFailedException) { + return null; + } + } +} diff --git a/test/assets/phpenmod/bad b/test/assets/phpenmod/bad new file mode 100755 index 00000000..33514cfc --- /dev/null +++ b/test/assets/phpenmod/bad @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +echo "something bad happened" +exit 1 diff --git a/test/assets/phpenmod/good b/test/assets/phpenmod/good new file mode 100755 index 00000000..5e2e73e3 --- /dev/null +++ b/test/assets/phpenmod/good @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +echo "hi" +exit 0 diff --git a/test/unit/Installing/Ini/OndrejPhpenmodTest.php b/test/unit/Installing/Ini/OndrejPhpenmodTest.php new file mode 100644 index 00000000..4fcecb4e --- /dev/null +++ b/test/unit/Installing/Ini/OndrejPhpenmodTest.php @@ -0,0 +1,465 @@ +output = new BufferedOutput(BufferedOutput::VERBOSITY_VERBOSE); + + $this->mockPhpBinary = $this->createMock(PhpBinaryPath::class); + /** + * @psalm-suppress PossiblyNullFunctionCall + * @psalm-suppress UndefinedThisPropertyAssignment + */ + (fn () => $this->phpBinaryPath = '/path/to/php') + ->bindTo($this->mockPhpBinary, PhpBinaryPath::class)(); + + $this->checkAndAddExtensionToIniIfNeeded = $this->createMock(CheckAndAddExtensionToIniIfNeeded::class); + + $this->targetPlatform = new TargetPlatform( + OperatingSystem::NonWindows, + OperatingSystemFamily::Linux, + $this->mockPhpBinary, + Architecture::x86_64, + ThreadSafetyMode::ThreadSafe, + 1, + null, + ); + + $this->downloadedPackage = DownloadedPackage::fromPackageAndExtractedPath( + new Package( + $this->createMock(CompletePackageInterface::class), + ExtensionType::PhpModule, + ExtensionName::normaliseFromString('foobar'), + 'foo/bar', + '1.2.3', + null, + [], + true, + true, + null, + null, + null, + 99, + ), + '/path/to/extracted/source', + ); + + $this->binaryFile = new BinaryFile('/path/to/compiled/extension.so', 'fake checksum'); + } + + #[RequiresOperatingSystemFamily('Windows')] + public function testCanBeUsedReturnsFalseOnWindows(): void + { + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->canBeUsed($this->targetPlatform), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testCanBeUsedReturnsFalseWhenPhpenmodNotInPath(): void + { + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::NON_EXISTENT_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->canBeUsed($this->targetPlatform), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testCanBeUsedReturnsTrueWhenPhpenmodInPath(): void + { + self::assertTrue( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->canBeUsed($this->targetPlatform), + ); + } + + #[RequiresOperatingSystemFamily('Windows')] + public function testSetupReturnsFalseOnWindows(): void + { + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseWhenPhpenmodNotInPath(): void + { + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::NON_EXISTENT_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseWhenAdditionalPhpIniPathNotSet(): void + { + $this->mockPhpBinary + ->expects(self::once()) + ->method('additionalIniDirectory') + ->willReturn(null); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertStringContainsString( + 'Additional INI file path was not set - may not be Ondrej PHP repo', + $this->output->fetch(), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseWhenModsAvailablePathDoesNotExist(): void + { + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + self::NON_EXISTENT_MODS_AVAILABLE_PATH, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertStringContainsString( + 'Mods available path ' . self::NON_EXISTENT_MODS_AVAILABLE_PATH . ' does not exist', + $this->output->fetch(), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseWhenModsAvailablePathNotADirectory(): void + { + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + __FILE__, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertStringContainsString( + 'Mods available path ' . __FILE__ . ' is not a directory', + $this->output->fetch(), + ); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseWhenModsAvailablePathNotWritable(): void + { + if (TargetPlatform::isRunningAsRoot()) { + self::markTestSkipped('Test cannot be run as root, as root can always write files'); + } + + $modsAvailablePath = tempnam(sys_get_temp_dir(), 'pie_test_mods_available_path'); + unlink($modsAvailablePath); + mkdir($modsAvailablePath, 000, true); + + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::never()) + ->method('__invoke'); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + $modsAvailablePath, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertStringContainsString( + 'Mods available path ' . $modsAvailablePath . ' is not writable', + $this->output->fetch(), + ); + + rmdir($modsAvailablePath); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseAndRemovesPieCreatedIniFileWhenPhpenmodAdditionalStepFails(): void + { + $modsAvailablePath = tempnam(sys_get_temp_dir(), 'pie_test_mods_available_path'); + unlink($modsAvailablePath); + mkdir($modsAvailablePath, recursive: true); + + $expectedIniFile = $modsAvailablePath . DIRECTORY_SEPARATOR . 'foobar.ini'; + + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + self::isType(IsType::TYPE_CALLABLE), + ) + ->willReturnCallback( + /** @param callable():bool $additionalEnableStep */ + static function ( + string $iniFile, + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + OutputInterface $output, + callable $additionalEnableStep, + ): bool { + return $additionalEnableStep(); + }, + ); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::BAD_PHPENMOD, + $modsAvailablePath, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertFileDoesNotExist($expectedIniFile); + + self::assertStringContainsString( + 'something bad happened', + $this->output->fetch(), + ); + + rmdir($modsAvailablePath); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsFalseAndRemovesPieCreatedIniFileWhenCheckAndAddExtensionFails(): void + { + $modsAvailablePath = tempnam(sys_get_temp_dir(), 'pie_test_mods_available_path'); + unlink($modsAvailablePath); + mkdir($modsAvailablePath, recursive: true); + + $expectedIniFile = $modsAvailablePath . DIRECTORY_SEPARATOR . 'foobar.ini'; + + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + self::isType(IsType::TYPE_CALLABLE), + ) + ->willReturn(false); + + self::assertFalse( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + $modsAvailablePath, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertFileDoesNotExist($expectedIniFile); + + rmdir($modsAvailablePath); + } + + #[RequiresOperatingSystemFamily('Linux')] + public function testSetupReturnsTrueWhenExtensionIsEnabled(): void + { + $modsAvailablePath = tempnam(sys_get_temp_dir(), 'pie_test_mods_available_path'); + unlink($modsAvailablePath); + mkdir($modsAvailablePath, recursive: true); + + $expectedIniFile = $modsAvailablePath . DIRECTORY_SEPARATOR . 'foobar.ini'; + + $this->mockPhpBinary + ->method('additionalIniDirectory') + ->willReturn('/value/does/not/matter'); + + $this->checkAndAddExtensionToIniIfNeeded + ->expects(self::once()) + ->method('__invoke') + ->with( + $expectedIniFile, + $this->targetPlatform, + $this->downloadedPackage, + $this->output, + self::isType(IsType::TYPE_CALLABLE), + ) + ->willReturnCallback( + /** @param callable():bool $additionalEnableStep */ + static function ( + string $iniFile, + TargetPlatform $targetPlatform, + DownloadedPackage $downloadedPackage, + OutputInterface $output, + callable $additionalEnableStep, + ): bool { + return $additionalEnableStep(); + }, + ); + + self::assertTrue( + (new OndrejPhpenmod( + $this->checkAndAddExtensionToIniIfNeeded, + self::GOOD_PHPENMOD, + $modsAvailablePath, + ))->setup( + $this->targetPlatform, + $this->downloadedPackage, + $this->binaryFile, + $this->output, + ), + ); + + self::assertFileExists($expectedIniFile); + + unlink($expectedIniFile); + rmdir($modsAvailablePath); + } +} From 8887791b16348d4d152a2865c26a12e996184bad Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 23 Dec 2024 09:27:59 +0000 Subject: [PATCH 18/18] Validate extension really is enabled in PHP --- test/behaviour/CliContext.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/behaviour/CliContext.php b/test/behaviour/CliContext.php index b1b3bff4..801dbe9b 100644 --- a/test/behaviour/CliContext.php +++ b/test/behaviour/CliContext.php @@ -17,6 +17,7 @@ /** @psalm-api */ class CliContext implements Context { + private const PHP_BINARY = 'php'; private string|null $output = null; private int|null $exitCode = null; /** @var list */ @@ -37,7 +38,7 @@ public function iRunACommandToDownloadSpecificVersionOfAnExtension(string $versi /** @param list $command */ public function runPieCommand(array $command): void { - $pieCommand = array_merge(['php', ...$this->phpArguments, 'bin/pie'], $command, ['--skip-enable-extension']); + $pieCommand = array_merge([self::PHP_BINARY, ...$this->phpArguments, 'bin/pie'], $command); $proc = (new Process($pieCommand))->mustRun(); @@ -123,6 +124,8 @@ public function theExtensionShouldHaveBeenInstalled(): void { $this->assertCommandSuccessful(); + Assert::contains($this->output, 'Extension is enabled and loaded'); + if (Platform::isWindows()) { Assert::regex($this->output, '#Copied DLL to: [-\\\_:.a-zA-Z0-9]+\\\php_example_pie_extension.dll#'); @@ -130,6 +133,12 @@ public function theExtensionShouldHaveBeenInstalled(): void } Assert::regex($this->output, '#Install complete: [-_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('yes', $isExtEnabled); } #[Given('I have an invalid extension installed')]