Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix replace conflict bug #203

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion src/ComposerIntegration/PhpBinaryPathBasedPlatformRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand All @@ -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
Expand All @@ -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));
}

Expand Down
6 changes: 5 additions & 1 deletion src/ComposerIntegration/PieComposerInstaller.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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(
Expand All @@ -51,6 +54,7 @@ public static function createWithPhpBinary(

$composerInstaller->phpBinaryPath = $php;
$composerInstaller->extensionBeingInstalled = $extensionBeingInstalled;
$composerInstaller->composer = $composer;

return $composerInstaller;
}
Expand Down
3 changes: 2 additions & 1 deletion src/ComposerIntegration/VersionSelectorFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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),
);
}
}
23 changes: 15 additions & 8 deletions src/ExtensionName.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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')
Expand All @@ -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(
[
Expand All @@ -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);
Expand All @@ -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(
[
Expand Down
15 changes: 14 additions & 1 deletion test/unit/ComposerIntegration/VersionSelectorFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down
Loading