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 PoolOptimizer should consider disjunctive MultiConstraints #10579

Merged
merged 8 commits into from Mar 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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"
}
]