Merge pull request #203 from asgrim/fix-replace-conflict

Fix replace conflict bug

Author: asgrim

src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php

@@ -4,23 +4,29 @@
 
 namespace Php\Pie\ComposerIntegration;
 
+use Composer\Composer;
 use Composer\Package\CompletePackage;
 use Composer\Pcre\Preg;
 use Composer\Repository\PlatformRepository;
 use Composer\Semver\VersionParser;
 use Php\Pie\ExtensionName;
+use Php\Pie\Platform\InstalledPiePackages;
 use Php\Pie\Platform\TargetPhp\PhpBinaryPath;
 use UnexpectedValueException;
 
+use function in_array;
 use function str_replace;
+use function str_starts_with;
+use function strlen;
 use function strtolower;
+use function substr;
 
 /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
 class PhpBinaryPathBasedPlatformRepository extends PlatformRepository
 {
     private VersionParser $versionParser;
 
-    public function __construct(PhpBinaryPath $phpBinaryPath, ExtensionName|null $extensionBeingInstalled)
+    public function __construct(PhpBinaryPath $phpBinaryPath, Composer $composer, InstalledPiePackages $installedPiePackages, ExtensionName|null $extensionBeingInstalled)
     {
         $this->versionParser = new VersionParser();
         $this->packages      = [];
@@ -32,6 +38,22 @@ public function __construct(PhpBinaryPath $phpBinaryPath, ExtensionName|null $ex
 
         $extVersions = $phpBinaryPath->extensions();
 
+        $piePackages                          = $installedPiePackages->allPiePackages($composer);
+        $extensionsBeingReplacedByPiePackages = [];
+        foreach ($piePackages as $piePackage) {
+            foreach ($piePackage->composerPackage()->getReplaces() as $replaceLink) {
+                $target = $replaceLink->getTarget();
+                if (
+                    ! str_starts_with($target, 'ext-')
+                    || ! ExtensionName::isValidExtensionName(substr($target, strlen('ext-')))
+                ) {
+                    continue;
+                }
+
+                $extensionsBeingReplacedByPiePackages[] = ExtensionName::normaliseFromString($replaceLink->getTarget())->name();
+            }
+        }
+
         foreach ($extVersions as $extension => $extensionVersion) {
             /**
              * If the extension we're trying to exclude is not excluded from this list if it is already installed
@@ -43,6 +65,15 @@ public function __construct(PhpBinaryPath $phpBinaryPath, ExtensionName|null $ex
                 continue;
             }
 
+            /**
+             * If any extensions present have `replaces`, we need to remove them otherwise it conflicts too
+             *
+             * @link https://github.com/php/pie/issues/161
+             */
+            if (in_array($extension, $extensionsBeingReplacedByPiePackages)) {
+                continue;
+            }
+
             $this->addPackage($this->packageForExtension($extension, $extensionVersion));
         }
 

src/ComposerIntegration/PieComposerInstaller.php

@@ -9,6 +9,7 @@
 use Composer\IO\IOInterface;
 use Composer\Repository\PlatformRepository;
 use Php\Pie\ExtensionName;
+use Php\Pie\Platform\InstalledPiePackages;
 use Php\Pie\Platform\TargetPhp\PhpBinaryPath;
 use Webmozart\Assert\Assert;
 
@@ -21,13 +22,15 @@ class PieComposerInstaller extends Installer
 {
     private PhpBinaryPath|null $phpBinaryPath           = null;
     private ExtensionName|null $extensionBeingInstalled = null;
+    private Composer|null $composer                     = null;
 
     protected function createPlatformRepo(bool $forUpdate): PlatformRepository
     {
         Assert::notNull($this->phpBinaryPath, '$phpBinaryPath was not set, maybe createWithPhpBinary was not used?');
         Assert::notNull($this->extensionBeingInstalled, '$extensionBeingInstalled was not set, maybe createWithPhpBinary was not used?');
+        Assert::notNull($this->composer, '$composer was not set, maybe createWithPhpBinary was not used?');
 
-        return new PhpBinaryPathBasedPlatformRepository($this->phpBinaryPath, $this->extensionBeingInstalled);
+        return new PhpBinaryPathBasedPlatformRepository($this->phpBinaryPath, $this->composer, new InstalledPiePackages(), $this->extensionBeingInstalled);
     }
 
     public static function createWithPhpBinary(
@@ -51,6 +54,7 @@ public static function createWithPhpBinary(
 
         $composerInstaller->phpBinaryPath           = $php;
         $composerInstaller->extensionBeingInstalled = $extensionBeingInstalled;
+        $composerInstaller->composer                = $composer;
 
         return $composerInstaller;
     }

src/ComposerIntegration/VersionSelectorFactory.php

@@ -10,6 +10,7 @@
 use Composer\Repository\RepositorySet;
 use Php\Pie\DependencyResolver\DetermineMinimumStability;
 use Php\Pie\DependencyResolver\RequestedPackageAndVersion;
+use Php\Pie\Platform\InstalledPiePackages;
 use Php\Pie\Platform\TargetPlatform;
 
 /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
@@ -35,7 +36,7 @@ public static function make(
     ): VersionSelector {
         return new VersionSelector(
             self::factoryRepositorySet($composer, $requestedPackageAndVersion->version),
-            new PhpBinaryPathBasedPlatformRepository($targetPlatform->phpBinaryPath, null),
+            new PhpBinaryPathBasedPlatformRepository($targetPlatform->phpBinaryPath, $composer, new InstalledPiePackages(), null),
         );
     }
 }

src/ExtensionName.php

@@ -5,12 +5,14 @@
 namespace Php\Pie;
 
 use Composer\Package\PackageInterface;
+use InvalidArgumentException;
 use Webmozart\Assert\Assert;
 
 use function array_key_exists;
-use function assert;
 use function explode;
 use function is_string;
+use function preg_match;
+use function sprintf;
 use function str_starts_with;
 use function strlen;
 use function substr;
@@ -37,17 +39,22 @@ final class ExtensionName
 
     private function __construct(string $normalisedExtensionName)
     {
-        Assert::regex(
-            $normalisedExtensionName,
-            self::VALID_PACKAGE_NAME_REGEX,
-            'The value %s is not a valid extension name. An extension must start with a letter, and only'
-            . ' contain alphanumeric characters or underscores',
-        );
-        assert($normalisedExtensionName !== '');
+        if (! self::isValidExtensionName($normalisedExtensionName)) {
+            throw new InvalidArgumentException(sprintf(
+                'The value "%s" is not a valid extension name. An extension must start with a letter, and only contain alphanumeric characters or underscores',
+                $normalisedExtensionName,
+            ));
+        }
 
         $this->normalisedExtensionName = $normalisedExtensionName;
     }
 
+    /** @psalm-assert-if-true non-empty-string $extensionName */
+    public static function isValidExtensionName(string $extensionName): bool
+    {
+        return preg_match(self::VALID_PACKAGE_NAME_REGEX, $extensionName) >= 1;
+    }
+
     public static function determineFromComposerPackage(PackageInterface $package): self
     {
         $phpExt = $package->getPhpExt();

test/unit/ComposerIntegration/PhpBinaryPathBasedPlatformRepositoryTest.php

@@ -4,9 +4,15 @@
 
 namespace Php\PieUnitTest\ComposerIntegration;
 
+use Composer\Composer;
+use Composer\Package\CompletePackage;
+use Composer\Package\Link;
 use Composer\Package\PackageInterface;
+use Composer\Semver\Constraint\Constraint;
 use Php\Pie\ComposerIntegration\PhpBinaryPathBasedPlatformRepository;
+use Php\Pie\DependencyResolver\Package;
 use Php\Pie\ExtensionName;
+use Php\Pie\Platform\InstalledPiePackages;
 use Php\Pie\Platform\TargetPhp\PhpBinaryPath;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
@@ -18,6 +24,11 @@ final class PhpBinaryPathBasedPlatformRepositoryTest extends TestCase
 {
     public function testPlatformRepositoryContainsExpectedPacakges(): void
     {
+        $composer = $this->createMock(Composer::class);
+
+        $installedPiePackages = $this->createMock(InstalledPiePackages::class);
+        $installedPiePackages->method('allPiePackages')->willReturn([]);
+
         $phpBinaryPath = $this->createMock(PhpBinaryPath::class);
         $phpBinaryPath->expects(self::once())
             ->method('version')
@@ -31,7 +42,7 @@ public function testPlatformRepositoryContainsExpectedPacakges(): void
                 'another' => '1.2.3-alpha.34',
             ]);
 
-        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, null);
+        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, $composer, $installedPiePackages, null);
 
         self::assertSame(
             [
@@ -50,6 +61,11 @@ public function testPlatformRepositoryContainsExpectedPacakges(): void
 
     public function testPlatformRepositoryExcludesExtensionBeingInstalled(): void
     {
+        $composer = $this->createMock(Composer::class);
+
+        $installedPiePackages = $this->createMock(InstalledPiePackages::class);
+        $installedPiePackages->method('allPiePackages')->willReturn([]);
+
         $extensionBeingInstalled = ExtensionName::normaliseFromString('extension_being_installed');
 
         $phpBinaryPath = $this->createMock(PhpBinaryPath::class);
@@ -63,7 +79,47 @@ public function testPlatformRepositoryExcludesExtensionBeingInstalled(): void
                 'extension_being_installed' => '1.2.3',
             ]);
 
-        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, $extensionBeingInstalled);
+        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, $composer, $installedPiePackages, $extensionBeingInstalled);
+
+        self::assertSame(
+            [
+                'php:8.1.0',
+                'ext-foo:8.1.0',
+            ],
+            array_map(
+                static fn (PackageInterface $package): string => $package->getName() . ':' . $package->getPrettyVersion(),
+                $platformRepository->getPackages(),
+            ),
+        );
+    }
+
+    public function testPlatformRepositoryExcludesReplacedExtensions(): void
+    {
+        $composer = $this->createMock(Composer::class);
+
+        $composerPackage = new CompletePackage('myvendor/replaced_extension', '1.2.3.0', '1.2.3');
+        $composerPackage->setReplaces([
+            'ext-replaced_extension' => new Link('myvendor/replaced_extension', 'ext-replaced_extension', new Constraint('==', '*')),
+        ]);
+        $installedPiePackages = $this->createMock(InstalledPiePackages::class);
+        $installedPiePackages->method('allPiePackages')->willReturn([
+            Package::fromComposerCompletePackage($composerPackage),
+        ]);
+
+        $extensionBeingInstalled = ExtensionName::normaliseFromString('extension_being_installed');
+
+        $phpBinaryPath = $this->createMock(PhpBinaryPath::class);
+        $phpBinaryPath->expects(self::once())
+            ->method('version')
+            ->willReturn('8.1.0');
+        $phpBinaryPath->expects(self::once())
+            ->method('extensions')
+            ->willReturn([
+                'foo' => '8.1.0',
+                'replaced_extension' => '3.0.0',
+            ]);
+
+        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, $composer, $installedPiePackages, $extensionBeingInstalled);
 
         self::assertSame(
             [

test/unit/ComposerIntegration/VersionSelectorFactoryTest.php

@@ -6,8 +6,11 @@
 
 use Composer\Composer;
 use Composer\Package\CompletePackage;
+use Composer\Package\Link;
 use Composer\Repository\ArrayRepository;
+use Composer\Repository\InstalledRepositoryInterface;
 use Composer\Repository\RepositoryManager;
+use Composer\Semver\Constraint\Constraint;
 use Php\Pie\ComposerIntegration\VersionSelectorFactory;
 use Php\Pie\DependencyResolver\RequestedPackageAndVersion;
 use Php\Pie\Platform\Architecture;
@@ -30,15 +33,25 @@ public function testVersionSelectorFactory(): void
             new CompletePackage('foo/bar', '2.0.0.0', '2.0.0'),
         ]);
 
+        $packageWithReplaces = new CompletePackage('already/installed2', '1.2.3.0', '1.2.3');
+        $packageWithReplaces->setReplaces([
+            'ext-installed2' => new Link('root/package', 'ext-installed2', new Constraint('==', '*')),
+        ]);
+        $localRepository = $this->createMock(InstalledRepositoryInterface::class);
+        $localRepository->method('getPackages')->willReturn([
+            new CompletePackage('already/installed1', '1.2.3.0', '1.2.3'),
+            $packageWithReplaces,
+        ]);
+
         $repoMananger = $this->createMock(RepositoryManager::class);
         $repoMananger
             ->expects(self::once())
             ->method('getRepositories')
             ->willReturn([$repository]);
+        $repoMananger->method('getLocalRepository')->willReturn($localRepository);
 
         $composer = $this->createMock(Composer::class);
         $composer
-            ->expects(self::once())
             ->method('getRepositoryManager')
             ->willReturn($repoMananger);