1
0
Fork 0

Merge pull request #8567 from naderman/provide-no-conflict-with-replace

Remove obsolete rules
pull/8740/head
Nils Adermann 2020-04-07 11:21:09 +02:00 committed by GitHub
commit ce8e9d24bd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 69 additions and 179 deletions

View File

@ -27,14 +27,8 @@ use Composer\Package\PackageInterface;
*/ */
class Pool implements \Countable class Pool implements \Countable
{ {
const MATCH_NONE = 0;
const MATCH = 1;
const MATCH_PROVIDE = 2;
const MATCH_REPLACE = 3;
protected $packages = array(); protected $packages = array();
protected $packageByName = array(); protected $packageByName = array();
protected $packageByExactName = array();
protected $versionParser; protected $versionParser;
protected $providerCache = array(); protected $providerCache = array();
protected $unacceptableFixedPackages; protected $unacceptableFixedPackages;
@ -54,7 +48,6 @@ class Pool implements \Countable
$this->packages[] = $package; $this->packages[] = $package;
$package->id = $id++; $package->id = $id++;
$this->packageByExactName[$package->getName()][$package->id] = $package;
foreach ($package->getNames() as $provided) { foreach ($package->getNames() as $provided) {
$this->packageByName[$provided][] = $package; $this->packageByName[$provided][] = $package;
@ -87,50 +80,32 @@ class Pool implements \Countable
* @param string $name The package name to be searched for * @param string $name The package name to be searched for
* @param ConstraintInterface $constraint A constraint that all returned * @param ConstraintInterface $constraint A constraint that all returned
* packages must match or null to return all * packages must match or null to return all
* @param bool $mustMatchName Whether the name of returned packages
* must match the given name
* @return PackageInterface[] A set of packages * @return PackageInterface[] A set of packages
*/ */
public function whatProvides($name, ConstraintInterface $constraint = null, $mustMatchName = false) public function whatProvides($name, ConstraintInterface $constraint = null)
{ {
$key = ((int) $mustMatchName).$constraint; $key = (string) $constraint;
if (isset($this->providerCache[$name][$key])) { if (isset($this->providerCache[$name][$key])) {
return $this->providerCache[$name][$key]; return $this->providerCache[$name][$key];
} }
return $this->providerCache[$name][$key] = $this->computeWhatProvides($name, $constraint, $mustMatchName); return $this->providerCache[$name][$key] = $this->computeWhatProvides($name, $constraint);
} }
/** /**
* @see whatProvides * @see whatProvides
*/ */
private function computeWhatProvides($name, $constraint, $mustMatchName = false) private function computeWhatProvides($name, $constraint)
{ {
$candidates = array(); if (!isset($this->packageByName[$name])) {
return array();
if ($mustMatchName) {
if (isset($this->packageByExactName[$name])) {
$candidates = $this->packageByExactName[$name];
}
} elseif (isset($this->packageByName[$name])) {
$candidates = $this->packageByName[$name];
} }
$matches = array(); $matches = array();
foreach ($candidates as $candidate) { foreach ($this->packageByName[$name] as $candidate) {
switch ($this->match($candidate, $name, $constraint)) { if ($this->match($candidate, $name, $constraint)) {
case self::MATCH_NONE: $matches[] = $candidate;
break;
case self::MATCH:
case self::MATCH_PROVIDE:
case self::MATCH_REPLACE:
$matches[] = $candidate;
break;
default:
throw new \UnexpectedValueException('Unexpected match type');
} }
} }
@ -164,7 +139,7 @@ class Pool implements \Countable
* @param PackageInterface $candidate * @param PackageInterface $candidate
* @param string $name Name of the package to be matched * @param string $name Name of the package to be matched
* @param ConstraintInterface $constraint The constraint to verify * @param ConstraintInterface $constraint The constraint to verify
* @return int One of the MATCH* constants of this class or 0 if there is no match * @return bool
*/ */
public function match($candidate, $name, ConstraintInterface $constraint = null) public function match($candidate, $name, ConstraintInterface $constraint = null)
{ {
@ -175,10 +150,10 @@ class Pool implements \Countable
$pkgConstraint = new Constraint('==', $candidateVersion); $pkgConstraint = new Constraint('==', $candidateVersion);
if ($constraint === null || $constraint->matches($pkgConstraint)) { if ($constraint === null || $constraint->matches($pkgConstraint)) {
return self::MATCH; return true;
} }
return self::MATCH_NONE; return false;
} }
$provides = $candidate->getProvides(); $provides = $candidate->getProvides();
@ -188,28 +163,28 @@ class Pool implements \Countable
if (isset($replaces[0]) || isset($provides[0])) { if (isset($replaces[0]) || isset($provides[0])) {
foreach ($provides as $link) { foreach ($provides as $link) {
if ($link->getTarget() === $name && ($constraint === null || $constraint->matches($link->getConstraint()))) { if ($link->getTarget() === $name && ($constraint === null || $constraint->matches($link->getConstraint()))) {
return self::MATCH_PROVIDE; return true;
} }
} }
foreach ($replaces as $link) { foreach ($replaces as $link) {
if ($link->getTarget() === $name && ($constraint === null || $constraint->matches($link->getConstraint()))) { if ($link->getTarget() === $name && ($constraint === null || $constraint->matches($link->getConstraint()))) {
return self::MATCH_REPLACE; return true;
} }
} }
return self::MATCH_NONE; return false;
} }
if (isset($provides[$name]) && ($constraint === null || $constraint->matches($provides[$name]->getConstraint()))) { if (isset($provides[$name]) && ($constraint === null || $constraint->matches($provides[$name]->getConstraint()))) {
return self::MATCH_PROVIDE; return true;
} }
if (isset($replaces[$name]) && ($constraint === null || $constraint->matches($replaces[$name]->getConstraint()))) { if (isset($replaces[$name]) && ($constraint === null || $constraint->matches($replaces[$name]->getConstraint()))) {
return self::MATCH_REPLACE; return true;
} }
return self::MATCH_NONE; return false;
} }
public function isUnacceptableFixedPackage(PackageInterface $package) public function isUnacceptableFixedPackage(PackageInterface $package)

View File

@ -29,10 +29,7 @@ abstract class Rule
const RULE_FIXED = 3; const RULE_FIXED = 3;
const RULE_PACKAGE_CONFLICT = 6; const RULE_PACKAGE_CONFLICT = 6;
const RULE_PACKAGE_REQUIRES = 7; const RULE_PACKAGE_REQUIRES = 7;
const RULE_PACKAGE_OBSOLETES = 8;
const RULE_INSTALLED_PACKAGE_OBSOLETES = 9;
const RULE_PACKAGE_SAME_NAME = 10; const RULE_PACKAGE_SAME_NAME = 10;
const RULE_PACKAGE_IMPLICIT_OBSOLETES = 11;
const RULE_LEARNED = 12; const RULE_LEARNED = 12;
const RULE_PACKAGE_ALIAS = 13; const RULE_PACKAGE_ALIAS = 13;
@ -191,83 +188,50 @@ abstract class Rule
return $text; return $text;
case self::RULE_PACKAGE_OBSOLETES:
if (count($literals) === 2 && $literals[0] < 0 && $literals[1] < 0) {
$package1 = $pool->literalToPackage($literals[0]);
$package2 = $pool->literalToPackage($literals[1]);
$replaces1 = $this->getReplacedNames($package1);
$replaces2 = $this->getReplacedNames($package2);
$reason = null;
if ($conflictingNames = array_values(array_intersect($replaces1, $replaces2))) {
$reason = 'They both replace '.(count($conflictingNames) > 1 ? '['.implode(', ', $conflictingNames).']' : $conflictingNames[0]).' and thus cannot coexist.';
} elseif (in_array($package1->getName(), $replaces2, true)) {
$reason = $package2->getName().' replaces '.$package1->getName().' and thus cannot coexist with it.';
} elseif (in_array($package2->getName(), $replaces1, true)) {
$reason = $package1->getName().' replaces '.$package2->getName().' and thus cannot coexist with it.';
}
if ($reason) {
if (isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) {
// swap vars so the if below passes
$tmp = $package2;
$package2 = $package1;
$package1 = $tmp;
}
if (!isset($installedMap[$package1->id]) && isset($installedMap[$package2->id])) {
return $package1->getPrettyString().' cannot be installed as that would require removing '.$package2->getPrettyString().'. '.$reason;
}
if (!isset($installedMap[$package1->id]) && !isset($installedMap[$package2->id])) {
return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'. '.$reason;
}
}
return 'Only one of these can be installed: '.$package1->getPrettyString().', '.$package2->getPrettyString().'.';
}
return $ruleText;
case self::RULE_INSTALLED_PACKAGE_OBSOLETES:
return $ruleText;
case self::RULE_PACKAGE_SAME_NAME: case self::RULE_PACKAGE_SAME_NAME:
$replacedNames = null;
$packageNames = array(); $packageNames = array();
foreach ($literals as $literal) { foreach ($literals as $literal) {
$package = $pool->literalToPackage($literal); $package = $pool->literalToPackage($literal);
$pkgReplaces = $this->getReplacedNames($package);
if ($pkgReplaces) {
if ($replacedNames === null) {
$replacedNames = $this->getReplacedNames($package);
} else {
$replacedNames = array_intersect($replacedNames, $this->getReplacedNames($package));
}
}
$packageNames[$package->getName()] = true; $packageNames[$package->getName()] = true;
} }
$replacedName = $this->reasonData;
if ($replacedNames) { if (count($packageNames) > 1) {
$replacedNames = array_values(array_intersect(array_keys($packageNames), $replacedNames)); $reason = null;
}
if ($replacedNames && count($packageNames) > 1) { if (!isset($packageNames[$replacedName])) {
$replacer = null; $reason = 'They '.(count($literals) == 2 ? 'both' : 'all').' replace '.$replacedName.' and thus cannot coexist.';
} else {
$replacerNames = $packageNames;
unset($replacerNames[$replacedName]);
$replacerNames = array_keys($replacerNames);
if (count($replacerNames) == 1) {
$reason = $replacerNames[0] . ' replaces ';
} else {
$reason = '['.implode(', ', $replacerNames).'] replace ';
}
$reason .= $replacedName.' and thus cannot coexist with it.';
}
$installedPackages = array();
$removablePackages = array();
foreach ($literals as $literal) { foreach ($literals as $literal) {
$package = $pool->literalToPackage($literal); if (isset($installedMap[abs($literal)])) {
if (array_intersect($replacedNames, $this->getReplacedNames($package))) { $installedPackages[] = $pool->literalToPackage($literal);
$replacer = $package; } else {
break; $removablePackages[] = $pool->literalToPackage($literal);
} }
} }
$replacedNames = count($replacedNames) > 1 ? '['.implode(', ', $replacedNames).']' : $replacedNames[0];
if ($replacer) { if ($installedPackages && $removablePackages) {
return 'Only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '. '.$replacer->getName().' replaces '.$replacedNames.' and thus cannot coexist with it.'; return $this->formatPackagesUnique($pool, $removablePackages).' cannot be installed as that would require removing '.$this->formatPackagesUnique($pool, $installedPackages).'. '.$reason;
} }
return 'Only one of these can be installed: '.$this->formatPackagesUnique($pool, $literals).'. '.$reason;
} }
return 'You can only install one version of a package, so only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '.'; return 'You can only install one version of a package, so only one of these can be installed: ' . $this->formatPackagesUnique($pool, $literals) . '.';
case self::RULE_PACKAGE_IMPLICIT_OBSOLETES:
return $ruleText;
case self::RULE_LEARNED: case self::RULE_LEARNED:
if (isset($learnedPool[$this->reasonData])) { if (isset($learnedPool[$this->reasonData])) {
$learnedString = ', learned rules:'."\n - "; $learnedString = ', learned rules:'."\n - ";

View File

@ -160,8 +160,12 @@ class RuleSetGenerator
$this->addedMap[$package->id] = true; $this->addedMap[$package->id] = true;
$this->addedPackages[] = $package; $this->addedPackages[] = $package;
foreach ($package->getNames() as $name) { if (!$package instanceof AliasPackage) {
$this->addedPackagesByNames[$name][] = $package; foreach ($package->getNames(false) as $name) {
$this->addedPackagesByNames[$name][] = $package;
}
} else {
$this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, array($package->getAliasOf()), Rule::RULE_PACKAGE_ALIAS, $package));
} }
foreach ($package->getRequires() as $link) { foreach ($package->getRequires() as $link) {
@ -177,29 +181,6 @@ class RuleSetGenerator
$workQueue->enqueue($require); $workQueue->enqueue($require);
} }
} }
$packageName = $package->getName();
$obsoleteProviders = $this->pool->whatProvides($packageName, null);
foreach ($obsoleteProviders as $provider) {
if ($provider === $package) {
continue;
}
if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) {
$this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, $package));
} else {
if (!isset($this->conflictsForName[$packageName])) {
$this->conflictsForName[$packageName] = array();
}
if (!$package instanceof AliasPackage) {
$this->conflictsForName[$packageName][$package->id] = $package;
}
if (!$provider instanceof AliasPackage) {
$this->conflictsForName[$packageName][$provider->id] = $provider;
}
}
}
} }
} }
@ -218,39 +199,17 @@ class RuleSetGenerator
/** @var PackageInterface $possibleConflict */ /** @var PackageInterface $possibleConflict */
foreach ($this->addedPackagesByNames[$link->getTarget()] as $possibleConflict) { foreach ($this->addedPackagesByNames[$link->getTarget()] as $possibleConflict) {
$conflictMatch = $this->pool->match($possibleConflict, $link->getTarget(), $link->getConstraint()); if ($this->pool->match($possibleConflict, $link->getTarget(), $link->getConstraint())) {
if ($conflictMatch === Pool::MATCH || $conflictMatch === Pool::MATCH_REPLACE) {
$this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $possibleConflict, Rule::RULE_PACKAGE_CONFLICT, $link)); $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $possibleConflict, Rule::RULE_PACKAGE_CONFLICT, $link));
} }
}
}
// check obsoletes and implicit obsoletes of a package
foreach ($package->getReplaces() as $link) {
if (!isset($this->addedPackagesByNames[$link->getTarget()])) {
continue;
}
/** @var PackageInterface $possibleConflict */
foreach ($this->addedPackagesByNames[$link->getTarget()] as $provider) {
if ($provider === $package) {
continue;
}
if (!$this->obsoleteImpossibleForAlias($package, $provider)) {
$reason = Rule::RULE_PACKAGE_OBSOLETES;
$this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $provider, $reason, $link));
}
} }
} }
} }
foreach ($this->conflictsForName as $name => $packages) { foreach ($this->addedPackagesByNames as $name => $packages) {
if (count($packages) > 1) { if (count($packages) > 1) {
$reason = Rule::RULE_PACKAGE_SAME_NAME; $reason = Rule::RULE_PACKAGE_SAME_NAME;
$this->addRule(RuleSet::TYPE_PACKAGE, $this->createMultiConflictRule($packages, $reason, null)); $this->addRule(RuleSet::TYPE_PACKAGE, $this->createMultiConflictRule($packages, $reason, $name));
} }
} }
} }

View File

@ -89,14 +89,16 @@ abstract class BasePackage implements PackageInterface
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function getNames() public function getNames($provides = true)
{ {
$names = array( $names = array(
$this->getName() => true, $this->getName() => true,
); );
foreach ($this->getProvides() as $link) { if ($provides) {
$names[$link->getTarget()] = true; foreach ($this->getProvides() as $link) {
$names[$link->getTarget()] = true;
}
} }
foreach ($this->getReplaces() as $link) { foreach ($this->getReplaces() as $link) {

View File

@ -45,9 +45,11 @@ interface PackageInterface
* No version or release type information should be included in any of the * No version or release type information should be included in any of the
* names. Provided or replaced package names need to be returned as well. * names. Provided or replaced package names need to be returned as well.
* *
* @param bool $provides Whether provided names should be included
*
* @return array An array of strings referring to this package * @return array An array of strings referring to this package
*/ */
public function getNames(); public function getNames($provides = true);
/** /**
* Allows the solver to set an id for this package to refer to it. * Allows the solver to set an id for this package to refer to it.

View File

@ -42,7 +42,7 @@ Your requirements could not be resolved to an installable set of packages.
Problem 1 Problem 1
- __root__ is present at version 1.2.3 and cannot be modified by Composer - __root__ is present at version 1.2.3 and cannot be modified by Composer
- provider/pkg 1.0.0 cannot be installed as that would require removing __root__ 1.2.3. They both replace root-replaced/transitive-replaced and thus cannot coexist. - provider/pkg[1.0.0] cannot be installed as that would require removing __root__[1.2.3]. They both replace root-replaced/transitive-replaced and thus cannot coexist.
- Root composer.json requires provider/pkg * -> satisfiable by provider/pkg[1.0.0]. - Root composer.json requires provider/pkg * -> satisfiable by provider/pkg[1.0.0].
--EXPECT-- --EXPECT--

View File

@ -1,5 +1,5 @@
--TEST-- --TEST--
Test that names provided by two dependents cause a conflict Providers of a replaced name should be installable
--COMPOSER-- --COMPOSER--
{ {
"repositories": [ "repositories": [
@ -28,18 +28,6 @@ Test that names provided by two dependents cause a conflict
--RUN-- --RUN--
update update
--EXPECT-EXIT-CODE--
2
--EXPECT-OUTPUT--
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Root composer.json requires provider/pkg * -> satisfiable by provider/pkg[1.0.0].
- Only one of these can be installed: replacer/pkg 1.0.0, provider/pkg 1.0.0.
- Root composer.json requires replacer/pkg * -> satisfiable by replacer/pkg[1.0.0].
--EXPECT-- --EXPECT--
Installing provider/pkg (1.0.0)
Installing replacer/pkg (1.0.0)

View File

@ -48,7 +48,7 @@ Your requirements could not be resolved to an installable set of packages.
Problem 1 Problem 1
- current/dep is locked to version 1.0.0 and an update of this package was not requested. - current/dep is locked to version 1.0.0 and an update of this package was not requested.
- new/pkg 1.0.0 cannot be installed as that would require removing current/dep 1.0.0. new/pkg replaces current/dep and thus cannot coexist with it. - new/pkg[1.0.0] cannot be installed as that would require removing current/dep[1.0.0]. new/pkg replaces current/dep and thus cannot coexist with it.
- Root composer.json requires new/pkg 1.* -> satisfiable by new/pkg[1.0.0]. - Root composer.json requires new/pkg 1.* -> satisfiable by new/pkg[1.0.0].
Use the option --with-all-dependencies to allow updates and removals for packages currently locked to specific versions. Use the option --with-all-dependencies to allow updates and removals for packages currently locked to specific versions.