Skip to content

Commit

Permalink
Fix PoolOptimizer should consider disjunctive MultiConstraints (#10579)
Browse files Browse the repository at this point in the history
  • Loading branch information
Toflar committed Mar 12, 2022
1 parent b3f99fa commit ced24da
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 7 deletions.
60 changes: 54 additions & 6 deletions src/Composer/DependencyResolver/PoolOptimizer.php
Expand Up @@ -110,21 +110,18 @@ private function prepare(Request $request, Pool $pool)

// Extract requested package requirements
foreach ($request->getRequires() as $require => $constraint) {
$constraint = Intervals::compactConstraint($constraint);
$this->requireConstraintsPerPackage[$require][(string) $constraint] = $constraint;
$this->extractRequireConstraintsPerPackage($require, $constraint);
}

// First pass over all packages to extract information and mark package constraints irremovable
foreach ($pool->getPackages() as $package) {
// Extract package requirements
foreach ($package->getRequires() as $link) {
$constraint = Intervals::compactConstraint($link->getConstraint());
$this->requireConstraintsPerPackage[$link->getTarget()][(string) $constraint] = $constraint;
$this->extractRequireConstraintsPerPackage($link->getTarget(), $link->getConstraint());
}
// Extract package conflicts
foreach ($package->getConflicts() as $link) {
$constraint = Intervals::compactConstraint($link->getConstraint());
$this->conflictConstraintsPerPackage[$link->getTarget()][(string) $constraint] = $constraint;
$this->extractConflictConstraintsPerPackage($link->getTarget(), $link->getConstraint());
}

// Keep track of alias packages for every package so if either the alias or aliased is kept
Expand Down Expand Up @@ -453,4 +450,55 @@ private function optimizeImpossiblePackagesAway(Request $request, Pool $pool)
}
}
}

/**
* Disjunctive require constraints need to be considered in their own group. E.g. "^2.14 || ^3.3" needs to generate
* two require constraint groups in order for us to keep the best matching package for "^2.14" AND "^3.3" as otherwise, we'd
* only keep either one which can cause trouble (e.g. when using --prefer-lowest).
*
* @param string $package
* @param ConstraintInterface $constraint
* @return void
*/
private function extractRequireConstraintsPerPackage($package, ConstraintInterface $constraint)
{
foreach ($this->expandDisjunctiveMultiConstraints($constraint) as $expanded) {
$this->requireConstraintsPerPackage[$package][(string) $expanded] = $expanded;
}
}

/**
* Disjunctive conflict constraints need to be considered in their own group. E.g. "^2.14 || ^3.3" needs to generate
* two conflict constraint groups in order for us to keep the best matching package for "^2.14" AND "^3.3" as otherwise, we'd
* only keep either one which can cause trouble (e.g. when using --prefer-lowest).
*
* @param string $package
* @param ConstraintInterface $constraint
* @return void
*/
private function extractConflictConstraintsPerPackage($package, ConstraintInterface $constraint)
{
foreach ($this->expandDisjunctiveMultiConstraints($constraint) as $expanded) {
$this->conflictConstraintsPerPackage[$package][(string) $expanded] = $expanded;
}
}

/**
*
* @param ConstraintInterface $constraint
* @return ConstraintInterface[]
*/
private function expandDisjunctiveMultiConstraints(ConstraintInterface $constraint)
{
$constraint = Intervals::compactConstraint($constraint);

if ($constraint instanceof MultiConstraint && $constraint->isDisjunctive()) {
// No need to call ourselves recursively here because Intervals::compactConstraint() ensures that there
// are no nested disjunctive MultiConstraint instances possible
return $constraint->getConstraints();
}

// Regular constraints and conjunctive MultiConstraints
return array($constraint);
}
}
Expand Up @@ -15,7 +15,7 @@ Test filters irrelevant package "package/b" in version 1.0.0
"name": "package/a",
"version": "1.0.0",
"require": {
"package/b": "^1.0"
"package/b": ">=1.0 <1.1 || ^1.2"
}
},
{
Expand All @@ -25,6 +25,10 @@ Test filters irrelevant package "package/b" in version 1.0.0
{
"name": "package/b",
"version": "1.0.1"
},
{
"name": "package/b",
"version": "1.2.0"
}
]

Expand All @@ -41,6 +45,10 @@ Test filters irrelevant package "package/b" in version 1.0.0
{
"name": "package/b",
"version": "1.0.1"
},
{
"name": "package/b",
"version": "1.2.0"
}
]

@@ -0,0 +1,55 @@
--TEST--
Test keeps package "package/b" in version 2.2.0 because for prefer-lowest either one might be relevant

--REQUEST--
{
"require": {
"package/a": "^1.0"
},
"preferLowest": true
}


--POOL-BEFORE--
[
{
"name": "package/a",
"version": "1.0.0",
"require": {
"package/b": "^1.0 || ^2.2"
}
},
{
"name": "package/b",
"version": "1.0.0"
},
{
"name": "package/b",
"version": "1.0.1"
},
{
"name": "package/b",
"version": "2.2.0"
}
]


--POOL-AFTER--
[
{
"name": "package/a",
"version": "1.0.0",
"require": {
"package/b": "^1.0"
}
},
{
"name": "package/b",
"version": "1.0.0"
},
{
"name": "package/b",
"version": "2.2.0"
}
]

0 comments on commit ced24da

Please sign in to comment.