Repository: php/pie (php)

Issue:
Package conflict with installed extension where `replaces` is used
I think this follows on from #150 - my test extension `asgrim/example-pie-extension` uses `replace`, and this is causing issues.

**Steps to reproduce:**

 - Install the example ext (`bin/pie install -v asgrim/example-pie-extension`)
 - Then try to install a different, unrelated extension, e.g. (`bin/pie install -v xdebug/xdebug`)
 
**Expected results:**

`xdebug` should be installed

**Actual results:**

```
You are running PHP 8.3.14
Target PHP installation: 8.3.14 ts, on Linux/OSX/etc x86_64 (from /home/james/workspace/oss/php-src-build/bin/php)
Found package: xdebug/xdebug:3.4.0 which provides ext-xdebug
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - ext-example_pie_extension is present at version 0.1.0 and cannot be modified by Composer
    - asgrim/example-pie-extension is locked to version 2.0.2 and an update of this package was not requested.
    - Only one of these can be installed: asgrim/example-pie-extension[2.0.2], ext-example_pie_extension[0.1.0]. asgrim/example-pie-extension replaces ext-example_pie_extension and thus cannot coexist with it.
```

**Observations:**

 - The dependency was correctly resolved in `\Php\Pie\DependencyResolver\ResolveDependencyWithComposer`; so the issue comes from the Composer Installer `\Php\Pie\ComposerIntegration\ComposerIntegrationHandler`
 - The installed/reported extension version is `0.1.0` (I never updated the [`PHP_EXAMPLE_PIE_EXTENSION_VERSION` constant](https://github.com/asgrim/example-pie-extension/blob/f9ed13ea95dada34c6cc5a052da258dbda059d27/php_example_pie_extension.h#L9C10-L9C43) to be the actual version) but I don't think this is related; even if they were the same version I think it is just the fact that asgrim/example-pie-extension `replaces` ext-example-pie-extension, but both are present, so Composer has a little meltdown from it
 - The fix I had done in #150 was to exclude the ext being installed from the reported list of extensions for the `\Php\Pie\ComposerIntegration\PhpBinaryPathBasedPlatformRepository`, but I don't think this is the right approach. I think perhaps something like: work out if a Composer package (asgrim/example-pie-extension) does a `replaces` for an extension (example_pie_extension) and exclude all of those when building the platform in `PhpBinaryPathBasedPlatformRepository`
 - I think this behaviour is strictly related to using `replaces` in `composer.json`, which checking into [the Packagist extensions list](https://packagist.org/extensions) shows that there are only two extensions using this at the moment ([asgrim/example-pie-extension](https://packagist.org/packages/asgrim/example-pie-extension), [solidworx/aspect](https://packagist.org/packages/solidworx/aspect)), so it is not (yet) a widespread problem at the moment.

Pull request:
Fix replace conflict bug
Fixes #161 

Patch:
diff --git a/src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php b/src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php
index fae9d17..1a4085d 100644
--- a/src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php
+++ b/src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php
@@ -4,23 +4,29 @@ declare(strict_types=1);
 
 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 @@ class PhpBinaryPathBasedPlatformRepository extends PlatformRepository
 
         $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 @@ class PhpBinaryPathBasedPlatformRepository extends PlatformRepository
                 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));
         }
 
diff --git a/src/ComposerIntegration/PieComposerInstaller.php b/src/ComposerIntegration/PieComposerInstaller.php
index 57febc0..1334e18 100644
--- a/src/ComposerIntegration/PieComposerInstaller.php
+++ b/src/ComposerIntegration/PieComposerInstaller.php
@@ -9,6 +9,7 @@ use Composer\Installer;
 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 @@ class PieComposerInstaller extends Installer
 
         $composerInstaller->phpBinaryPath           = $php;
         $composerInstaller->extensionBeingInstalled = $extensionBeingInstalled;
+        $composerInstaller->composer                = $composer;
 
         return $composerInstaller;
     }
diff --git a/src/ComposerIntegration/VersionSelectorFactory.php b/src/ComposerIntegration/VersionSelectorFactory.php
index ef81ef6..5d9c56a 100644
--- a/src/ComposerIntegration/VersionSelectorFactory.php
+++ b/src/ComposerIntegration/VersionSelectorFactory.php
@@ -10,6 +10,7 @@ use Composer\Repository\CompositeRepository;
 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 @@ final class VersionSelectorFactory
     ): VersionSelector {
         return new VersionSelector(
             self::factoryRepositorySet($composer, $requestedPackageAndVersion->version),
-            new PhpBinaryPathBasedPlatformRepository($targetPlatform->phpBinaryPath, null),
+            new PhpBinaryPathBasedPlatformRepository($targetPlatform->phpBinaryPath, $composer, new InstalledPiePackages(), null),
         );
     }
 }
diff --git a/src/ExtensionName.php b/src/ExtensionName.php
index afe876b..5d58d92 100644
--- a/src/ExtensionName.php
+++ b/src/ExtensionName.php
@@ -5,12 +5,14 @@ declare(strict_types=1);
 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 patch:
diff --git a/test/unit/ComposerIntegration/PhpBinaryPathBasedPlatformRepositoryTest.php b/test/unit/ComposerIntegration/PhpBinaryPathBasedPlatformRepositoryTest.php
index 1fc7c65..1bc77d9 100644
--- a/test/unit/ComposerIntegration/PhpBinaryPathBasedPlatformRepositoryTest.php
+++ b/test/unit/ComposerIntegration/PhpBinaryPathBasedPlatformRepositoryTest.php
@@ -4,9 +4,15 @@ declare(strict_types=1);
 
 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 @@ final class PhpBinaryPathBasedPlatformRepositoryTest extends TestCase
                 'another' => '1.2.3-alpha.34',
             ]);
 
-        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, null);
+        $platformRepository = new PhpBinaryPathBasedPlatformRepository($phpBinaryPath, $composer, $installedPiePackages, null);
 
         self::assertSame(
             [
@@ -50,6 +61,11 @@ final class PhpBinaryPathBasedPlatformRepositoryTest extends TestCase
 
     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 @@ final class PhpBinaryPathBasedPlatformRepositoryTest extends TestCase
                 '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(
             [
diff --git a/test/unit/ComposerIntegration/VersionSelectorFactoryTest.php b/test/unit/ComposerIntegration/VersionSelectorFactoryTest.php
index 56a34e4..4069e6c 100644
--- a/test/unit/ComposerIntegration/VersionSelectorFactoryTest.php
+++ b/test/unit/ComposerIntegration/VersionSelectorFactoryTest.php
@@ -6,8 +6,11 @@ namespace Php\PieUnitTest\ComposerIntegration;
 
 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 @@ final class VersionSelectorFactoryTest extends TestCase
             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);
 
diff --git a/test/unit/DependencyResolver/ResolveDependencyWithComposerTest.php b/test/unit/DependencyResolver/ResolveDependencyWithComposerTest.php
index 9e9f828..e6b472d 100644
--- a/test/unit/DependencyResolver/ResolveDependencyWithComposerTest.php
+++ b/test/unit/DependencyResolver/ResolveDependencyWithComposerTest.php
@@ -7,10 +7,13 @@ namespace Php\PieUnitTest\DependencyResolver;
 use Composer\Composer;
 use Composer\IO\NullIO;
 use Composer\Package\CompletePackage;
+use Composer\Package\Link;
 use Composer\Repository\ArrayRepository;
 use Composer\Repository\CompositeRepository;
+use Composer\Repository\InstalledRepositoryInterface;
 use Composer\Repository\RepositoryFactory;
 use Composer\Repository\RepositoryManager;
+use Composer\Semver\Constraint\Constraint;
 use Php\Pie\ComposerIntegration\QuieterConsoleIO;
 use Php\Pie\DependencyResolver\IncompatibleOperatingSystemFamily;
 use Php\Pie\DependencyResolver\IncompatibleThreadSafetyMode;
@@ -25,6 +28,7 @@ use Php\Pie\Platform\TargetPlatform;
 use Php\Pie\Platform\ThreadSafetyMode;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 #[CoversClass(ResolveDependencyWithComposer::class)]
@@ -32,13 +36,26 @@ final class ResolveDependencyWithComposerTest extends TestCase
 {
     private Composer $composer;
 
+    private InstalledRepositoryInterface&MockObject $localRepo;
+
     public function setUp(): void
     {
         parent::setUp();
 
+        $packageWithReplaces = new CompletePackage('already/installed2', '1.2.3.0', '1.2.3');
+        $packageWithReplaces->setReplaces([
+            'ext-installed2' => new Link('root/package', 'ext-installed2', new Constraint('==', '*')),
+        ]);
+        $this->localRepo = $this->createMock(InstalledRepositoryInterface::class);
+        $this->localRepo->method('getPackages')->willReturn([
+            new CompletePackage('already/installed1', '1.2.3.0', '1.2.3'),
+            $packageWithReplaces,
+        ]);
+
         $repoManager = $this->createMock(RepositoryManager::class);
         $repoManager->method('getRepositories')
             ->willReturn([new CompositeRepository(RepositoryFactory::defaultReposWithDefaultManager(new NullIO()))]);
+        $repoManager->method('getLocalRepository')->willReturn($this->localRepo);
 
         $this->composer = $this->createMock(Composer::class);
         $this->composer->method('getRepositoryManager')
@@ -175,6 +192,7 @@ final class ResolveDependencyWithComposerTest extends TestCase
         $repoManager = $this->createMock(RepositoryManager::class);
         $repoManager->method('getRepositories')
             ->willReturn([new ArrayRepository([$pkg])]);
+        $repoManager->method('getLocalRepository')->willReturn($this->localRepo);
 
         $this->composer = $this->createMock(Composer::class);
         $this->composer->method('getRepositoryManager')
@@ -222,6 +240,7 @@ final class ResolveDependencyWithComposerTest extends TestCase
         $repoManager = $this->createMock(RepositoryManager::class);
         $repoManager->method('getRepositories')
             ->willReturn([new ArrayRepository([$pkg])]);
+        $repoManager->method('getLocalRepository')->willReturn($this->localRepo);
 
         $this->composer = $this->createMock(Composer::class);
         $this->composer->method('getRepositoryManager')
@@ -269,6 +288,7 @@ final class ResolveDependencyWithComposerTest extends TestCase
         $repoManager = $this->createMock(RepositoryManager::class);
         $repoManager->method('getRepositories')
             ->willReturn([new ArrayRepository([$pkg])]);
+        $repoManager->method('getLocalRepository')->willReturn($this->localRepo);
 
         $this->composer = $this->createMock(Composer::class);
         $this->composer->method('getRepositoryManager')
@@ -316,6 +336,7 @@ final class ResolveDependencyWithComposerTest extends TestCase
         $repoManager = $this->createMock(RepositoryManager::class);
         $repoManager->method('getRepositories')
             ->willReturn([new ArrayRepository([$pkg])]);
+        $repoManager->method('getLocalRepository')->willReturn($this->localRepo);
 
         $this->composer = $this->createMock(Composer::class);
         $this->composer->method('getRepositoryManager')
@@ -350,4 +371,35 @@ final class ResolveDependencyWithComposerTest extends TestCase
             false,
         );
     }
+
+    public function testPackageThatCanBeResolvedWithReplaceConflict(): void
+    {
+        $phpBinaryPath = $this->createMock(PhpBinaryPath::class);
+        $phpBinaryPath->expects(self::any())
+            ->method('version')
+            ->willReturn('8.3.0');
+        $phpBinaryPath->expects(self::any())
+            ->method('extensions')
+            ->willReturn([
+                'installed1' => '1.2.3',
+                'installed2' => '1.2.3',
+            ]);
+
+        $targetPlatform = new TargetPlatform(
+            OperatingSystem::NonWindows,
+            OperatingSystemFamily::Linux,
+            $phpBinaryPath,
+            Architecture::x86_64,
+            ThreadSafetyMode::ThreadSafe,
+            1,
+            null,
+        );
+
+        $package = (new ResolveDependencyWithComposer(
+            $this->createMock(QuieterConsoleIO::class),
+        ))($this->composer, $targetPlatform, new RequestedPackageAndVersion('asgrim/example-pie-extension', '^1.0'), false);
+
+        self::assertSame('asgrim/example-pie-extension', $package->name());
+        self::assertStringStartsWith('1.', $package->version());
+    }
 }