Adjust according to feedback

main
Jordi Boggiano 11 years ago
parent db4055b778
commit 0700cd9186

@ -60,14 +60,14 @@ class DefaultPolicy implements PolicyInterface
return $pool->getPriority($package->getRepository()); return $pool->getPriority($package->getRepository());
} }
public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals, Rule $rule = null) public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals, $requiredPackage = null)
{ {
$packages = $this->groupLiteralsByNamePreferInstalled($pool, $installedMap, $literals); $packages = $this->groupLiteralsByNamePreferInstalled($pool, $installedMap, $literals);
foreach ($packages as &$literals) { foreach ($packages as &$literals) {
$policy = $this; $policy = $this;
usort($literals, function ($a, $b) use ($policy, $pool, $installedMap, $rule) { usort($literals, function ($a, $b) use ($policy, $pool, $installedMap, $requiredPackage) {
return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $rule, true); return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage, true);
}); });
} }
@ -82,8 +82,8 @@ class DefaultPolicy implements PolicyInterface
$selected = call_user_func_array('array_merge', $packages); $selected = call_user_func_array('array_merge', $packages);
// now sort the result across all packages to respect replaces across packages // now sort the result across all packages to respect replaces across packages
usort($selected, function ($a, $b) use ($policy, $pool, $installedMap, $rule) { usort($selected, function ($a, $b) use ($policy, $pool, $installedMap, $requiredPackage) {
return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $rule); return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), $requiredPackage);
}); });
return $selected; return $selected;
@ -112,7 +112,7 @@ class DefaultPolicy implements PolicyInterface
/** /**
* @protected * @protected
*/ */
public function compareByPriorityPreferInstalled(Pool $pool, array $installedMap, PackageInterface $a, PackageInterface $b, Rule $rule = null, $ignoreReplace = false) public function compareByPriorityPreferInstalled(Pool $pool, array $installedMap, PackageInterface $a, PackageInterface $b, $requiredPackage = null, $ignoreReplace = false)
{ {
if ($a->getRepository() === $b->getRepository()) { if ($a->getRepository() === $b->getRepository()) {
// prefer aliases to the original package // prefer aliases to the original package
@ -138,15 +138,9 @@ class DefaultPolicy implements PolicyInterface
// for replacers not replacing each other, put a higher prio on replacing // for replacers not replacing each other, put a higher prio on replacing
// packages with the same vendor as the required package // packages with the same vendor as the required package
if ($rule) { if ($requiredPackage && false !== ($pos = strpos($requiredPackage, '/'))) {
if ($rule->getReason() === Rule::RULE_JOB_INSTALL) { $requiredVendor = substr($requiredPackage, 0, $pos);
$required = $rule->getReasonData();
} elseif ($rule->getReason() === Rule::RULE_PACKAGE_REQUIRES) {
$required = $rule->getReasonData()->getTarget();
}
if ($required && (false !== ($pos = strpos($required, '/')))) {
$requiredVendor = substr($required, 0, $pos);
$aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor; $aIsSameVendor = substr($a->getName(), 0, $pos) === $requiredVendor;
$bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor; $bIsSameVendor = substr($b->getName(), 0, $pos) === $requiredVendor;
@ -155,7 +149,6 @@ class DefaultPolicy implements PolicyInterface
} }
} }
} }
}
// priority equal, sort by package id to make reproducible // priority equal, sort by package id to make reproducible
if ($a->getId() === $b->getId()) { if ($a->getId() === $b->getId()) {

@ -82,14 +82,15 @@ class Rule
return $this->job; return $this->job;
} }
public function getReason() public function getRequiredPackage()
{ {
return $this->reason; if ($this->reason === self::RULE_JOB_INSTALL) {
return $this->reasonData;
} }
public function getReasonData() if ($this->reason === self::RULE_PACKAGE_REQUIRES) {
{ return $this->reasonData->getTarget();
return $this->reasonData; }
} }
/** /**

@ -321,7 +321,7 @@ class Solver
private function selectAndInstall($level, array $decisionQueue, $disableRules, Rule $rule) private function selectAndInstall($level, array $decisionQueue, $disableRules, Rule $rule)
{ {
// choose best package to install from decisionQueue // choose best package to install from decisionQueue
$literals = $this->policy->selectPreferedPackages($this->pool, $this->installedMap, $decisionQueue, $rule); $literals = $this->policy->selectPreferedPackages($this->pool, $this->installedMap, $decisionQueue, $rule->getRequiredPackage());
$selectedLiteral = array_shift($literals); $selectedLiteral = array_shift($literals);

@ -195,6 +195,7 @@ class DefaultPolicyTest extends TestCase
public function testPreferReplacingPackageFromSameVendor() public function testPreferReplacingPackageFromSameVendor()
{ {
// test with default order
$this->repo->addPackage($packageB = $this->getPackage('vendor-b/replacer', '1.0')); $this->repo->addPackage($packageB = $this->getPackage('vendor-b/replacer', '1.0'));
$this->repo->addPackage($packageA = $this->getPackage('vendor-a/replacer', '1.0')); $this->repo->addPackage($packageA = $this->getPackage('vendor-a/replacer', '1.0'));
@ -206,16 +207,21 @@ class DefaultPolicyTest extends TestCase
$literals = array($packageA->getId(), $packageB->getId()); $literals = array($packageA->getId(), $packageB->getId());
$expected = $literals; $expected = $literals;
// test with install rule $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, 'vendor-a/package');
$rule = new Rule($this->pool, $literals, Rule::RULE_JOB_INSTALL, 'vendor-a/package');
$selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, $rule);
$this->assertEquals($expected, $selected); $this->assertEquals($expected, $selected);
// test with requires rule // test with reversed order in repo
$rule = new Rule($this->pool, $literals, Rule::RULE_PACKAGE_REQUIRES, new Link('foo', 'vendor-a/package')); $repo = new ArrayRepository;
$selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, $rule); $repo->addPackage($packageA = clone $packageA);
$repo->addPackage($packageB = clone $packageB);
$pool = new Pool('dev');
$pool->addRepository($this->repo);
$literals = array($packageA->getId(), $packageB->getId());
$expected = $literals;
$selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals, 'vendor-a/package');
$this->assertEquals($expected, $selected); $this->assertEquals($expected, $selected);
} }

Loading…
Cancel
Save