From 8fc09afbae6bc6cf4d7f70da5668cd00ddc7138b Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 19:55:41 +0200 Subject: [PATCH 01/27] Move transaction generation to a separate class --- .../DependencyResolver/DefaultPolicy.php | 2 +- .../DependencyResolver/PolicyInterface.php | 2 +- src/Composer/DependencyResolver/Solver.php | 162 +---------------- .../DependencyResolver/Transaction.php | 169 ++++++++++++++++++ 4 files changed, 174 insertions(+), 161 deletions(-) create mode 100644 src/Composer/DependencyResolver/Transaction.php diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index ac3d38995..1fe22ff3c 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -30,7 +30,7 @@ class DefaultPolicy implements PolicyInterface return $constraint->matchSpecific($version); } - public function findUpdatePackages(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package) + public function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package) { $packages = array(); diff --git a/src/Composer/DependencyResolver/PolicyInterface.php b/src/Composer/DependencyResolver/PolicyInterface.php index 7c26ab63f..1f65c1bf9 100644 --- a/src/Composer/DependencyResolver/PolicyInterface.php +++ b/src/Composer/DependencyResolver/PolicyInterface.php @@ -21,7 +21,7 @@ use Composer\Package\PackageInterface; interface PolicyInterface { function versionCompare(PackageInterface $a, PackageInterface $b, $operator); - function findUpdatePackages(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); + function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package); function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); function selectPreferedPackages(Pool $pool, array $installedMap, array $literals); } diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index a03e27464..5a243a2ec 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -84,30 +84,6 @@ class Solver return new Rule($literals, $reason, $reasonData); } - /** - * Create a new rule for updating a package - * - * If package A1 can be updated to A2 or A3 the rule is (A1|A2|A3). - * - * @param PackageInterface $package The package to be updated - * @param array $updates An array of update candidate packages - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that - * goes with the reason - * @return Rule The generated rule or null if tautology - */ - protected function createUpdateRule(PackageInterface $package, array $updates, $reason, $reasonData = null) - { - $literals = array(new Literal($package, true)); - - foreach ($updates as $update) { - $literals[] = new Literal($update, true); - } - - return new Rule($literals, $reason, $reasonData); - } - /** * Creates a new rule for installing a package * @@ -308,7 +284,7 @@ class Solver */ private function addRulesForUpdatePackages(PackageInterface $package) { - $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package); + $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); $this->addRulesForPackage($package); @@ -568,13 +544,6 @@ class Solver } } - foreach ($this->installedMap as $package) { - $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package); - $rule = $this->createUpdateRule($package, $updates, Rule::RULE_INTERNAL_ALLOW_UPDATE, (string) $package); - - $this->packageToUpdateRule[$package->getId()] = $rule; - } - foreach ($this->jobs as $job) { switch ($job['cmd']) { case 'install': @@ -627,133 +596,8 @@ class Solver throw new SolverProblemsException($this->problems); } - return $this->createTransaction(); - } - - protected function createTransaction() - { - $transaction = array(); - $installMeansUpdateMap = array(); - - foreach ($this->decisionQueue as $i => $literal) { - $package = $literal->getPackage(); - - // !wanted & installed - if (!$literal->isWanted() && isset($this->installedMap[$package->getId()])) { - $literals = array(); - - if (isset($this->packageToUpdateRule[$package->getId()])) { - $literals = array_merge($literals, $this->packageToUpdateRule[$package->getId()]->getLiterals()); - } - - foreach ($literals as $updateLiteral) { - if (!$updateLiteral->equals($literal)) { - $installMeansUpdateMap[$updateLiteral->getPackageId()] = $package; - } - } - } - } - - foreach ($this->decisionQueue as $i => $literal) { - $package = $literal->getPackage(); - - // wanted & installed || !wanted & !installed - if ($literal->isWanted() == (isset($this->installedMap[$package->getId()]))) { - continue; - } - - if ($literal->isWanted()) { - if ($package instanceof AliasPackage) { - $transaction[] = new Operation\MarkAliasInstalledOperation( - $package, $this->decisionQueueWhy[$i] - ); - continue; - } - - if (isset($installMeansUpdateMap[$literal->getPackageId()])) { - - $source = $installMeansUpdateMap[$literal->getPackageId()]; - - $transaction[] = new Operation\UpdateOperation( - $source, $package, $this->decisionQueueWhy[$i] - ); - - // avoid updates to one package from multiple origins - unset($installMeansUpdateMap[$literal->getPackageId()]); - $ignoreRemove[$source->getId()] = true; - } else { - $transaction[] = new Operation\InstallOperation( - $package, $this->decisionQueueWhy[$i] - ); - } - } else if (!isset($ignoreRemove[$package->getId()])) { - if ($package instanceof AliasPackage) { - $transaction[] = new Operation\MarkAliasInstalledOperation( - $package, $this->decisionQueueWhy[$i] - ); - } else { - $transaction[] = new Operation\UninstallOperation( - $package, $this->decisionQueueWhy[$i] - ); - } - } - } - - $allDecidedMap = $this->decisionMap; - foreach ($this->decisionMap as $packageId => $decision) { - if ($decision != 0) { - $package = $this->pool->packageById($packageId); - if ($package instanceof AliasPackage) { - $allDecidedMap[$package->getAliasOf()->getId()] = $decision; - } - } - } - - foreach ($allDecidedMap as $packageId => $decision) { - if ($packageId === 0) { - continue; - } - - if (0 == $decision && isset($this->installedMap[$packageId])) { - $package = $this->pool->packageById($packageId); - - if ($package instanceof AliasPackage) { - $transaction[] = new Operation\MarkAliasInstalledOperation( - $package, null - ); - } else { - $transaction[] = new Operation\UninstallOperation( - $package, null - ); - } - - $this->decisionMap[$packageId] = -1; - } - } - - foreach ($this->decisionMap as $packageId => $decision) { - if ($packageId === 0) { - continue; - } - - if (0 == $decision && isset($this->installedMap[$packageId])) { - $package = $this->pool->packageById($packageId); - - if ($package instanceof AliasPackage) { - $transaction[] = new Operation\MarkAliasInstalledOperation( - $package, null - ); - } else { - $transaction[] = new Operation\UninstallOperation( - $package, null - ); - } - - $this->decisionMap[$packageId] = -1; - } - } - - return array_reverse($transaction); + $transaction = new Transaction($this->policy, $this->pool, $this->installedMap, $this->decisionMap, $this->decisionQueue, $this->decisionQueueWhy); + return $transaction->getOperations(); } protected function literalFromId($id) diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php new file mode 100644 index 000000000..14fe11836 --- /dev/null +++ b/src/Composer/DependencyResolver/Transaction.php @@ -0,0 +1,169 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +use Composer\Repository\RepositoryInterface; +use Composer\Package\PackageInterface; +use Composer\Package\AliasPackage; +use Composer\DependencyResolver\Operation; + +/** + * @author Nils Adermann + */ +class Transaction +{ + protected $policy; + protected $pool; + protected $installedMap; + protected $decisionMap; + protected $decisionQueue; + protected $decisionQueueWhy; + + public function __construct($policy, $pool, $installedMap, $decisionMap, array $decisionQueue, $decisionQueueWhy) + { + $this->policy = $policy; + $this->pool = $pool; + $this->installedMap = $installedMap; + $this->decisionMap = $decisionMap; + $this->decisionQueue = $decisionQueue; + $this->decisionQueueWhy = $decisionQueueWhy; + } + + public function getOperations() + { + $transaction = array(); + $installMeansUpdateMap = array(); + + foreach ($this->decisionQueue as $i => $literal) { + $package = $literal->getPackage(); + + // !wanted & installed + if (!$literal->isWanted() && isset($this->installedMap[$package->getId()])) { + $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); + + $literals = array(new Literal($package, true)); + + foreach ($updates as $update) { + $literals[] = new Literal($update, true); + } + + foreach ($literals as $updateLiteral) { + if (!$updateLiteral->equals($literal)) { + $installMeansUpdateMap[$updateLiteral->getPackageId()] = $package; + } + } + } + } + + foreach ($this->decisionQueue as $i => $literal) { + $package = $literal->getPackage(); + + // wanted & installed || !wanted & !installed + if ($literal->isWanted() == (isset($this->installedMap[$package->getId()]))) { + continue; + } + + if ($literal->isWanted()) { + if ($package instanceof AliasPackage) { + $transaction[] = new Operation\MarkAliasInstalledOperation( + $package, $this->decisionQueueWhy[$i] + ); + continue; + } + + if (isset($installMeansUpdateMap[$literal->getPackageId()])) { + + $source = $installMeansUpdateMap[$literal->getPackageId()]; + + $transaction[] = new Operation\UpdateOperation( + $source, $package, $this->decisionQueueWhy[$i] + ); + + // avoid updates to one package from multiple origins + unset($installMeansUpdateMap[$literal->getPackageId()]); + $ignoreRemove[$source->getId()] = true; + } else { + $transaction[] = new Operation\InstallOperation( + $package, $this->decisionQueueWhy[$i] + ); + } + } else if (!isset($ignoreRemove[$package->getId()])) { + if ($package instanceof AliasPackage) { + $transaction[] = new Operation\MarkAliasInstalledOperation( + $package, $this->decisionQueueWhy[$i] + ); + } else { + $transaction[] = new Operation\UninstallOperation( + $package, $this->decisionQueueWhy[$i] + ); + } + } + } + + $allDecidedMap = $this->decisionMap; + foreach ($this->decisionMap as $packageId => $decision) { + if ($decision != 0) { + $package = $this->pool->packageById($packageId); + if ($package instanceof AliasPackage) { + $allDecidedMap[$package->getAliasOf()->getId()] = $decision; + } + } + } + + foreach ($allDecidedMap as $packageId => $decision) { + if ($packageId === 0) { + continue; + } + + if (0 == $decision && isset($this->installedMap[$packageId])) { + $package = $this->pool->packageById($packageId); + + if ($package instanceof AliasPackage) { + $transaction[] = new Operation\MarkAliasInstalledOperation( + $package, null + ); + } else { + $transaction[] = new Operation\UninstallOperation( + $package, null + ); + } + + $this->decisionMap[$packageId] = -1; + } + } + + foreach ($this->decisionMap as $packageId => $decision) { + if ($packageId === 0) { + continue; + } + + if (0 == $decision && isset($this->installedMap[$packageId])) { + $package = $this->pool->packageById($packageId); + + if ($package instanceof AliasPackage) { + $transaction[] = new Operation\MarkAliasInstalledOperation( + $package, null + ); + } else { + $transaction[] = new Operation\UninstallOperation( + $package, null + ); + } + + $this->decisionMap[$packageId] = -1; + } + } + + return array_reverse($transaction); + } +} From bd66c27ff6fc3169eb5579d8d10aeddf95f7521b Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 19:57:55 +0200 Subject: [PATCH 02/27] We mark packages as uninstallable by removing them from the repo As it is faster to remove packages from the repos and keep them out of the package pool to begin with, we don't need an installable() method on the policy. --- src/Composer/DependencyResolver/DefaultPolicy.php | 6 ------ src/Composer/DependencyResolver/PolicyInterface.php | 1 - src/Composer/DependencyResolver/Rule.php | 4 ---- src/Composer/DependencyResolver/Solver.php | 5 ----- 4 files changed, 16 deletions(-) diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 1fe22ff3c..bbbf43025 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -43,12 +43,6 @@ class DefaultPolicy implements PolicyInterface return $packages; } - public function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package) - { - // todo: package blacklist? - return true; - } - public function getPriority(Pool $pool, PackageInterface $package) { return $pool->getPriority($package->getRepository()); diff --git a/src/Composer/DependencyResolver/PolicyInterface.php b/src/Composer/DependencyResolver/PolicyInterface.php index 1f65c1bf9..f00f857d4 100644 --- a/src/Composer/DependencyResolver/PolicyInterface.php +++ b/src/Composer/DependencyResolver/PolicyInterface.php @@ -22,6 +22,5 @@ interface PolicyInterface { function versionCompare(PackageInterface $a, PackageInterface $b, $operator); function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package); - function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); function selectPreferedPackages(Pool $pool, array $installedMap, array $literals); } diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 3b9ffcc28..4370d38c6 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -21,7 +21,6 @@ class Rule const RULE_JOB_INSTALL = 2; const RULE_JOB_REMOVE = 3; const RULE_JOB_LOCK = 4; - const RULE_NOT_INSTALLABLE = 5; const RULE_PACKAGE_CONFLICT = 6; const RULE_PACKAGE_REQUIRES = 7; const RULE_PACKAGE_OBSOLETES = 8; @@ -200,9 +199,6 @@ class Rule case self::RULE_JOB_LOCK: return "Lock command rule ($ruleText)"; - case self::RULE_NOT_INSTALLABLE: - return $ruleText; - case self::RULE_PACKAGE_CONFLICT: $package1 = $this->literals[0]->getPackage(); $package2 = $this->literals[1]->getPackage(); diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 5a243a2ec..514412ab7 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -198,11 +198,6 @@ class Solver $this->addedMap[$package->getId()] = true; - if (!$this->policy->installable($this, $this->pool, $this->installedMap, $package)) { - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRemoveRule($package, Rule::RULE_NOT_INSTALLABLE, (string) $package)); - continue; - } - foreach ($package->getRequires() as $link) { $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); From 92ecf5a603a3d779da3e1ddfeb5be30078c24853 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 20:00:03 +0200 Subject: [PATCH 03/27] Remove unused variables --- src/Composer/DependencyResolver/Solver.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 514412ab7..68f087737 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -32,12 +32,9 @@ class Solver protected $addedMap = array(); protected $updateMap = array(); protected $watches = array(); - protected $removeWatches = array(); protected $decisionMap; protected $installedMap; - protected $packageToUpdateRule = array(); - protected $decisionQueue = array(); protected $decisionQueueWhy = array(); protected $decisionQueueFree = array(); From 1d60ae1bfcd1ee659243d29ad57b18dda9454060 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 20:01:51 +0200 Subject: [PATCH 04/27] We no longer support recommended packages, remove left over code --- src/Composer/DependencyResolver/Solver.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 68f087737..85fa97aee 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -42,7 +42,6 @@ class Solver protected $branches = array(); protected $problems = array(); protected $learnedPool = array(); - protected $recommendsIndex; public function __construct(PolicyInterface $policy, Pool $pool, RepositoryInterface $installed) { @@ -581,8 +580,7 @@ class Solver /* make decisions based on job/update assertions */ $this->makeAssertionRuleDecisions(); - $installRecommended = 0; - $this->runSat(true, $installRecommended); + $this->runSat(true); if ($this->problems) { throw new SolverProblemsException($this->problems); @@ -801,8 +799,6 @@ class Solver array_pop($this->branches); } - - $this->recommendsIndex = -1; } /**------------------------------------------------------------------- @@ -1139,9 +1135,7 @@ class Solver $this->decisionQueueWhy = array(); $this->decisionQueueFree = array(); - $this->recommendsIndex = -1; $this->propagateIndex = 0; - $this->recommendations = array(); $this->branches = array(); $this->enableDisableLearnedRules(); @@ -1177,7 +1171,7 @@ class Solver } } - private function runSat($disableRules = true, $installRecommended = false) + private function runSat($disableRules = true) { $this->propagateIndex = 0; From 3fcd042fd405aebeaad40c1e4dbfa20c3670deef Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 20:03:57 +0200 Subject: [PATCH 05/27] Skip adding (ignored) duplicate rules which were already added --- src/Composer/DependencyResolver/Solver.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 85fa97aee..6cd3c55a3 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -277,8 +277,6 @@ class Solver { $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); - $this->addRulesForPackage($package); - foreach ($updates as $update) { $this->addRulesForPackage($update); } @@ -517,13 +515,9 @@ class Solver foreach ($this->installedMap as $package) { $this->addRulesForPackage($package); - } - - foreach ($this->installedMap as $package) { $this->addRulesForUpdatePackages($package); } - foreach ($this->jobs as $job) { foreach ($job['packages'] as $package) { switch ($job['cmd']) { From 70e306f05514cfa07d2ca8ef1065be56d46ce3fe Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 20:19:38 +0200 Subject: [PATCH 06/27] Process all jobs in one method and remove lock jobs which are not used --- src/Composer/DependencyResolver/Rule.php | 4 - src/Composer/DependencyResolver/Solver.php | 116 +++++++++------------ 2 files changed, 48 insertions(+), 72 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 4370d38c6..a7248f65a 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -20,7 +20,6 @@ class Rule const RULE_INTERNAL_ALLOW_UPDATE = 1; const RULE_JOB_INSTALL = 2; const RULE_JOB_REMOVE = 3; - const RULE_JOB_LOCK = 4; const RULE_PACKAGE_CONFLICT = 6; const RULE_PACKAGE_REQUIRES = 7; const RULE_PACKAGE_OBSOLETES = 8; @@ -196,9 +195,6 @@ class Rule case self::RULE_JOB_REMOVE: return "Remove command rule ($ruleText)"; - case self::RULE_JOB_LOCK: - return "Lock command rule ($ruleText)"; - case self::RULE_PACKAGE_CONFLICT: $package1 = $this->literals[0]->getPackage(); $package2 = $this->literals[1]->getPackage(); diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 6cd3c55a3..a447479e9 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -481,6 +481,53 @@ class Solver } } + protected function addRulesForJobs() + { + foreach ($this->jobs as $job) { + switch ($job['cmd']) { + case 'update': + foreach ($job['packages'] as $package) { + if (isset($this->installedMap[$package->getId()])) { + $this->updateMap[$package->getId()] = true; + } + } + break; + + case 'update-all': + foreach ($this->installedMap as $package) { + $this->updateMap[$package->getId()] = true; + } + break; + case 'install': + if (empty($job['packages'])) { + $problem = new Problem(); + $problem->addJobRule($job); + $this->problems[] = $problem; + } else { + foreach ($job['packages'] as $package) { + if (!isset($this->installedMap[$package->getId()])) { + $this->addRulesForPackage($package); + } + } + + $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job['packageName']); + $this->addRule(RuleSet::TYPE_JOB, $rule); + $this->ruleToJob[$rule->getId()] = $job; + } + break; + case 'remove': + // remove all packages with this name including uninstalled + // ones to make sure none of them are picked as replacements + foreach ($job['packages'] as $package) { + $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE); + $this->addRule(RuleSet::TYPE_JOB, $rule); + $this->ruleToJob[$rule->getId()] = $job; + } + break; + } + } + } + public function solve(Request $request) { $this->jobs = $request->getJobs(); @@ -493,79 +540,12 @@ class Solver $this->decisionMap = array_fill(0, $this->pool->getMaxId() + 1, 0); } - foreach ($this->jobs as $job) { - foreach ($job['packages'] as $package) { - switch ($job['cmd']) { - case 'update': - if (isset($this->installedMap[$package->getId()])) { - $this->updateMap[$package->getId()] = true; - } - break; - } - } - - switch ($job['cmd']) { - case 'update-all': - foreach ($this->installedMap as $package) { - $this->updateMap[$package->getId()] = true; - } - break; - } - } - foreach ($this->installedMap as $package) { $this->addRulesForPackage($package); $this->addRulesForUpdatePackages($package); } - foreach ($this->jobs as $job) { - foreach ($job['packages'] as $package) { - switch ($job['cmd']) { - case 'install': - $this->installCandidateMap[$package->getId()] = true; - $this->addRulesForPackage($package); - break; - } - } - } - - foreach ($this->jobs as $job) { - switch ($job['cmd']) { - case 'install': - if (empty($job['packages'])) { - $problem = new Problem(); - $problem->addJobRule($job); - $this->problems[] = $problem; - } else { - $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job['packageName']); - $this->addRule(RuleSet::TYPE_JOB, $rule); - $this->ruleToJob[$rule->getId()] = $job; - } - break; - case 'remove': - // remove all packages with this name including uninstalled - // ones to make sure none of them are picked as replacements - - // todo: cleandeps - foreach ($job['packages'] as $package) { - $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE); - $this->addRule(RuleSet::TYPE_JOB, $rule); - $this->ruleToJob[$rule->getId()] = $job; - } - break; - case 'lock': - foreach ($job['packages'] as $package) { - if (isset($this->installedMap[$package->getId()])) { - $rule = $this->createInstallRule($package, Rule::RULE_JOB_LOCK); - } else { - $rule = $this->createRemoveRule($package, Rule::RULE_JOB_LOCK); - } - $this->addRule(RuleSet::TYPE_JOB, $rule); - $this->ruleToJob[$rule->getId()] = $job; - } - break; - } - } + $this->addRulesForJobs(); foreach ($this->rules as $rule) { $this->addWatchesToRule($rule); From 83c499cf29223cd729a4985580ec5170370598e2 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 20:29:21 +0200 Subject: [PATCH 07/27] Remove unecessary use statement in Transaction class --- src/Composer/DependencyResolver/Transaction.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php index 14fe11836..79235e5cb 100644 --- a/src/Composer/DependencyResolver/Transaction.php +++ b/src/Composer/DependencyResolver/Transaction.php @@ -12,8 +12,6 @@ namespace Composer\DependencyResolver; -use Composer\Repository\RepositoryInterface; -use Composer\Package\PackageInterface; use Composer\Package\AliasPackage; use Composer\DependencyResolver\Operation; From 46e4ae0e6a320466c32de28fcb824aaeee78d980 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 21:36:47 +0200 Subject: [PATCH 08/27] Move rule generation from solver into separate rule set generator --- src/Composer/DependencyResolver/Problem.php | 16 +- src/Composer/DependencyResolver/Rule.php | 11 +- .../DependencyResolver/RuleSetGenerator.php | 306 ++++++++++++++++ src/Composer/DependencyResolver/Solver.php | 342 ++---------------- 4 files changed, 342 insertions(+), 333 deletions(-) create mode 100644 src/Composer/DependencyResolver/RuleSetGenerator.php diff --git a/src/Composer/DependencyResolver/Problem.php b/src/Composer/DependencyResolver/Problem.php index 8ac2ed4bb..b9266bae2 100644 --- a/src/Composer/DependencyResolver/Problem.php +++ b/src/Composer/DependencyResolver/Problem.php @@ -25,20 +25,6 @@ class Problem */ protected $reasons; - /** - * Add a job as a reason - * - * @param array $job A job descriptor which is a reason for this problem - * @param Rule $rule An optional rule associated with the job - */ - public function addJobRule($job, Rule $rule = null) - { - $this->addReason(serialize($job), array( - 'rule' => $rule, - 'job' => $job, - )); - } - /** * Add a rule as a reason * @@ -48,7 +34,7 @@ class Problem { $this->addReason($rule->getId(), array( 'rule' => $rule, - 'job' => null, + 'job' => $rule->getJob(), )); } diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index a7248f65a..f96d62300 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,6 +35,8 @@ class Rule protected $id; protected $weak; + protected $job; + public $watch1; public $watch2; @@ -43,7 +45,7 @@ class Rule public $ruleHash; - public function __construct(array $literals, $reason, $reasonData) + public function __construct(array $literals, $reason, $reasonData, $job = null) { // sort all packages ascending by id usort($literals, array($this, 'compareLiteralsById')); @@ -55,6 +57,8 @@ class Rule $this->disabled = false; $this->weak = false; + $this->job = $job; + $this->watch1 = (count($this->literals) > 0) ? $literals[0]->getId() : 0; $this->watch2 = (count($this->literals) > 1) ? $literals[1]->getId() : 0; @@ -80,6 +84,11 @@ class Rule return $this->id; } + public function getJob() + { + return $this->job; + } + /** * Checks if this rule is equal to another one * diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php new file mode 100644 index 000000000..e95edc77d --- /dev/null +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -0,0 +1,306 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +use Composer\Repository\RepositoryInterface; +use Composer\Package\PackageInterface; +use Composer\Package\AliasPackage; +use Composer\DependencyResolver\Operation; + +/** + * @author Nils Adermann + */ +class RuleSetGenerator +{ + protected $policy; + protected $pool; + protected $rules; + protected $jobs; + protected $installedMap; + + public function __construct(PolicyInterface $policy, Pool $pool) + { + $this->policy = $policy; + $this->pool = $pool; + } + + /** + * Creates a new rule for the requirements of a package + * + * This rule is of the form (-A|B|C), where B and C are the providers of + * one requirement of the package A. + * + * @param PackageInterface $package The package with a requirement + * @param array $providers The providers of the requirement + * @param int $reason A RULE_* constant describing the + * reason for generating this rule + * @param mixed $reasonData Any data, e.g. the requirement name, + * that goes with the reason + * @return Rule The generated rule or null if tautological + */ + protected function createRequireRule(PackageInterface $package, array $providers, $reason, $reasonData = null) + { + $literals = array(new Literal($package, false)); + + foreach ($providers as $provider) { + // self fulfilling rule? + if ($provider === $package) { + return null; + } + $literals[] = new Literal($provider, true); + } + + return new Rule($literals, $reason, $reasonData); + } + + /** + * Creates a new rule for installing a package + * + * The rule is simply (A) for a package A to be installed. + * + * @param PackageInterface $package The package to be installed + * @param int $reason A RULE_* constant describing the + * reason for generating this rule + * @param mixed $reasonData Any data, e.g. the package name, that + * goes with the reason + * @return Rule The generated rule + */ + protected function createInstallRule(PackageInterface $package, $reason, $reasonData = null) + { + return new Rule(new Literal($package, true)); + } + + /** + * Creates a rule to install at least one of a set of packages + * + * The rule is (A|B|C) with A, B and C different packages. If the given + * set of packages is empty an impossible rule is generated. + * + * @param array $packages The set of packages to choose from + * @param int $reason A RULE_* constant describing the reason for + * generating this rule + * @param array $job The job this rule was created from + * @return Rule The generated rule + */ + protected function createInstallOneOfRule(array $packages, $reason, $job) + { + $literals = array(); + foreach ($packages as $package) { + $literals[] = new Literal($package, true); + } + + return new Rule($literals, $reason, $job['packageName'], $job); + } + + /** + * Creates a rule to remove a package + * + * The rule for a package A is (-A). + * + * @param PackageInterface $package The package to be removed + * @param int $reason A RULE_* constant describing the + * reason for generating this rule + * @param array $job The job this rule was created from + * @return Rule The generated rule + */ + protected function createRemoveRule(PackageInterface $package, $reason, $job) + { + return new Rule(array(new Literal($package, false)), $reason, $job['packageName'], $job); + } + + /** + * Creates a rule for two conflicting packages + * + * The rule for conflicting packages A and B is (-A|-B). A is called the issuer + * and B the provider. + * + * @param PackageInterface $issuer The package declaring the conflict + * @param Package $provider The package causing the conflict + * @param int $reason A RULE_* constant describing the + * reason for generating this rule + * @param mixed $reasonData Any data, e.g. the package name, that + * goes with the reason + * @return Rule The generated rule + */ + protected function createConflictRule(PackageInterface $issuer, PackageInterface $provider, $reason, $reasonData = null) + { + // ignore self conflict + if ($issuer === $provider) { + return null; + } + + return new Rule(array(new Literal($issuer, false), new Literal($provider, false)), $reason, $reasonData); + } + + /** + * Adds a rule unless it duplicates an existing one of any type + * + * To be able to directly pass in the result of one of the rule creation + * methods. + * + * @param int $type A TYPE_* constant defining the rule type + * @param Rule $newRule The rule about to be added + */ + private function addRule($type, Rule $newRule = null) { + if ($this->rules->containsEqual($newRule)) { + return; + } + + $this->rules->add($newRule, $type); + } + + protected function addRulesForPackage(PackageInterface $package) + { + $workQueue = new \SplQueue; + $workQueue->enqueue($package); + + while (!$workQueue->isEmpty()) { + $package = $workQueue->dequeue(); + if (isset($this->addedMap[$package->getId()])) { + continue; + } + + $this->addedMap[$package->getId()] = true; + + foreach ($package->getRequires() as $link) { + $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, $possibleRequires, Rule::RULE_PACKAGE_REQUIRES, (string) $link)); + + foreach ($possibleRequires as $require) { + $workQueue->enqueue($require); + } + } + + foreach ($package->getConflicts() as $link) { + $possibleConflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + foreach ($possibleConflicts as $conflict) { + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createConflictRule($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, (string) $link)); + } + } + + // check obsoletes and implicit obsoletes of a package + $isInstalled = (isset($this->installedMap[$package->getId()])); + + foreach ($package->getReplaces() as $link) { + $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); + + foreach ($obsoleteProviders as $provider) { + if ($provider === $package) { + continue; + } + + if (!$this->obsoleteImpossibleForAlias($package, $provider)) { + $reason = ($isInstalled) ? Rule::RULE_INSTALLED_PACKAGE_OBSOLETES : Rule::RULE_PACKAGE_OBSOLETES; + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createConflictRule($package, $provider, $reason, (string) $link)); + } + } + } + + // check implicit obsoletes + // for installed packages we only need to check installed/installed problems, + // as the others are picked up when looking at the uninstalled package. + if (!$isInstalled) { + $obsoleteProviders = $this->pool->whatProvides($package->getName(), null); + + foreach ($obsoleteProviders as $provider) { + if ($provider === $package) { + continue; + } + + if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) { + $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, (string) $package)); + } else if (!$this->obsoleteImpossibleForAlias($package, $provider)) { + $reason = ($package->getName() == $provider->getName()) ? Rule::RULE_PACKAGE_SAME_NAME : Rule::RULE_PACKAGE_IMPLICIT_OBSOLETES; + $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createConflictRule($package, $provider, $reason, (string) $package)); + } + } + } + } + } + + protected function obsoleteImpossibleForAlias($package, $provider) + { + $packageIsAlias = $package instanceof AliasPackage; + $providerIsAlias = $provider instanceof AliasPackage; + + $impossible = ( + ($packageIsAlias && $package->getAliasOf() === $provider) || + ($providerIsAlias && $provider->getAliasOf() === $package) || + ($packageIsAlias && $providerIsAlias && $provider->getAliasOf() === $package->getAliasOf()) + ); + + return $impossible; + } + + /** + * Adds all rules for all update packages of a given package + * + * @param PackageInterface $package Rules for this package's updates are to + * be added + * @param bool $allowAll Whether downgrades are allowed + */ + private function addRulesForUpdatePackages(PackageInterface $package) + { + $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); + + foreach ($updates as $update) { + $this->addRulesForPackage($update); + } + } + + protected function addRulesForJobs() + { + foreach ($this->jobs as $job) { + switch ($job['cmd']) { + case 'install': + if ($job['packages']) { + foreach ($job['packages'] as $package) { + if (!isset($this->installedMap[$package->getId()])) { + $this->addRulesForPackage($package); + } + } + + $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job); + $this->addRule(RuleSet::TYPE_JOB, $rule); + } + break; + case 'remove': + // remove all packages with this name including uninstalled + // ones to make sure none of them are picked as replacements + foreach ($job['packages'] as $package) { + $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE, $job); + $this->addRule(RuleSet::TYPE_JOB, $rule); + } + break; + } + } + } + + public function getRulesFor($jobs, $installedMap) + { + $this->jobs = $jobs; + $this->rules = new RuleSet; + $this->installedMap = $installedMap; + + foreach ($this->installedMap as $package) { + $this->addRulesForPackage($package); + $this->addRulesForUpdatePackages($package); + } + + $this->addRulesForJobs(); + + return $this->rules; + } +} diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index a447479e9..faa97bd14 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -26,9 +26,9 @@ class Solver protected $pool; protected $installed; protected $rules; + protected $ruleSetGenerator; protected $updateAll; - protected $ruleToJob = array(); protected $addedMap = array(); protected $updateMap = array(); protected $watches = array(); @@ -48,238 +48,7 @@ class Solver $this->policy = $policy; $this->pool = $pool; $this->installed = $installed; - $this->rules = new RuleSet; - } - - /** - * Creates a new rule for the requirements of a package - * - * This rule is of the form (-A|B|C), where B and C are the providers of - * one requirement of the package A. - * - * @param PackageInterface $package The package with a requirement - * @param array $providers The providers of the requirement - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the requirement name, - * that goes with the reason - * @return Rule The generated rule or null if tautological - */ - protected function createRequireRule(PackageInterface $package, array $providers, $reason, $reasonData = null) - { - $literals = array(new Literal($package, false)); - - foreach ($providers as $provider) { - // self fulfilling rule? - if ($provider === $package) { - return null; - } - $literals[] = new Literal($provider, true); - } - - return new Rule($literals, $reason, $reasonData); - } - - /** - * Creates a new rule for installing a package - * - * The rule is simply (A) for a package A to be installed. - * - * @param PackageInterface $package The package to be installed - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that - * goes with the reason - * @return Rule The generated rule - */ - protected function createInstallRule(PackageInterface $package, $reason, $reasonData = null) - { - return new Rule(new Literal($package, true)); - } - - /** - * Creates a rule to install at least one of a set of packages - * - * The rule is (A|B|C) with A, B and C different packages. If the given - * set of packages is empty an impossible rule is generated. - * - * @param array $packages The set of packages to choose from - * @param int $reason A RULE_* constant describing the reason for - * generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that goes with - * the reason - * @return Rule The generated rule - */ - protected function createInstallOneOfRule(array $packages, $reason, $reasonData = null) - { - $literals = array(); - foreach ($packages as $package) { - $literals[] = new Literal($package, true); - } - - return new Rule($literals, $reason, $reasonData); - } - - /** - * Creates a rule to remove a package - * - * The rule for a package A is (-A). - * - * @param PackageInterface $package The package to be removed - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that - * goes with the reason - * @return Rule The generated rule - */ - protected function createRemoveRule(PackageInterface $package, $reason, $reasonData = null) - { - return new Rule(array(new Literal($package, false)), $reason, $reasonData); - } - - /** - * Creates a rule for two conflicting packages - * - * The rule for conflicting packages A and B is (-A|-B). A is called the issuer - * and B the provider. - * - * @param PackageInterface $issuer The package declaring the conflict - * @param Package $provider The package causing the conflict - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that - * goes with the reason - * @return Rule The generated rule - */ - protected function createConflictRule(PackageInterface $issuer, PackageInterface $provider, $reason, $reasonData = null) - { - // ignore self conflict - if ($issuer === $provider) { - return null; - } - - return new Rule(array(new Literal($issuer, false), new Literal($provider, false)), $reason, $reasonData); - } - - /** - * Adds a rule unless it duplicates an existing one of any type - * - * To be able to directly pass in the result of one of the rule creation - * methods the rule may also be null to indicate that no rule should be - * added. - * - * @param int $type A TYPE_* constant defining the rule type - * @param Rule $newRule The rule about to be added - */ - private function addRule($type, Rule $newRule = null) { - if ($newRule) { - if ($this->rules->containsEqual($newRule)) { - return; - } - - $this->rules->add($newRule, $type); - } - } - - protected function addRulesForPackage(PackageInterface $package) - { - $workQueue = new \SplQueue; - $workQueue->enqueue($package); - - while (!$workQueue->isEmpty()) { - $package = $workQueue->dequeue(); - if (isset($this->addedMap[$package->getId()])) { - continue; - } - - $this->addedMap[$package->getId()] = true; - - foreach ($package->getRequires() as $link) { - $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); - - $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, $possibleRequires, Rule::RULE_PACKAGE_REQUIRES, (string) $link)); - - foreach ($possibleRequires as $require) { - $workQueue->enqueue($require); - } - } - - foreach ($package->getConflicts() as $link) { - $possibleConflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); - - foreach ($possibleConflicts as $conflict) { - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createConflictRule($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, (string) $link)); - } - } - - // check obsoletes and implicit obsoletes of a package - $isInstalled = (isset($this->installedMap[$package->getId()])); - - foreach ($package->getReplaces() as $link) { - $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint()); - - foreach ($obsoleteProviders as $provider) { - if ($provider === $package) { - continue; - } - - if (!$this->obsoleteImpossibleForAlias($package, $provider)) { - $reason = ($isInstalled) ? Rule::RULE_INSTALLED_PACKAGE_OBSOLETES : Rule::RULE_PACKAGE_OBSOLETES; - $this->addRule(RuleSet::TYPE_PACKAGE, $this->createConflictRule($package, $provider, $reason, (string) $link)); - } - } - } - - // check implicit obsoletes - // for installed packages we only need to check installed/installed problems, - // as the others are picked up when looking at the uninstalled package. - if (!$isInstalled) { - $obsoleteProviders = $this->pool->whatProvides($package->getName(), null); - - foreach ($obsoleteProviders as $provider) { - if ($provider === $package) { - continue; - } - - if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) { - $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, (string) $package)); - } else if (!$this->obsoleteImpossibleForAlias($package, $provider)) { - $reason = ($package->getName() == $provider->getName()) ? Rule::RULE_PACKAGE_SAME_NAME : Rule::RULE_PACKAGE_IMPLICIT_OBSOLETES; - $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createConflictRule($package, $provider, $reason, (string) $package)); - } - } - } - } - } - - protected function obsoleteImpossibleForAlias($package, $provider) - { - $packageIsAlias = $package instanceof AliasPackage; - $providerIsAlias = $provider instanceof AliasPackage; - - $impossible = ( - ($packageIsAlias && $package->getAliasOf() === $provider) || - ($providerIsAlias && $provider->getAliasOf() === $package) || - ($packageIsAlias && $providerIsAlias && $provider->getAliasOf() === $package->getAliasOf()) - ); - - return $impossible; - } - - /** - * Adds all rules for all update packages of a given package - * - * @param PackageInterface $package Rules for this package's updates are to - * be added - * @param bool $allowAll Whether downgrades are allowed - */ - private function addRulesForUpdatePackages(PackageInterface $package) - { - $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); - - foreach ($updates as $update) { - $this->addRulesForPackage($update); - } + $this->ruleSetGenerator = new RuleSetGenerator($policy, $pool); } /** @@ -387,17 +156,9 @@ class Solver $problem = new Problem; - if ($rule->getType() == RuleSet::TYPE_JOB) { - $job = $this->ruleToJob[$rule->getId()]; - - $problem->addJobRule($job, $rule); - $problem->addRule($conflict); - $this->disableProblem($job); - } else { - $problem->addRule($rule); - $problem->addRule($conflict); - $this->disableProblem($rule); - } + $problem->addRule($rule); + $problem->addRule($conflict); + $this->disableProblem($rule); $this->problems[] = $problem; continue; } @@ -421,15 +182,8 @@ class Solver continue; } - if ($assertRule->getType() === RuleSet::TYPE_JOB) { - $job = $this->ruleToJob[$assertRule->getId()]; - - $problem->addJobRule($job, $assertRule); - $this->disableProblem($job); - } else { - $problem->addRule($assertRule); - $this->disableProblem($assertRule); - } + $problem->addRule($assertRule); + $this->disableProblem($assertRule); } $this->problems[] = $problem; @@ -463,13 +217,7 @@ class Solver } // conflict, but this is a weak rule => disable - if ($rule->getType() == RuleSet::TYPE_JOB) { - $why = $this->ruleToJob[$rule->getId()]; - } else { - $why = $rule; - } - - $this->disableProblem($why); + $this->disableProblem($rule); } } @@ -479,10 +227,7 @@ class Solver foreach ($this->installed->getPackages() as $package) { $this->installedMap[$package->getId()] = $package; } - } - protected function addRulesForJobs() - { foreach ($this->jobs as $job) { switch ($job['cmd']) { case 'update': @@ -498,30 +243,12 @@ class Solver $this->updateMap[$package->getId()] = true; } break; - case 'install': - if (empty($job['packages'])) { - $problem = new Problem(); - $problem->addJobRule($job); - $this->problems[] = $problem; - } else { - foreach ($job['packages'] as $package) { - if (!isset($this->installedMap[$package->getId()])) { - $this->addRulesForPackage($package); - } - } - $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job['packageName']); - $this->addRule(RuleSet::TYPE_JOB, $rule); - $this->ruleToJob[$rule->getId()] = $job; - } - break; - case 'remove': - // remove all packages with this name including uninstalled - // ones to make sure none of them are picked as replacements - foreach ($job['packages'] as $package) { - $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE); - $this->addRule(RuleSet::TYPE_JOB, $rule); - $this->ruleToJob[$rule->getId()] = $job; + case 'install': + if (!$job['packages']) { + $problem = new Problem(); + $problem->addRule(new Rule(array(), null, null, $job)); + $this->problems[] = $problem; } break; } @@ -540,12 +267,7 @@ class Solver $this->decisionMap = array_fill(0, $this->pool->getMaxId() + 1, 0); } - foreach ($this->installedMap as $package) { - $this->addRulesForPackage($package); - $this->addRulesForUpdatePackages($package); - } - - $this->addRulesForJobs(); + $this->rules = $this->ruleSetGenerator->getRulesFor($this->jobs, $this->installedMap); foreach ($this->rules as $rule) { $this->addWatchesToRule($rule); @@ -827,7 +549,7 @@ class Solver $this->revert($level); - $this->addRule(RuleSet::TYPE_LEARNED, $newRule); + $this->rules->add($newRule, RuleSet::TYPE_LEARNED); $this->learnedWhy[$newRule->getId()] = $why; @@ -987,12 +709,7 @@ class Solver } } - if ($conflictRule->getType() == RuleSet::TYPE_JOB) { - $job = $this->ruleToJob[$conflictRule->getId()]; - $problem->addJobRule($job, $conflictRule); - } else { - $problem->addRule($conflictRule); - } + $problem->addRule($conflictRule); } private function analyzeUnsolvable($conflictRule, $disableRules) @@ -1058,13 +775,7 @@ class Solver if ($lastWeakWhy) { array_pop($this->problems); - if ($lastWeakWhy->getType() === RuleSet::TYPE_JOB) { - $why = $this->ruleToJob[$lastWeakWhy]; - } else { - $why = $lastWeakWhy; - } - - $this->disableProblem($why); + $this->disableProblem($lastWeakWhy); $this->resetSolver(); return 1; @@ -1072,11 +783,7 @@ class Solver if ($disableRules) { foreach ($this->problems[count($this->problems) - 1] as $reason) { - if ($reason['job']) { - $this->disableProblem($reason['job']); - } else { - $this->disableProblem($reason['rule']); - } + $this->disableProblem($reason['rule']); } $this->resetSolver(); @@ -1088,14 +795,15 @@ class Solver private function disableProblem($why) { - if ($why instanceof Rule) { - $why->disable(); - } else if (is_array($why)) { + $job = $why->getJob(); + if (!$job) { + $why->disable(); + } else { // disable all rules of this job - foreach ($this->ruleToJob as $ruleId => $job) { - if ($why === $job) { - $this->rules->ruleById($ruleId)->disable(); + foreach ($this->rules as $rule) { + if ($job === $rule->getJob()) { + $rule->disable(); } } } From 286593cf999ecbc2e27ff2517f0fe65a1ca86436 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 21:37:57 +0200 Subject: [PATCH 09/27] Move solver debugging code into solver subclass --- .../DependencyResolver/DebugSolver.php | 83 +++++++++++++++++++ src/Composer/DependencyResolver/Solver.php | 64 -------------- 2 files changed, 83 insertions(+), 64 deletions(-) create mode 100644 src/Composer/DependencyResolver/DebugSolver.php diff --git a/src/Composer/DependencyResolver/DebugSolver.php b/src/Composer/DependencyResolver/DebugSolver.php new file mode 100644 index 000000000..b3ff0ca63 --- /dev/null +++ b/src/Composer/DependencyResolver/DebugSolver.php @@ -0,0 +1,83 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +/** + * @author Nils Adermann + */ +class DebugSolver extends Solver +{ + private function printDecisionMap() + { + echo "\nDecisionMap: \n"; + foreach ($this->decisionMap as $packageId => $level) { + if ($packageId === 0) { + continue; + } + if ($level > 0) { + echo ' +' . $this->pool->packageById($packageId)."\n"; + } elseif ($level < 0) { + echo ' -' . $this->pool->packageById($packageId)."\n"; + } else { + echo ' ?' . $this->pool->packageById($packageId)."\n"; + } + } + echo "\n"; + } + + private function printDecisionQueue() + { + echo "DecisionQueue: \n"; + foreach ($this->decisionQueue as $i => $literal) { + echo ' ' . $literal . ' ' . $this->decisionQueueWhy[$i]." level ".$this->decisionMap[$literal->getPackageId()]."\n"; + } + echo "\n"; + } + + private function printWatches() + { + echo "\nWatches:\n"; + foreach ($this->watches as $literalId => $watch) { + echo ' '.$this->literalFromId($literalId)."\n"; + $queue = array(array(' ', $watch)); + + while (!empty($queue)) { + list($indent, $watch) = array_pop($queue); + + echo $indent.$watch; + + if ($watch) { + echo ' [id='.$watch->getId().',watch1='.$this->literalFromId($watch->watch1).',watch2='.$this->literalFromId($watch->watch2)."]"; + } + + echo "\n"; + + if ($watch && ($watch->next1 == $watch || $watch->next2 == $watch)) { + if ($watch->next1 == $watch) { + echo $indent." 1 *RECURSION*"; + } + if ($watch->next2 == $watch) { + echo $indent." 2 *RECURSION*"; + } + } elseif ($watch && ($watch->next1 || $watch->next2)) { + $indent = str_replace(array('1', '2'), ' ', $indent); + + array_push($queue, array($indent.' 2 ', $watch->next2)); + array_push($queue, array($indent.' 1 ', $watch->next1)); + } + } + + echo "\n"; + } + } +} diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index faa97bd14..8e77264db 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -1050,68 +1050,4 @@ class Solver break; } } - - private function printDecisionMap() - { - echo "\nDecisionMap: \n"; - foreach ($this->decisionMap as $packageId => $level) { - if ($packageId === 0) { - continue; - } - if ($level > 0) { - echo ' +' . $this->pool->packageById($packageId)."\n"; - } elseif ($level < 0) { - echo ' -' . $this->pool->packageById($packageId)."\n"; - } else { - echo ' ?' . $this->pool->packageById($packageId)."\n"; - } - } - echo "\n"; - } - - private function printDecisionQueue() - { - echo "DecisionQueue: \n"; - foreach ($this->decisionQueue as $i => $literal) { - echo ' ' . $literal . ' ' . $this->decisionQueueWhy[$i]." level ".$this->decisionMap[$literal->getPackageId()]."\n"; - } - echo "\n"; - } - - private function printWatches() - { - echo "\nWatches:\n"; - foreach ($this->watches as $literalId => $watch) { - echo ' '.$this->literalFromId($literalId)."\n"; - $queue = array(array(' ', $watch)); - - while (!empty($queue)) { - list($indent, $watch) = array_pop($queue); - - echo $indent.$watch; - - if ($watch) { - echo ' [id='.$watch->getId().',watch1='.$this->literalFromId($watch->watch1).',watch2='.$this->literalFromId($watch->watch2)."]"; - } - - echo "\n"; - - if ($watch && ($watch->next1 == $watch || $watch->next2 == $watch)) { - if ($watch->next1 == $watch) { - echo $indent." 1 *RECURSION*"; - } - if ($watch->next2 == $watch) { - echo $indent." 2 *RECURSION*"; - } - } elseif ($watch && ($watch->next1 || $watch->next2)) { - $indent = str_replace(array('1', '2'), ' ', $indent); - - array_push($queue, array($indent.' 2 ', $watch->next2)); - array_push($queue, array($indent.' 1 ', $watch->next1)); - } - } - - echo "\n"; - } - } } From 71ee5c8f4c0846bdbd4b34c208d03a38ee411d53 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 15 May 2012 21:46:52 +0200 Subject: [PATCH 10/27] We don't have a systemsolvable at the lowest level --- src/Composer/DependencyResolver/Solver.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 8e77264db..255f9308f 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -118,8 +118,6 @@ class Solver // aka solver_makeruledecisions private function makeAssertionRuleDecisions() { - // do we need to decide a SYSTEMSOLVABLE at level 1? - $decisionStart = count($this->decisionQueue); for ($ruleIndex = 0; $ruleIndex < count($this->rules); $ruleIndex++) { @@ -150,7 +148,6 @@ class Solver } $conflict = $this->findDecisionRule($literal->getPackage()); - /** TODO: handle conflict with systemsolvable? */ if ($conflict && RuleSet::TYPE_PACKAGE === $conflict->getType()) { From 731a451dfebf0f6ddfc945b23cf0e7fced41e0b5 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 01:27:57 +0200 Subject: [PATCH 11/27] Move handling of watch graph to separate classes --- .../DependencyResolver/RuleWatchGraph.php | 134 ++++++++++++++++ .../DependencyResolver/RuleWatchNode.php | 84 ++++++++++ src/Composer/DependencyResolver/Solver.php | 144 +++--------------- 3 files changed, 242 insertions(+), 120 deletions(-) create mode 100644 src/Composer/DependencyResolver/RuleWatchGraph.php create mode 100644 src/Composer/DependencyResolver/RuleWatchNode.php diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php new file mode 100644 index 000000000..df736086e --- /dev/null +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -0,0 +1,134 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +/** + * @author Nils Adermann + */ +class RuleWatchGraph +{ + protected $watches = array(); + + /** + * Alters watch chains for a rule. + * + * Next1/2 always points to the next rule that is watching the same package. + * The watches array contains rules to start from for each package + * + */ + public function insert(RuleWatchNode $node) + { + // skip simple assertions of the form (A) or (-A) + if ($node->getRule()->isAssertion()) { + return; + } + + if (!isset($this->watches[$node->watch1])) { + $this->watches[$node->watch1] = null; + } + + $node->next1 = $this->watches[$node->watch1]; + $this->watches[$node->watch1] = $node; + + if (!isset($this->watches[$node->watch2])) { + $this->watches[$node->watch2] = null; + } + + $node->next2 = $this->watches[$node->watch2]; + $this->watches[$node->watch2] = $node; + } + + public function contains($literalId) + { + return isset($this->watches[$literalId]); + } + + public function walkLiteral($literalId, $level, $skipCallback, $conflictCallback, $decideCallback) + { + if (!isset($this->watches[$literalId])) { + return; + } + + $prevNode = null; + for ($node = $this->watches[$literalId]; $node !== null; $prevNode = $node, $node = $nextNode) { + $nextNode = $node->getNext($literalId); + + if ($node->getRule()->isDisabled()) { + continue; + } + + $otherWatch = $node->getOtherWatch($literalId); + + if (call_user_func($skipCallback, $otherWatch)) { + continue; + } + + $ruleLiterals = $node->getRule()->getLiterals(); + + if (sizeof($ruleLiterals) > 2) { + foreach ($ruleLiterals as $ruleLiteral) { + if ($otherWatch !== $ruleLiteral->getId() && + !call_user_func($conflictCallback, $ruleLiteral->getId())) { + + $node = $this->moveWatch($literalId, $ruleLiteral->getId(), $prevNode, $node, $nextNode); + + continue 2; + } + } + } + + // yay, we found a unit clause! try setting it to true + if (call_user_func($conflictCallback, $otherWatch)) { + return $node->getRule(); + } + + call_user_func($decideCallback, $otherWatch, $level, $node->getRule()); + } + + return null; + } + + public function moveWatch($fromLiteral, $toLiteral, $prevNode, $node, $nextNode) { + if ($fromLiteral == $node->watch1) { + $node->watch1 = $toLiteral; + $node->next1 = (isset($this->watches[$toLiteral])) ? $this->watches[$toLiteral] : null; + } else { + $node->watch2 = $toLiteral; + $node->next2 = (isset($this->watches[$toLiteral])) ? $this->watches[$toLiteral] : null; + } + + if ($prevNode) { + if ($prevNode->next1 === $node) { + $prevNode->next1 = $nextNode; + } else { + $prevNode->next2 = $nextNode; + } + } else { + $this->watches[$fromLiteral] = $nextNode; + } + + $this->watches[$toLiteral] = $node; + + if ($prevNode) { + return $prevNode; + } + + $tmpNode = new RuleWatchNode(new Rule(array(), null, null)); + $tmpNode->watch1 = $fromLiteral; + $tmpNode->next1 = $nextNode; + $tmpNode->watch2 = $fromLiteral; + $tmpNode->next2 = $nextNode; + + return $tmpNode; + } +} diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php new file mode 100644 index 000000000..c1589bdb0 --- /dev/null +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -0,0 +1,84 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +/** + * @author Nils Adermann + */ +class RuleWatchNode +{ + protected $rule; + + public $watch1; + public $watch2; + + public $next1; + public $next2; + + public function __construct($rule) + { + $this->rule = $rule; + + $literals = $rule->getLiterals(); + + $this->watch1 = (count($literals) > 0) ? $literals[0]->getId() : 0; + $this->watch2 = (count($literals) > 1) ? $literals[1]->getId() : 0; + } + + /** + * Put watch2 on rule's literal with highest level + */ + public function watch2OnHighest($decisionMap) + { + $literals = $this->rule->getLiterals(); + + // if there are only 2 elements, both are being watched anyway + if ($literals < 3) { + return; + } + + $watchLevel = 0; + + foreach ($literals as $literal) { + $level = abs($decisionMap[$literal->getPackageId()]); + + if ($level > $watchLevel) { + $this->rule->watch2 = $literal->getId(); + $watchLevel = $level; + } + } + } + + public function getRule() + { + return $this->rule; + } + + public function getNext($literalId) + { + if ($this->watch1 == $literalId) { + return $this->next1; + } else { + return $this->next2; + } + } + + public function getOtherWatch($literalId) + { + if ($this->watch1 == $literalId) { + return $this->watch2; + } else { + return $this->watch1; + } + } +} diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 255f9308f..0e6f094ab 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -31,7 +31,7 @@ class Solver protected $addedMap = array(); protected $updateMap = array(); - protected $watches = array(); + protected $watchGraph; protected $decisionMap; protected $installedMap; @@ -51,59 +51,6 @@ class Solver $this->ruleSetGenerator = new RuleSetGenerator($policy, $pool); } - /** - * Alters watch chains for a rule. - * - * Next1/2 always points to the next rule that is watching the same package. - * The watches array contains rules to start from for each package - * - */ - private function addWatchesToRule(Rule $rule) - { - // skip simple assertions of the form (A) or (-A) - if ($rule->isAssertion()) { - return; - } - - if (!isset($this->watches[$rule->watch1])) { - $this->watches[$rule->watch1] = null; - } - - $rule->next1 = $this->watches[$rule->watch1]; - $this->watches[$rule->watch1] = $rule; - - if (!isset($this->watches[$rule->watch2])) { - $this->watches[$rule->watch2] = null; - } - - $rule->next2 = $this->watches[$rule->watch2]; - $this->watches[$rule->watch2] = $rule; - } - - /** - * Put watch2 on rule's literal with highest level - */ - private function watch2OnHighest(Rule $rule) - { - $literals = $rule->getLiterals(); - - // if there are only 2 elements, both are being watched anyway - if ($literals < 3) { - return; - } - - $watchLevel = 0; - - foreach ($literals as $literal) { - $level = abs($this->decisionMap[$literal->getPackageId()]); - - if ($level > $watchLevel) { - $rule->watch2 = $literal->getId(); - $watchLevel = $level; - } - } - } - private function findDecisionRule(PackageInterface $package) { foreach ($this->decisionQueue as $i => $literal) { @@ -265,9 +212,10 @@ class Solver } $this->rules = $this->ruleSetGenerator->getRulesFor($this->jobs, $this->installedMap); + $this->watchGraph = new RuleWatchGraph; foreach ($this->rules as $rule) { - $this->addWatchesToRule($rule); + $this->watchGraph->insert(new RuleWatchNode($rule)); } /* make decisions based on job/update assertions */ @@ -313,6 +261,13 @@ class Solver } } + public function decide($literal, $level, $why) + { + $this->addDecisionId($literal, $level); + $this->decisionQueue[] = $this->literalFromId($literal); + $this->decisionQueueWhy[] = $why; + } + protected function decisionsContain(Literal $l) { return ( @@ -321,7 +276,7 @@ class Solver ); } - protected function decisionsContainId($literalId) + public function decisionsContainId($literalId) { $packageId = abs($literalId); return ( @@ -344,7 +299,7 @@ class Solver ); } - protected function decisionsConflictId($literalId) + public function decisionsConflictId($literalId) { $packageId = abs($literalId); return ( @@ -390,68 +345,16 @@ class Solver $this->propagateIndex++; - // /* foreach rule where 'pkg' is now FALSE */ - //for (rp = watches + pkg; *rp; rp = next_rp) - if (!isset($this->watches[$literal->getId()])) { - continue; - } + $conflict = $this->watchGraph->walkLiteral( + $literal->getId(), + $level, + array($this, 'decisionsContainId'), + array($this, 'decisionsConflictId'), + array($this, 'decide') + ); - $prevRule = null; - for ($rule = $this->watches[$literal->getId()]; $rule !== null; $prevRule = $rule, $rule = $nextRule) { - $nextRule = $rule->getNext($literal); - - if ($rule->isDisabled()) { - continue; - } - - $otherWatch = $rule->getOtherWatch($literal); - - if ($this->decisionsContainId($otherWatch)) { - continue; - } - - $ruleLiterals = $rule->getLiterals(); - - if (sizeof($ruleLiterals) > 2) { - foreach ($ruleLiterals as $ruleLiteral) { - if ($otherWatch !== $ruleLiteral->getId() && - !$this->decisionsConflict($ruleLiteral)) { - - if ($literal->getId() === $rule->watch1) { - $rule->watch1 = $ruleLiteral->getId(); - $rule->next1 = (isset($this->watches[$ruleLiteral->getId()])) ? $this->watches[$ruleLiteral->getId()] : null; - } else { - $rule->watch2 = $ruleLiteral->getId(); - $rule->next2 = (isset($this->watches[$ruleLiteral->getId()])) ? $this->watches[$ruleLiteral->getId()] : null; - } - - if ($prevRule) { - if ($prevRule->next1 == $rule) { - $prevRule->next1 = $nextRule; - } else { - $prevRule->next2 = $nextRule; - } - } else { - $this->watches[$literal->getId()] = $nextRule; - } - - $this->watches[$ruleLiteral->getId()] = $rule; - - $rule = $prevRule; - continue 2; - } - } - } - - // yay, we found a unit clause! try setting it to true - if ($this->decisionsConflictId($otherWatch)) { - return $rule; - } - - $this->addDecisionId($otherWatch, $level); - - $this->decisionQueue[] = $this->literalFromId($otherWatch); - $this->decisionQueueWhy[] = $rule; + if ($conflict) { + return $conflict; } } @@ -550,8 +453,9 @@ class Solver $this->learnedWhy[$newRule->getId()] = $why; - $this->watch2OnHighest($newRule); - $this->addWatchesToRule($newRule); + $ruleNode = new RuleWatchNode($newRule); + $ruleNode->watch2OnHighest($this->decisionMap); + $this->watchGraph->insert($ruleNode); $this->addDecision($learnLiteral, $level); $this->decisionQueue[] = $learnLiteral; From cdf3b4e012dbc53bcadd19bd8e5a594a07511585 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 02:24:45 +0200 Subject: [PATCH 12/27] Use SplDoublyLinkedList instead of custom linked list --- .../DependencyResolver/RuleWatchChain.php | 34 ++++++ .../DependencyResolver/RuleWatchGraph.php | 105 ++++++------------ .../DependencyResolver/RuleWatchNode.php | 21 ++-- 3 files changed, 76 insertions(+), 84 deletions(-) create mode 100644 src/Composer/DependencyResolver/RuleWatchChain.php diff --git a/src/Composer/DependencyResolver/RuleWatchChain.php b/src/Composer/DependencyResolver/RuleWatchChain.php new file mode 100644 index 000000000..00ff0bc56 --- /dev/null +++ b/src/Composer/DependencyResolver/RuleWatchChain.php @@ -0,0 +1,34 @@ + + * Jordi Boggiano + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Composer\DependencyResolver; + +/** + * @author Nils Adermann + */ +class RuleWatchChain extends \SplDoublyLinkedList +{ + protected $offset = 0; + + public function seek($offset) + { + $this->rewind(); + for ($i = 0; $i < $offset; $i++, $this->next()); + } + + public function remove() + { + $offset = $this->key(); + $this->offsetUnset($offset); + $this->seek($offset); + } +} diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index df736086e..2bc5cb7a3 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -22,30 +22,20 @@ class RuleWatchGraph /** * Alters watch chains for a rule. * - * Next1/2 always points to the next rule that is watching the same package. - * The watches array contains rules to start from for each package - * */ public function insert(RuleWatchNode $node) { - // skip simple assertions of the form (A) or (-A) if ($node->getRule()->isAssertion()) { return; } - if (!isset($this->watches[$node->watch1])) { - $this->watches[$node->watch1] = null; + foreach (array($node->watch1, $node->watch2) as $literal) { + if (!isset($this->watches[$literal])) { + $this->watches[$literal] = new RuleWatchChain; + } + + $this->watches[$literal]->unshift($node); } - - $node->next1 = $this->watches[$node->watch1]; - $this->watches[$node->watch1] = $node; - - if (!isset($this->watches[$node->watch2])) { - $this->watches[$node->watch2] = null; - } - - $node->next2 = $this->watches[$node->watch2]; - $this->watches[$node->watch2] = $node; } public function contains($literalId) @@ -56,79 +46,50 @@ class RuleWatchGraph public function walkLiteral($literalId, $level, $skipCallback, $conflictCallback, $decideCallback) { if (!isset($this->watches[$literalId])) { - return; + return null; } - $prevNode = null; - for ($node = $this->watches[$literalId]; $node !== null; $prevNode = $node, $node = $nextNode) { - $nextNode = $node->getNext($literalId); - - if ($node->getRule()->isDisabled()) { - continue; - } - + $this->watches[$literalId]->rewind(); + while ($this->watches[$literalId]->valid()) { + $node = $this->watches[$literalId]->current(); $otherWatch = $node->getOtherWatch($literalId); - if (call_user_func($skipCallback, $otherWatch)) { - continue; - } + if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { + $ruleLiterals = $node->getRule()->getLiterals(); - $ruleLiterals = $node->getRule()->getLiterals(); + if (sizeof($ruleLiterals) > 2) { + foreach ($ruleLiterals as $ruleLiteral) { + if ($otherWatch !== $ruleLiteral->getId() && + !call_user_func($conflictCallback, $ruleLiteral->getId())) { - if (sizeof($ruleLiterals) > 2) { - foreach ($ruleLiterals as $ruleLiteral) { - if ($otherWatch !== $ruleLiteral->getId() && - !call_user_func($conflictCallback, $ruleLiteral->getId())) { + $this->moveWatch($literalId, $ruleLiteral->getId(), $node); - $node = $this->moveWatch($literalId, $ruleLiteral->getId(), $prevNode, $node, $nextNode); - - continue 2; + continue 2; + } } } + + if (call_user_func($conflictCallback, $otherWatch)) { + return $node->getRule(); + } + + call_user_func($decideCallback, $otherWatch, $level, $node->getRule()); } - // yay, we found a unit clause! try setting it to true - if (call_user_func($conflictCallback, $otherWatch)) { - return $node->getRule(); - } - - call_user_func($decideCallback, $otherWatch, $level, $node->getRule()); + $this->watches[$literalId]->next(); } return null; } - public function moveWatch($fromLiteral, $toLiteral, $prevNode, $node, $nextNode) { - if ($fromLiteral == $node->watch1) { - $node->watch1 = $toLiteral; - $node->next1 = (isset($this->watches[$toLiteral])) ? $this->watches[$toLiteral] : null; - } else { - $node->watch2 = $toLiteral; - $node->next2 = (isset($this->watches[$toLiteral])) ? $this->watches[$toLiteral] : null; + protected function moveWatch($fromLiteral, $toLiteral, $node) + { + if (!isset($this->watches[$toLiteral])) { + $this->watches[$toLiteral] = new RuleWatchChain; } - if ($prevNode) { - if ($prevNode->next1 === $node) { - $prevNode->next1 = $nextNode; - } else { - $prevNode->next2 = $nextNode; - } - } else { - $this->watches[$fromLiteral] = $nextNode; - } - - $this->watches[$toLiteral] = $node; - - if ($prevNode) { - return $prevNode; - } - - $tmpNode = new RuleWatchNode(new Rule(array(), null, null)); - $tmpNode->watch1 = $fromLiteral; - $tmpNode->next1 = $nextNode; - $tmpNode->watch2 = $fromLiteral; - $tmpNode->next2 = $nextNode; - - return $tmpNode; + $node->moveWatch($fromLiteral, $toLiteral); + $this->watches[$fromLiteral]->remove(); + $this->watches[$toLiteral]->unshift($node); } } diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php index c1589bdb0..037d14e3d 100644 --- a/src/Composer/DependencyResolver/RuleWatchNode.php +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -22,9 +22,6 @@ class RuleWatchNode public $watch1; public $watch2; - public $next1; - public $next2; - public function __construct($rule) { $this->rule = $rule; @@ -64,15 +61,6 @@ class RuleWatchNode return $this->rule; } - public function getNext($literalId) - { - if ($this->watch1 == $literalId) { - return $this->next1; - } else { - return $this->next2; - } - } - public function getOtherWatch($literalId) { if ($this->watch1 == $literalId) { @@ -81,4 +69,13 @@ class RuleWatchNode return $this->watch1; } } + + public function moveWatch($from, $to) + { + if ($this->watch1 == $from) { + $this->watch1 = $to; + } else { + $this->watch2 = $to; + } + } } From 9ffe0d13f51ccbc167f4ad46cc260ab8e8c117e0 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 02:28:09 +0200 Subject: [PATCH 13/27] Remove useless if --- .../DependencyResolver/RuleWatchGraph.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index 2bc5cb7a3..4f44b2017 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -57,15 +57,16 @@ class RuleWatchGraph if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { $ruleLiterals = $node->getRule()->getLiterals(); - if (sizeof($ruleLiterals) > 2) { - foreach ($ruleLiterals as $ruleLiteral) { - if ($otherWatch !== $ruleLiteral->getId() && - !call_user_func($conflictCallback, $ruleLiteral->getId())) { + foreach ($ruleLiterals as $ruleLiteral) { + $ruleLiteralId = $ruleLiteral->getId(); - $this->moveWatch($literalId, $ruleLiteral->getId(), $node); + if ($literalId !== $ruleLiteralId && + $otherWatch !== $ruleLiteralId && + !call_user_func($conflictCallback, $ruleLiteralId)) { - continue 2; - } + $this->moveWatch($literalId, $ruleLiteralId, $node); + + continue 2; } } From 451bab1c2cd58e05af6e21639b829408ad023463 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 20:38:56 +0200 Subject: [PATCH 14/27] Get rid of Literal object / literal id mix, use literals only to save memory --- .../DependencyResolver/DefaultPolicy.php | 36 ++-- src/Composer/DependencyResolver/Literal.php | 67 ------- src/Composer/DependencyResolver/Pool.php | 11 ++ src/Composer/DependencyResolver/Rule.php | 68 ++----- .../DependencyResolver/RuleSetGenerator.php | 31 +--- .../DependencyResolver/RuleWatchGraph.php | 28 ++- .../DependencyResolver/RuleWatchNode.php | 12 +- src/Composer/DependencyResolver/Solver.php | 172 +++++++----------- .../DependencyResolver/Transaction.php | 24 +-- .../DependencyResolver/DefaultPolicyTest.php | 27 ++- .../Test/DependencyResolver/LiteralTest.php | 63 ------- .../RuleSetIteratorTest.php | 9 +- .../Test/DependencyResolver/RuleSetTest.php | 44 +++-- .../Test/DependencyResolver/RuleTest.php | 87 ++++----- 14 files changed, 225 insertions(+), 454 deletions(-) delete mode 100644 src/Composer/DependencyResolver/Literal.php delete mode 100644 tests/Composer/Test/DependencyResolver/LiteralTest.php diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index bbbf43025..181993e8e 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -50,44 +50,44 @@ class DefaultPolicy implements PolicyInterface public function selectPreferedPackages(Pool $pool, array $installedMap, array $literals) { - $packages = $this->groupLiteralsByNamePreferInstalled($installedMap, $literals); + $packages = $this->groupLiteralsByNamePreferInstalled($pool,$installedMap, $literals); foreach ($packages as &$literals) { $policy = $this; usort($literals, function ($a, $b) use ($policy, $pool, $installedMap) { - return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $a->getPackage(), $b->getPackage(), true); + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b), true); }); } foreach ($packages as &$literals) { - $literals = $this->pruneToBestVersion($literals); + $literals = $this->pruneToBestVersion($pool, $literals); $literals = $this->pruneToHighestPriorityOrInstalled($pool, $installedMap, $literals); - $literals = $this->pruneRemoteAliases($literals); + $literals = $this->pruneRemoteAliases($pool, $literals); } $selected = call_user_func_array('array_merge', $packages); // now sort the result across all packages to respect replaces across packages usort($selected, function ($a, $b) use ($policy, $pool, $installedMap) { - return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $a->getPackage(), $b->getPackage()); + return $policy->compareByPriorityPreferInstalled($pool, $installedMap, $pool->literalToPackage($a), $pool->literalToPackage($b)); }); return $selected; } - protected function groupLiteralsByNamePreferInstalled(array $installedMap, $literals) + protected function groupLiteralsByNamePreferInstalled(Pool $pool, array $installedMap, $literals) { $packages = array(); foreach ($literals as $literal) { - $packageName = $literal->getPackage()->getName(); + $packageName = $pool->literalToPackage($literal)->getName(); if (!isset($packages[$packageName])) { $packages[$packageName] = array(); } - if (isset($installedMap[$literal->getPackageId()])) { + if (isset($installedMap[abs($literal)])) { array_unshift($packages[$packageName], $literal); } else { $packages[$packageName][] = $literal; @@ -165,19 +165,21 @@ class DefaultPolicy implements PolicyInterface return false; } - protected function pruneToBestVersion($literals) + protected function pruneToBestVersion(Pool $pool, $literals) { $bestLiterals = array($literals[0]); - $bestPackage = $literals[0]->getPackage(); + $bestPackage = $pool->literalToPackage($literals[0]); foreach ($literals as $i => $literal) { if (0 === $i) { continue; } - if ($this->versionCompare($literal->getPackage(), $bestPackage, '>')) { - $bestPackage = $literal->getPackage(); + $package = $pool->literalToPackage($literal); + + if ($this->versionCompare($package, $bestPackage, '>')) { + $bestPackage = $package; $bestLiterals = array($literal); - } else if ($this->versionCompare($literal->getPackage(), $bestPackage, '==')) { + } else if ($this->versionCompare($package, $bestPackage, '==')) { $bestLiterals[] = $literal; } } @@ -215,7 +217,7 @@ class DefaultPolicy implements PolicyInterface $priority = null; foreach ($literals as $literal) { - $package = $literal->getPackage(); + $package = $pool->literalToPackage($literal); if (isset($installedMap[$package->getId()])) { $selected[] = $literal; @@ -241,12 +243,12 @@ class DefaultPolicy implements PolicyInterface * * If no package is a local alias, nothing happens */ - protected function pruneRemoteAliases(array $literals) + protected function pruneRemoteAliases(Pool $pool, array $literals) { $hasLocalAlias = false; foreach ($literals as $literal) { - $package = $literal->getPackage(); + $package = $pool->literalToPackage($literal); if ($package instanceof AliasPackage && $package->isRootPackageAlias()) { $hasLocalAlias = true; @@ -260,7 +262,7 @@ class DefaultPolicy implements PolicyInterface $selected = array(); foreach ($literals as $literal) { - $package = $literal->getPackage(); + $package = $pool->literalToPackage($literal); if ($package instanceof AliasPackage && $package->isRootPackageAlias()) { $selected[] = $literal; diff --git a/src/Composer/DependencyResolver/Literal.php b/src/Composer/DependencyResolver/Literal.php deleted file mode 100644 index 7234b5857..000000000 --- a/src/Composer/DependencyResolver/Literal.php +++ /dev/null @@ -1,67 +0,0 @@ - - * Jordi Boggiano - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Composer\DependencyResolver; - -use Composer\Package\PackageInterface; - -/** - * @author Nils Adermann - */ -class Literal -{ - protected $package; - protected $wanted; - protected $id; - - public function __construct(PackageInterface $package, $wanted) - { - $this->package = $package; - $this->wanted = $wanted; - $this->id = ($this->wanted ? '' : '-') . $this->package->getId(); - } - - public function isWanted() - { - return $this->wanted; - } - - public function getPackage() - { - return $this->package; - } - - public function getPackageId() - { - return $this->package->getId(); - } - - public function getId() - { - return $this->id; - } - - public function __toString() - { - return ($this->wanted ? '+' : '-') . $this->getPackage(); - } - - public function inverted() - { - return new Literal($this->getPackage(), !$this->isWanted()); - } - - public function equals(Literal $b) - { - return $this->id === $b->id; - } -} diff --git a/src/Composer/DependencyResolver/Pool.php b/src/Composer/DependencyResolver/Pool.php index 2b8dec0ad..e56d906ef 100644 --- a/src/Composer/DependencyResolver/Pool.php +++ b/src/Composer/DependencyResolver/Pool.php @@ -151,4 +151,15 @@ class Pool return $result; } + + public function literalToPackage($literal) + { + $packageId = abs($literal); + return $this->packageById($packageId); + } + + public function literalToString($literal) + { + return ($literal > 0 ? '+' : '-') . $this->literalToPackage($literal); + } } diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index f96d62300..93d00fc26 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -29,6 +29,8 @@ class Rule const RULE_LEARNED = 12; const RULE_PACKAGE_ALIAS = 13; + protected $pool; + protected $disabled; protected $literals; protected $type; @@ -37,18 +39,14 @@ class Rule protected $job; - public $watch1; - public $watch2; - - public $next1; - public $next2; - public $ruleHash; - public function __construct(array $literals, $reason, $reasonData, $job = null) + public function __construct(Pool $pool, array $literals, $reason, $reasonData, $job = null) { + $this->pool = $pool; + // sort all packages ascending by id - usort($literals, array($this, 'compareLiteralsById')); + sort($literals); $this->literals = $literals; $this->reason = $reason; @@ -59,14 +57,9 @@ class Rule $this->job = $job; - $this->watch1 = (count($this->literals) > 0) ? $literals[0]->getId() : 0; - $this->watch2 = (count($this->literals) > 1) ? $literals[1]->getId() : 0; - $this->type = -1; - $this->ruleHash = substr(md5(implode(',', array_map(function ($l) { - return $l->getId(); - }, $this->literals))), 0, 5); + $this->ruleHash = substr(md5(implode(',', $this->literals)), 0, 5); } public function getHash() @@ -108,7 +101,7 @@ class Rule } for ($i = 0, $n = count($this->literals); $i < $n; $i++) { - if ($this->literals[$i]->getId() !== $rule->literals[$i]->getId()) { + if ($this->literals[$i] !== $rule->literals[$i]) { return false; } } @@ -166,24 +159,6 @@ class Rule return 1 === count($this->literals); } - public function getNext(Literal $literal) - { - if ($this->watch1 == $literal->getId()) { - return $this->next1; - } else { - return $this->next2; - } - } - - public function getOtherWatch(Literal $literal) - { - if ($this->watch1 == $literal->getId()) { - return $this->watch2; - } else { - return $this->watch1; - } - } - public function toHumanReadableString() { $ruleText = ''; @@ -191,7 +166,7 @@ class Rule if ($i != 0) { $ruleText .= '|'; } - $ruleText .= $literal; + $ruleText .= $this->pool->literalToString($literal); } switch ($this->reason) { @@ -205,18 +180,18 @@ class Rule return "Remove command rule ($ruleText)"; case self::RULE_PACKAGE_CONFLICT: - $package1 = $this->literals[0]->getPackage(); - $package2 = $this->literals[1]->getPackage(); + $package1 = $this->pool->literalToPackage($this->literals[0]); + $package2 = $this->pool->literalToPackage($this->literals[1]); return 'Package "'.$package1.'" conflicts with "'.$package2.'"'; case self::RULE_PACKAGE_REQUIRES: $literals = $this->literals; $sourceLiteral = array_shift($literals); - $sourcePackage = $sourceLiteral->getPackage(); + $sourcePackage = $this->pool->literalToPackage($sourceLiteral); $requires = array(); foreach ($literals as $literal) { - $requires[] = $literal->getPackage(); + $requires[] = $this->pool->literalToPackage($literal); } $text = 'Package "'.$sourcePackage.'" contains the rule '.$this->reasonData.'. '; @@ -255,26 +230,11 @@ class Rule if ($i != 0) { $result .= '|'; } - $result .= $literal; + $result .= $this->pool->literalToString($literal); } $result .= ')'; return $result; } - - /** - * Comparison function for sorting literals by their id - * - * @param Literal $a - * @param Literal $b - * @return int 0 if the literals are equal, 1 if b is larger than a, -1 else - */ - private function compareLiteralsById(Literal $a, Literal $b) - { - if ($a->getId() === $b->getId()) { - return 0; - } - return $a->getId() < $b->getId() ? -1 : 1; - } } diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index e95edc77d..1538e5695 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -50,34 +50,17 @@ class RuleSetGenerator */ protected function createRequireRule(PackageInterface $package, array $providers, $reason, $reasonData = null) { - $literals = array(new Literal($package, false)); + $literals = array(-$package->getId()); foreach ($providers as $provider) { // self fulfilling rule? if ($provider === $package) { return null; } - $literals[] = new Literal($provider, true); + $literals[] = $provider->getId(); } - return new Rule($literals, $reason, $reasonData); - } - - /** - * Creates a new rule for installing a package - * - * The rule is simply (A) for a package A to be installed. - * - * @param PackageInterface $package The package to be installed - * @param int $reason A RULE_* constant describing the - * reason for generating this rule - * @param mixed $reasonData Any data, e.g. the package name, that - * goes with the reason - * @return Rule The generated rule - */ - protected function createInstallRule(PackageInterface $package, $reason, $reasonData = null) - { - return new Rule(new Literal($package, true)); + return new Rule($this->pool, $literals, $reason, $reasonData); } /** @@ -96,10 +79,10 @@ class RuleSetGenerator { $literals = array(); foreach ($packages as $package) { - $literals[] = new Literal($package, true); + $literals[] = $package->getId(); } - return new Rule($literals, $reason, $job['packageName'], $job); + return new Rule($this->pool, $literals, $reason, $job['packageName'], $job); } /** @@ -115,7 +98,7 @@ class RuleSetGenerator */ protected function createRemoveRule(PackageInterface $package, $reason, $job) { - return new Rule(array(new Literal($package, false)), $reason, $job['packageName'], $job); + return new Rule($this->pool, array(-$package->getId()), $reason, $job['packageName'], $job); } /** @@ -139,7 +122,7 @@ class RuleSetGenerator return null; } - return new Rule(array(new Literal($issuer, false), new Literal($provider, false)), $reason, $reasonData); + return new Rule($this->pool, array(-$issuer->getId(), -$provider->getId()), $reason, $reasonData); } /** diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index 4f44b2017..b1d2dc737 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -38,33 +38,31 @@ class RuleWatchGraph } } - public function contains($literalId) + public function contains($literal) { - return isset($this->watches[$literalId]); + return isset($this->watches[$literal]); } - public function walkLiteral($literalId, $level, $skipCallback, $conflictCallback, $decideCallback) + public function walkLiteral($literal, $level, $skipCallback, $conflictCallback, $decideCallback) { - if (!isset($this->watches[$literalId])) { + if (!isset($this->watches[$literal])) { return null; } - $this->watches[$literalId]->rewind(); - while ($this->watches[$literalId]->valid()) { - $node = $this->watches[$literalId]->current(); - $otherWatch = $node->getOtherWatch($literalId); + $this->watches[$literal]->rewind(); + while ($this->watches[$literal]->valid()) { + $node = $this->watches[$literal]->current(); + $otherWatch = $node->getOtherWatch($literal); if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { $ruleLiterals = $node->getRule()->getLiterals(); foreach ($ruleLiterals as $ruleLiteral) { - $ruleLiteralId = $ruleLiteral->getId(); + if ($literal !== $ruleLiteral && + $otherWatch !== $ruleLiteral && + !call_user_func($conflictCallback, $ruleLiteral)) { - if ($literalId !== $ruleLiteralId && - $otherWatch !== $ruleLiteralId && - !call_user_func($conflictCallback, $ruleLiteralId)) { - - $this->moveWatch($literalId, $ruleLiteralId, $node); + $this->moveWatch($literal, $ruleLiteral, $node); continue 2; } @@ -77,7 +75,7 @@ class RuleWatchGraph call_user_func($decideCallback, $otherWatch, $level, $node->getRule()); } - $this->watches[$literalId]->next(); + $this->watches[$literal]->next(); } return null; diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php index 037d14e3d..1c8cfdeab 100644 --- a/src/Composer/DependencyResolver/RuleWatchNode.php +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -28,8 +28,8 @@ class RuleWatchNode $literals = $rule->getLiterals(); - $this->watch1 = (count($literals) > 0) ? $literals[0]->getId() : 0; - $this->watch2 = (count($literals) > 1) ? $literals[1]->getId() : 0; + $this->watch1 = (count($literals) > 0) ? $literals[0] : 0; + $this->watch2 = (count($literals) > 1) ? $literals[1] : 0; } /** @@ -47,10 +47,10 @@ class RuleWatchNode $watchLevel = 0; foreach ($literals as $literal) { - $level = abs($decisionMap[$literal->getPackageId()]); + $level = abs($decisionMap[abs($literal)]); if ($level > $watchLevel) { - $this->rule->watch2 = $literal->getId(); + $this->rule->watch2 = $literal; $watchLevel = $level; } } @@ -61,9 +61,9 @@ class RuleWatchNode return $this->rule; } - public function getOtherWatch($literalId) + public function getOtherWatch($literal) { - if ($this->watch1 == $literalId) { + if ($this->watch1 == $literal) { return $this->watch2; } else { return $this->watch1; diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 0e6f094ab..14328946a 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -13,7 +13,6 @@ namespace Composer\DependencyResolver; use Composer\Repository\RepositoryInterface; -use Composer\Package\PackageInterface; use Composer\Package\AliasPackage; use Composer\DependencyResolver\Operation; @@ -51,10 +50,10 @@ class Solver $this->ruleSetGenerator = new RuleSetGenerator($policy, $pool); } - private function findDecisionRule(PackageInterface $package) + private function findDecisionRule($packageId) { foreach ($this->decisionQueue as $i => $literal) { - if ($package === $literal->getPackage()) { + if ($packageId === abs($literal)) { return $this->decisionQueueWhy[$i]; } } @@ -77,10 +76,8 @@ class Solver $literals = $rule->getLiterals(); $literal = $literals[0]; - if (!$this->decided($literal->getPackage())) { - $this->decisionQueue[] = $literal; - $this->decisionQueueWhy[] = $rule; - $this->addDecision($literal, 1); + if (!$this->decided(abs($literal))) { + $this->decide($literal, 1, $rule); continue; } @@ -94,7 +91,7 @@ class Solver continue; } - $conflict = $this->findDecisionRule($literal->getPackage()); + $conflict = $this->findDecisionRule(abs($literal)); if ($conflict && RuleSet::TYPE_PACKAGE === $conflict->getType()) { @@ -122,7 +119,7 @@ class Solver $assertRuleLiterals = $assertRule->getLiterals(); $assertRuleLiteral = $assertRuleLiterals[0]; - if ($literal->getPackageId() !== $assertRuleLiteral->getPackageId()) { + if (abs($literal) !== abs($assertRuleLiteral)) { continue; } @@ -136,7 +133,7 @@ class Solver $decisionLiteral = array_pop($this->decisionQueue); array_pop($this->decisionQueueWhy); unset($this->decisionQueueFree[count($this->decisionQueue)]); - $this->decisionMap[$decisionLiteral->getPackageId()] = 0; + $this->decisionMap[abs($decisionLiteral)] = 0; } $ruleIndex = -1; } @@ -149,10 +146,8 @@ class Solver $literals = $rule->getLiterals(); $literal = $literals[0]; - if ($this->decisionMap[$literal->getPackageId()] == 0) { - $this->decisionQueue[] = $literal; - $this->decisionQueueWhy[] = $rule; - $this->addDecision($literal, 1); + if ($this->decisionMap[abs($literal)] == 0) { + $this->decide($literal, 1, $rule); continue; } @@ -191,7 +186,7 @@ class Solver case 'install': if (!$job['packages']) { $problem = new Problem(); - $problem->addRule(new Rule(array(), null, null, $job)); + $problem->addRule(new Rule($this->pool, array(), null, null, $job)); $this->problems[] = $problem; } break; @@ -237,24 +232,20 @@ class Solver return new Literal($package, $id > 0); } - protected function addDecision(Literal $l, $level) + protected function addDecision($literal, $level) { - $this->addDecisionId($l->getId(), $level); - } - - protected function addDecisionId($literalId, $level) - { - $packageId = abs($literalId); + $packageId = abs($literal); $previousDecision = $this->decisionMap[$packageId]; if ($previousDecision != 0) { - $literal = $this->literalFromId($literalId); + $literalString = $this->pool->literalToString($literal); + $package = $this->pool->literalToPackage($literal); throw new SolverBugException( - "Trying to decide $literal on level $level, even though ".$literal->getPackage()." was previously decided as ".(int) $previousDecision."." + "Trying to decide $literalString on level $level, even though $package was previously decided as ".(int) $previousDecision."." ); } - if ($literalId > 0) { + if ($literal > 0) { $this->decisionMap[$packageId] = $level; } else { $this->decisionMap[$packageId] = -$level; @@ -263,67 +254,49 @@ class Solver public function decide($literal, $level, $why) { - $this->addDecisionId($literal, $level); - $this->decisionQueue[] = $this->literalFromId($literal); + $this->addDecision($literal, $level); + $this->decisionQueue[] = $literal; $this->decisionQueueWhy[] = $why; } - protected function decisionsContain(Literal $l) + public function decisionsContain($literal) { + $packageId = abs($literal); return ( - $this->decisionMap[$l->getPackageId()] > 0 && $l->isWanted() || - $this->decisionMap[$l->getPackageId()] < 0 && !$l->isWanted() + $this->decisionMap[$packageId] > 0 && $literal > 0 || + $this->decisionMap[$packageId] < 0 && $literal < 0 ); } - public function decisionsContainId($literalId) + protected function decisionsSatisfy($literal) { - $packageId = abs($literalId); + $packageId = abs($literal); return ( - $this->decisionMap[$packageId] > 0 && $literalId > 0 || - $this->decisionMap[$packageId] < 0 && $literalId < 0 + $literal > 0 && $this->decisionMap[$packageId] > 0 || + $literal < 0 && $this->decisionMap[$packageId] < 0 ); } - protected function decisionsSatisfy(Literal $l) - { - return ($l->isWanted() && $this->decisionMap[$l->getPackageId()] > 0) || - (!$l->isWanted() && $this->decisionMap[$l->getPackageId()] < 0); - } - - protected function decisionsConflict(Literal $l) + public function decisionsConflict($literal) { + $packageId = abs($literal); return ( - $this->decisionMap[$l->getPackageId()] > 0 && !$l->isWanted() || - $this->decisionMap[$l->getPackageId()] < 0 && $l->isWanted() + ($this->decisionMap[$packageId] > 0 && $literal < 0) || + ($this->decisionMap[$packageId] < 0 && $literal > 0) ); } - - public function decisionsConflictId($literalId) + protected function decided($packageId) { - $packageId = abs($literalId); - return ( - ($this->decisionMap[$packageId] > 0 && $literalId < 0) || - ($this->decisionMap[$packageId] < 0 && $literalId > 0) - ); + return $this->decisionMap[$packageId] != 0; } - protected function decided(PackageInterface $p) + protected function undecided($packageId) { - return $this->decisionMap[$p->getId()] != 0; + return $this->decisionMap[$packageId] == 0; } - protected function undecided(PackageInterface $p) - { - return $this->decisionMap[$p->getId()] == 0; - } - - protected function decidedInstall(PackageInterface $p) { - return $this->decisionMap[$p->getId()] > 0; - } - - protected function decidedRemove(PackageInterface $p) { - return $this->decisionMap[$p->getId()] < 0; + protected function decidedInstall($packageId) { + return $this->decisionMap[$packageId] > 0; } /** @@ -341,15 +314,15 @@ class Solver // A was decided => (-A|B) now requires B to be true, so we look for // rules which are fulfilled by -A, rather than A. - $literal = $this->decisionQueue[$this->propagateIndex]->inverted(); + $literal = -$this->decisionQueue[$this->propagateIndex]; $this->propagateIndex++; $conflict = $this->watchGraph->walkLiteral( - $literal->getId(), + $literal, $level, - array($this, 'decisionsContainId'), - array($this, 'decisionsConflictId'), + array($this, 'decisionsContain'), + array($this, 'decisionsConflict'), array($this, 'decide') ); @@ -369,17 +342,17 @@ class Solver while (!empty($this->decisionQueue)) { $literal = $this->decisionQueue[count($this->decisionQueue) - 1]; - if (!$this->decisionMap[$literal->getPackageId()]) { + if (!$this->decisionMap[abs($literal)]) { break; } - $decisionLevel = abs($this->decisionMap[$literal->getPackageId()]); + $decisionLevel = abs($this->decisionMap[abs($literal)]); if ($decisionLevel <= $level) { break; } - $this->decisionMap[$literal->getPackageId()] = 0; + $this->decisionMap[abs($literal)] = 0; array_pop($this->decisionQueue); array_pop($this->decisionQueueWhy); @@ -412,13 +385,11 @@ class Solver * returns the new solver level or 0 if unsolvable * */ - private function setPropagateLearn($level, Literal $literal, $disableRules, Rule $rule) + private function setPropagateLearn($level, $literal, $disableRules, Rule $rule) { $level++; - $this->addDecision($literal, $level); - $this->decisionQueue[] = $literal; - $this->decisionQueueWhy[] = $rule; + $this->decide($literal, $level, $rule); $this->decisionQueueFree[count($this->decisionQueueWhy) - 1] = true; while (true) { @@ -457,9 +428,7 @@ class Solver $ruleNode->watch2OnHighest($this->decisionMap); $this->watchGraph->insert($ruleNode); - $this->addDecision($learnLiteral, $level); - $this->decisionQueue[] = $learnLiteral; - $this->decisionQueueWhy[] = $newRule; + $this->decide($learnLiteral, $level, $newRule); } return $level; @@ -502,12 +471,12 @@ class Solver continue; } - if (isset($seen[$literal->getPackageId()])) { + if (isset($seen[abs($literal)])) { continue; } - $seen[$literal->getPackageId()] = true; + $seen[abs($literal)] = true; - $l = abs($this->decisionMap[$literal->getPackageId()]); + $l = abs($this->decisionMap[abs($literal)]); if (1 === $l) { $l1num++; @@ -543,15 +512,15 @@ class Solver $literal = $this->decisionQueue[$decisionId]; - if (isset($seen[$literal->getPackageId()])) { + if (isset($seen[abs($literal)])) { break; } } - unset($seen[$literal->getPackageId()]); + unset($seen[abs($literal)]); if ($num && 0 === --$num) { - $learnedLiterals[0] = $this->literalFromId(-$literal->getPackageId()); + $learnedLiterals[0] = -abs($literal); if (!$l1num) { break 2; @@ -559,7 +528,7 @@ class Solver foreach ($learnedLiterals as $i => $learnedLiteral) { if ($i !== 0) { - unset($seen[$literal->getPackageId()]); + unset($seen[abs($learnedLiteral)]); } } // only level 1 marks left @@ -579,7 +548,7 @@ class Solver ); } - $newRule = new Rule($learnedLiterals, Rule::RULE_LEARNED, $why); + $newRule = new Rule($this->pool, $learnedLiterals, Rule::RULE_LEARNED, $why); return array($learnedLiterals[0], $ruleLevel, $newRule, $why); } @@ -626,18 +595,12 @@ class Solver $seen = array(); $literals = $conflictRule->getLiterals(); -/* unnecessary because unlike rule.d, watch2 == 2nd literal, unless watch2 changed - if (sizeof($literals) == 2) { - $literals[1] = $this->literalFromId($conflictRule->watch2); - } -*/ - foreach ($literals as $literal) { // skip the one true literal if ($this->decisionsSatisfy($literal)) { continue; } - $seen[$literal->getPackageId()] = true; + $seen[abs($literal)] = true; } $decisionId = count($this->decisionQueue); @@ -648,7 +611,7 @@ class Solver $literal = $this->decisionQueue[$decisionId]; // skip literals that are not in this rule - if (!isset($seen[$literal->getPackageId()])) { + if (!isset($seen[abs($literal)])) { continue; } @@ -658,18 +621,13 @@ class Solver $this->analyzeUnsolvableRule($problem, $why, $lastWeakWhy); $literals = $why->getLiterals(); -/* unnecessary because unlike rule.d, watch2 == 2nd literal, unless watch2 changed - if (sizeof($literals) == 2) { - $literals[1] = $this->literalFromId($why->watch2); - } -*/ foreach ($literals as $literal) { // skip the one true literal if ($this->decisionsSatisfy($literal)) { continue; } - $seen[$literal->getPackageId()] = true; + $seen[abs($literal)] = true; } } @@ -713,7 +671,7 @@ class Solver private function resetSolver() { while ($literal = array_pop($this->decisionQueue)) { - $this->decisionMap[$literal->getPackageId()] = 0; + $this->decisionMap[abs($literal)] = 0; } $this->decisionQueueWhy = array(); @@ -812,9 +770,9 @@ class Solver if (count($this->installed) != count($this->updateMap)) { $prunedQueue = array(); foreach ($decisionQueue as $literal) { - if (isset($this->installedMap[$literal->getPackageId()])) { + if (isset($this->installedMap[abs($literal)])) { $prunedQueue[] = $literal; - if (isset($this->updateMap[$literal->getPackageId()])) { + if (isset($this->updateMap[abs($literal)])) { $prunedQueue = $decisionQueue; break; } @@ -873,15 +831,15 @@ class Solver // just need to decide on the positive literals // foreach ($literals as $literal) { - if (!$literal->isWanted()) { - if (!$this->decidedInstall($literal->getPackage())) { + if ($literal <= 0) { + if (!$this->decidedInstall(abs($literal))) { continue 2; // next rule } } else { - if ($this->decidedInstall($literal->getPackage())) { + if ($this->decidedInstall(abs($literal))) { continue 2; // next rule } - if ($this->undecided($literal->getPackage())) { + if ($this->undecided(abs($literal))) { $decisionQueue[] = $literal; } } @@ -919,7 +877,7 @@ class Solver list($literals, $level) = $this->branches[$i]; foreach ($literals as $offset => $literal) { - if ($literal && $literal->isWanted() && $this->decisionMap[$literal->getPackageId()] > $level + 1) { + if ($literal && $literal > 0 && $this->decisionMap[abs($literal)] > $level + 1) { $lastLiteral = $literal; $lastBranchIndex = $i; $lastBranchOffset = $offset; diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php index 79235e5cb..ccf413581 100644 --- a/src/Composer/DependencyResolver/Transaction.php +++ b/src/Composer/DependencyResolver/Transaction.php @@ -43,35 +43,35 @@ class Transaction $installMeansUpdateMap = array(); foreach ($this->decisionQueue as $i => $literal) { - $package = $literal->getPackage(); + $package = $this->pool->literalToPackage($literal); // !wanted & installed - if (!$literal->isWanted() && isset($this->installedMap[$package->getId()])) { + if ($literal <= 0 && isset($this->installedMap[$package->getId()])) { $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package); - $literals = array(new Literal($package, true)); + $literals = array($package->getId()); foreach ($updates as $update) { - $literals[] = new Literal($update, true); + $literals[] = $update->getId(); } foreach ($literals as $updateLiteral) { - if (!$updateLiteral->equals($literal)) { - $installMeansUpdateMap[$updateLiteral->getPackageId()] = $package; + if ($updateLiteral !== $literal) { + $installMeansUpdateMap[abs($updateLiteral)] = $package; } } } } foreach ($this->decisionQueue as $i => $literal) { - $package = $literal->getPackage(); + $package = $this->pool->literalToPackage($literal); // wanted & installed || !wanted & !installed - if ($literal->isWanted() == (isset($this->installedMap[$package->getId()]))) { + if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) { continue; } - if ($literal->isWanted()) { + if ($literal > 0) { if ($package instanceof AliasPackage) { $transaction[] = new Operation\MarkAliasInstalledOperation( $package, $this->decisionQueueWhy[$i] @@ -79,16 +79,16 @@ class Transaction continue; } - if (isset($installMeansUpdateMap[$literal->getPackageId()])) { + if (isset($installMeansUpdateMap[abs($literal)])) { - $source = $installMeansUpdateMap[$literal->getPackageId()]; + $source = $installMeansUpdateMap[abs($literal)]; $transaction[] = new Operation\UpdateOperation( $source, $package, $this->decisionQueueWhy[$i] ); // avoid updates to one package from multiple origins - unset($installMeansUpdateMap[$literal->getPackageId()]); + unset($installMeansUpdateMap[abs($literal)]); $ignoreRemove[$source->getId()] = true; } else { $transaction[] = new Operation\InstallOperation( diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index 8a6c64922..81530fe06 100644 --- a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php +++ b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php @@ -16,7 +16,6 @@ use Composer\Repository\ArrayRepository; use Composer\Repository\RepositoryInterface; use Composer\DependencyResolver\DefaultPolicy; use Composer\DependencyResolver\Pool; -use Composer\DependencyResolver\Literal; use Composer\Package\Link; use Composer\Package\AliasPackage; use Composer\Package\LinkConstraint\VersionConstraint; @@ -44,8 +43,8 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA = $this->getPackage('A', '1.0')); $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA, true)); - $expected = array(new Literal($packageA, true)); + $literals = array($packageA->getId()); + $expected = array($packageA->getId()); $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); @@ -58,8 +57,8 @@ class DefaultPolicyTest extends TestCase $this->repo->addPackage($packageA2 = $this->getPackage('A', '2.0')); $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA1, true), new Literal($packageA2, true)); - $expected = array(new Literal($packageA2, true)); + $literals = array($packageA1->getId(), $packageA2->getId()); + $expected = array($packageA2->getId()); $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); @@ -73,8 +72,8 @@ class DefaultPolicyTest extends TestCase $this->pool->addRepository($this->repoInstalled); $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA, true), new Literal($packageAInstalled, true)); - $expected = array(new Literal($packageA, true)); + $literals = array($packageA->getId(), $packageAInstalled->getId()); + $expected = array($packageA->getId()); $selected = $this->policy->selectPreferedPackages($this->pool, $this->mapFromRepo($this->repoInstalled), $literals); @@ -92,8 +91,8 @@ class DefaultPolicyTest extends TestCase $this->pool->addRepository($this->repoImportant); $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA, true), new Literal($packageAImportant, true)); - $expected = array(new Literal($packageAImportant, true)); + $literals = array($packageA->getId(), $packageAImportant->getId()); + $expected = array($packageAImportant->getId()); $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); @@ -119,10 +118,10 @@ class DefaultPolicyTest extends TestCase $packages = $this->pool->whatProvides('a', new VersionConstraint('=', '2.1.9999999.9999999-dev')); $literals = array(); foreach ($packages as $package) { - $literals[] = new Literal($package, true); + $literals[] = $package->getId(); } - $expected = array(new Literal($packageAAliasImportant, true)); + $expected = array($packageAAliasImportant->getId()); $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); @@ -139,7 +138,7 @@ class DefaultPolicyTest extends TestCase $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA, true), new Literal($packageB, true)); + $literals = array($packageA->getId(), $packageB->getId()); $expected = $literals; $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); @@ -157,8 +156,8 @@ class DefaultPolicyTest extends TestCase $this->pool->addRepository($this->repo); - $literals = array(new Literal($packageA, true), new Literal($packageB, true)); - $expected = array(new Literal($packageA, true), new Literal($packageB, true)); + $literals = array($packageA->getId(), $packageB->getId()); + $expected = $literals; $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); diff --git a/tests/Composer/Test/DependencyResolver/LiteralTest.php b/tests/Composer/Test/DependencyResolver/LiteralTest.php deleted file mode 100644 index ef3638601..000000000 --- a/tests/Composer/Test/DependencyResolver/LiteralTest.php +++ /dev/null @@ -1,63 +0,0 @@ - - * Jordi Boggiano - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Composer\Test\DependencyResolver; - -use Composer\DependencyResolver\Literal; -use Composer\Test\TestCase; - -class LiteralTest extends TestCase -{ - protected $package; - - public function setUp() - { - $this->package = $this->getPackage('foo', '1'); - $this->package->setId(12); - } - - public function testLiteralWanted() - { - $literal = new Literal($this->package, true); - - $this->assertEquals(12, $literal->getId()); - $this->assertEquals('+'.(string) $this->package, (string) $literal); - } - - public function testLiteralUnwanted() - { - $literal = new Literal($this->package, false); - - $this->assertEquals(-12, $literal->getId()); - $this->assertEquals('-'.(string) $this->package, (string) $literal); - } - - public function testLiteralInverted() - { - $literal = new Literal($this->package, false); - - $inverted = $literal->inverted(); - - $this->assertInstanceOf('\Composer\DependencyResolver\Literal', $inverted); - $this->assertTrue($inverted->isWanted()); - $this->assertSame($this->package, $inverted->getPackage()); - $this->assertFalse($literal->equals($inverted)); - - $doubleInverted = $inverted->inverted(); - - $this->assertInstanceOf('\Composer\DependencyResolver\Literal', $doubleInverted); - $this->assertFalse($doubleInverted->isWanted()); - $this->assertSame($this->package, $doubleInverted->getPackage()); - - $this->assertTrue($literal->equals($doubleInverted)); - } -} diff --git a/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php index 56084f32a..10ec17501 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php @@ -15,6 +15,7 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Rule; use Composer\DependencyResolver\RuleSet; use Composer\DependencyResolver\RuleSetIterator; +use Composer\DependencyResolver\Pool; class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase { @@ -22,13 +23,15 @@ class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase protected function setUp() { + $this->pool = new Pool; + $this->rules = array( RuleSet::TYPE_JOB => array( - new Rule(array(), 'job1', null), - new Rule(array(), 'job2', null), + new Rule($this->pool, array(), 'job1', null), + new Rule($this->pool, array(), 'job2', null), ), RuleSet::TYPE_LEARNED => array( - new Rule(array(), 'update1', null), + new Rule($this->pool, array(), 'update1', null), ), RuleSet::TYPE_PACKAGE => array(), ); diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php index f319651ae..a6b108500 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -14,21 +14,29 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Rule; use Composer\DependencyResolver\RuleSet; -use Composer\DependencyResolver\Literal; +use Composer\DependencyResolver\Pool; +use Composer\Repository\ArrayRepository; use Composer\Test\TestCase; class RuleSetTest extends TestCase { + protected $pool; + + public function setUp() + { + $this->pool = new Pool; + } + public function testAdd() { $rules = array( RuleSet::TYPE_PACKAGE => array(), RuleSet::TYPE_JOB => array( - new Rule(array(), 'job1', null), - new Rule(array(), 'job2', null), + new Rule($this->pool, array(), 'job1', null), + new Rule($this->pool, array(), 'job2', null), ), RuleSet::TYPE_LEARNED => array( - new Rule(array(), 'update1', null), + new Rule($this->pool, array(), 'update1', null), ), ); @@ -48,15 +56,15 @@ class RuleSetTest extends TestCase { $ruleSet = new RuleSet; - $ruleSet->add(new Rule(array(), 'job1', null), 7); + $ruleSet->add(new Rule($this->pool, array(), 'job1', null), 7); } public function testCount() { $ruleSet = new RuleSet; - $ruleSet->add(new Rule(array(), 'job1', null), RuleSet::TYPE_JOB); - $ruleSet->add(new Rule(array(), 'job2', null), RuleSet::TYPE_JOB); + $ruleSet->add(new Rule($this->pool, array(), 'job1', null), RuleSet::TYPE_JOB); + $ruleSet->add(new Rule($this->pool, array(), 'job2', null), RuleSet::TYPE_JOB); $this->assertEquals(2, $ruleSet->count()); } @@ -65,7 +73,7 @@ class RuleSetTest extends TestCase { $ruleSet = new RuleSet; - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $ruleSet->add($rule, RuleSet::TYPE_JOB); $this->assertSame($rule, $ruleSet->ruleById(0)); @@ -75,8 +83,8 @@ class RuleSetTest extends TestCase { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), 'job1', null); - $rule2 = new Rule(array(), 'job1', null); + $rule1 = new Rule($this->pool, array(), 'job1', null); + $rule2 = new Rule($this->pool, array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -90,8 +98,8 @@ class RuleSetTest extends TestCase public function testGetIteratorFor() { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), 'job1', null); - $rule2 = new Rule(array(), 'job1', null); + $rule1 = new Rule($this->pool, array(), 'job1', null); + $rule2 = new Rule($this->pool, array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -104,8 +112,8 @@ class RuleSetTest extends TestCase public function testGetIteratorWithout() { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), 'job1', null); - $rule2 = new Rule(array(), 'job1', null); + $rule1 = new Rule($this->pool, array(), 'job1', null); + $rule2 = new Rule($this->pool, array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -149,9 +157,13 @@ class RuleSetTest extends TestCase public function testToString() { + $repo = new ArrayRepository; + $repo->addPackage($p = $this->getPackage('foo', '2.1')); + $this->pool->addRepository($repo); + $ruleSet = new RuleSet; - $literal = new Literal($this->getPackage('foo', '2.1'), true); - $rule = new Rule(array($literal), 'job1', null); + $literal = $p->getId(); + $rule = new Rule($this->pool, array($literal), 'job1', null); $ruleSet->add($rule, RuleSet::TYPE_JOB); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index f76f46eb1..739dbd7ff 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -13,14 +13,22 @@ namespace Composer\Test\DependencyResolver; use Composer\DependencyResolver\Rule; -use Composer\DependencyResolver\Literal; +use Composer\DependencyResolver\Pool; +use Composer\Repository\ArrayRepository; use Composer\Test\TestCase; class RuleTest extends TestCase { + protected $pool; + + public function setUp() + { + $this->pool = new Pool; + } + public function testGetHash() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->ruleHash = '123'; $this->assertEquals('123', $rule->getHash()); @@ -28,7 +36,7 @@ class RuleTest extends TestCase public function testSetAndGetId() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->setId(666); $this->assertEquals(666, $rule->getId()); @@ -36,10 +44,10 @@ class RuleTest extends TestCase public function testEqualsForRulesWithDifferentHashes() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->ruleHash = '123'; - $rule2 = new Rule(array(), 'job1', null); + $rule2 = new Rule($this->pool, array(), 'job1', null); $rule2->ruleHash = '321'; $this->assertFalse($rule->equals($rule2)); @@ -47,18 +55,10 @@ class RuleTest extends TestCase public function testEqualsForRulesWithDifferentLiterals() { - $literal = $this->getLiteralMock(); - $literal->expects($this->any()) - ->method('getId') - ->will($this->returnValue(1)); - $rule = new Rule(array($literal), 'job1', null); + $rule = new Rule($this->pool, array(1), 'job1', null); $rule->ruleHash = '123'; - $literal = $this->getLiteralMock(); - $literal->expects($this->any()) - ->method('getId') - ->will($this->returnValue(12)); - $rule2 = new Rule(array($literal), 'job1', null); + $rule2 = new Rule($this->pool, array(12), 'job1', null); $rule2->ruleHash = '123'; $this->assertFalse($rule->equals($rule2)); @@ -66,18 +66,9 @@ class RuleTest extends TestCase public function testEqualsForRulesWithDifferLiteralsQuantity() { - $literal = $this->getLiteralMock(); - $literal->expects($this->any()) - ->method('getId') - ->will($this->returnValue(1)); - $literal2 = $this->getLiteralMock(); - $literal2->expects($this->any()) - ->method('getId') - ->will($this->returnValue(12)); - - $rule = new Rule(array($literal, $literal2), 'job1', null); + $rule = new Rule($this->pool, array(1, 12), 'job1', null); $rule->ruleHash = '123'; - $rule2 = new Rule(array($literal), 'job1', null); + $rule2 = new Rule($this->pool, array(1), 'job1', null); $rule2->ruleHash = '123'; $this->assertFalse($rule->equals($rule2)); @@ -85,24 +76,15 @@ class RuleTest extends TestCase public function testEqualsForRulesWithThisSameLiterals() { - $literal = $this->getLiteralMock(); - $literal->expects($this->any()) - ->method('getId') - ->will($this->returnValue(1)); - $literal2 = $this->getLiteralMock(); - $literal2->expects($this->any()) - ->method('getId') - ->will($this->returnValue(12)); - - $rule = new Rule(array($literal, $literal2), 'job1', null); - $rule2 = new Rule(array($literal, $literal2), 'job1', null); + $rule = new Rule($this->pool, array(1, 12), 'job1', null); + $rule2 = new Rule($this->pool, array(1, 12), 'job1', null); $this->assertTrue($rule->equals($rule2)); } public function testSetAndGetType() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->setType('someType'); $this->assertEquals('someType', $rule->getType()); @@ -110,7 +92,7 @@ class RuleTest extends TestCase public function testEnable() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->disable(); $rule->enable(); @@ -120,7 +102,7 @@ class RuleTest extends TestCase public function testDisable() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->enable(); $rule->disable(); @@ -130,10 +112,10 @@ class RuleTest extends TestCase public function testSetWeak() { - $rule = new Rule(array(), 'job1', null); + $rule = new Rule($this->pool, array(), 'job1', null); $rule->setWeak(true); - $rule2 = new Rule(array(), 'job1', null); + $rule2 = new Rule($this->pool, array(), 'job1', null); $rule2->setWeak(false); $this->assertTrue($rule->isWeak()); @@ -142,10 +124,8 @@ class RuleTest extends TestCase public function testIsAssertions() { - $literal = $this->getLiteralMock(); - $literal2 = $this->getLiteralMock(); - $rule = new Rule(array($literal, $literal2), 'job1', null); - $rule2 = new Rule(array($literal), 'job1', null); + $rule = new Rule($this->pool, array(1, 12), 'job1', null); + $rule2 = new Rule($this->pool, array(1), 'job1', null); $this->assertFalse($rule->isAssertion()); $this->assertTrue($rule2->isAssertion()); @@ -153,18 +133,13 @@ class RuleTest extends TestCase public function testToString() { - $literal = new Literal($this->getPackage('foo', '2.1'), true); - $literal2 = new Literal($this->getPackage('baz', '1.1'), false); + $repo = new ArrayRepository; + $repo->addPackage($p1 = $this->getPackage('foo', '2.1')); + $repo->addPackage($p2 = $this->getPackage('baz', '1.1')); + $this->pool->addRepository($repo); - $rule = new Rule(array($literal, $literal2), 'job1', null); + $rule = new Rule($this->pool, array($p1->getId(), -$p2->getId()), 'job1', null); $this->assertEquals('(-baz-1.1.0.0|+foo-2.1.0.0)', $rule->__toString()); } - - private function getLiteralMock() - { - return $this->getMockBuilder('Composer\DependencyResolver\Literal') - ->disableOriginalConstructor() - ->getMock(); - } } From a395bc04d713a7c6e0dc458567bbf2c72e3e24e9 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 20:48:12 +0200 Subject: [PATCH 15/27] Get rid of continue 2; and use array_filter instead of manual looping --- .../DependencyResolver/RuleWatchGraph.php | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index b1d2dc737..d1c652daf 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -57,15 +57,16 @@ class RuleWatchGraph if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { $ruleLiterals = $node->getRule()->getLiterals(); - foreach ($ruleLiterals as $ruleLiteral) { - if ($literal !== $ruleLiteral && + $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $conflictCallback) { + return $literal !== $ruleLiteral && $otherWatch !== $ruleLiteral && - !call_user_func($conflictCallback, $ruleLiteral)) { + !call_user_func($conflictCallback, $ruleLiteral); + }); - $this->moveWatch($literal, $ruleLiteral, $node); - - continue 2; - } + if ($alternativeLiterals) { + reset($alternativeLiterals); + $this->moveWatch($literal, current($alternativeLiterals), $node); + continue; } if (call_user_func($conflictCallback, $otherWatch)) { From 025581b365d47cc697d3d48e6ec8a5b6d01f3c84 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 20:50:21 +0200 Subject: [PATCH 16/27] Rename walkLiteral method to more explicitly say what it does --- src/Composer/DependencyResolver/RuleWatchGraph.php | 2 +- src/Composer/DependencyResolver/Solver.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index d1c652daf..e4bd71417 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -43,7 +43,7 @@ class RuleWatchGraph return isset($this->watches[$literal]); } - public function walkLiteral($literal, $level, $skipCallback, $conflictCallback, $decideCallback) + public function propagateLiteral($literal, $level, $skipCallback, $conflictCallback, $decideCallback) { if (!isset($this->watches[$literal])) { return null; diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 14328946a..45410ada2 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -318,7 +318,7 @@ class Solver $this->propagateIndex++; - $conflict = $this->watchGraph->walkLiteral( + $conflict = $this->watchGraph->propagateLiteral( $literal, $level, array($this, 'decisionsContain'), From dd527a4049622202c882a59cc6eec16a82a63245 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sat, 19 May 2012 21:49:48 +0200 Subject: [PATCH 17/27] Remove weak rules Since we no longer have suggest/recommend rules and no longer use any update or feature rules so packages are removed by default, we do not need weak rules anymore. --- src/Composer/DependencyResolver/Rule.php | 12 ----- src/Composer/DependencyResolver/Solver.php | 54 +++---------------- .../Test/DependencyResolver/RuleTest.php | 12 ----- 3 files changed, 8 insertions(+), 70 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index 93d00fc26..f5cb4ed01 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -35,7 +35,6 @@ class Rule protected $literals; protected $type; protected $id; - protected $weak; protected $job; @@ -53,7 +52,6 @@ class Rule $this->reasonData = $reasonData; $this->disabled = false; - $this->weak = false; $this->job = $job; @@ -139,16 +137,6 @@ class Rule return !$this->disabled; } - public function isWeak() - { - return $this->weak; - } - - public function setWeak($weak) - { - $this->weak = $weak; - } - public function getLiterals() { return $this->literals; diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 45410ada2..2afb01c7d 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -69,7 +69,7 @@ class Solver for ($ruleIndex = 0; $ruleIndex < count($this->rules); $ruleIndex++) { $rule = $this->rules->ruleById($ruleIndex); - if ($rule->isWeak() || !$rule->isAssertion() || $rule->isDisabled()) { + if (!$rule->isAssertion() || $rule->isDisabled()) { continue; } @@ -104,15 +104,15 @@ class Solver continue; } - // conflict with another job or update/feature rule + // conflict with another job $problem = new Problem; $problem->addRule($rule); $problem->addRule($conflict); - // push all of our rules (can only be feature or job rules) + // push all of our rules (can only be job rules) // asserting this literal on the problem stack foreach ($this->rules->getIteratorFor(RuleSet::TYPE_JOB) as $assertRule) { - if ($assertRule->isDisabled() || !$assertRule->isAssertion() || $assertRule->isWeak()) { + if ($assertRule->isDisabled() || !$assertRule->isAssertion()) { continue; } @@ -137,27 +137,6 @@ class Solver } $ruleIndex = -1; } - - foreach ($this->rules as $rule) { - if (!$rule->isWeak() || !$rule->isAssertion() || $rule->isDisabled()) { - continue; - } - - $literals = $rule->getLiterals(); - $literal = $literals[0]; - - if ($this->decisionMap[abs($literal)] == 0) { - $this->decide($literal, 1, $rule); - continue; - } - - if ($this->decisionsSatisfy($literals[0])) { - continue; - } - - // conflict, but this is a weak rule => disable - $this->disableProblem($rule); - } } protected function setupInstalledMap() @@ -553,7 +532,7 @@ class Solver return array($learnedLiterals[0], $ruleLevel, $newRule, $why); } - private function analyzeUnsolvableRule($problem, $conflictRule, &$lastWeakWhy) + private function analyzeUnsolvableRule($problem, $conflictRule) { $why = $conflictRule->getId(); @@ -562,7 +541,7 @@ class Solver $problemRules = $this->learnedPool[$learnedWhy]; foreach ($problemRules as $problemRule) { - $this->analyzeUnsolvableRule($problem, $problemRule, $lastWeakWhy); + $this->analyzeUnsolvableRule($problem, $problemRule); } return; } @@ -572,23 +551,15 @@ class Solver return; } - if ($conflictRule->isWeak()) { - /** TODO why > or < lastWeakWhy? */ - if (!$lastWeakWhy || $why > $lastWeakWhy->getId()) { - $lastWeakWhy = $conflictRule; - } - } - $problem->addRule($conflictRule); } private function analyzeUnsolvable($conflictRule, $disableRules) { - $lastWeakWhy = null; $problem = new Problem; $problem->addRule($conflictRule); - $this->analyzeUnsolvableRule($problem, $conflictRule, $lastWeakWhy); + $this->analyzeUnsolvableRule($problem, $conflictRule); $this->problems[] = $problem; @@ -618,7 +589,7 @@ class Solver $why = $this->decisionQueueWhy[$decisionId]; $problem->addRule($why); - $this->analyzeUnsolvableRule($problem, $why, $lastWeakWhy); + $this->analyzeUnsolvableRule($problem, $why); $literals = $why->getLiterals(); @@ -631,15 +602,6 @@ class Solver } } - if ($lastWeakWhy) { - array_pop($this->problems); - - $this->disableProblem($lastWeakWhy); - $this->resetSolver(); - - return 1; - } - if ($disableRules) { foreach ($this->problems[count($this->problems) - 1] as $reason) { $this->disableProblem($reason['rule']); diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 739dbd7ff..739e275ad 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -110,18 +110,6 @@ class RuleTest extends TestCase $this->assertFalse($rule->isEnabled()); } - public function testSetWeak() - { - $rule = new Rule($this->pool, array(), 'job1', null); - $rule->setWeak(true); - - $rule2 = new Rule($this->pool, array(), 'job1', null); - $rule2->setWeak(false); - - $this->assertTrue($rule->isWeak()); - $this->assertFalse($rule2->isWeak()); - } - public function testIsAssertions() { $rule = new Rule($this->pool, array(1, 12), 'job1', null); From 2dfea8a5e04b5e8e189ab4670e2beaf6ae89fb10 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 20 May 2012 15:44:15 +0200 Subject: [PATCH 18/27] Only consider undecided literals for selectAndInstall Fixes #707 --- src/Composer/DependencyResolver/Solver.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 2afb01c7d..9b0d12ad1 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -723,7 +723,9 @@ class Solver $noneSatisfied = false; break; } - $decisionQueue[] = $literal; + if ($literal > 0 && $this->undecided($literal)) { + $decisionQueue[] = $literal; + } } if ($noneSatisfied && count($decisionQueue)) { From fa7bd35413443485903fd0ff6594ee8d93384b68 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 20 May 2012 15:49:58 +0200 Subject: [PATCH 19/27] Make debug solver methods protected --- src/Composer/DependencyResolver/DebugSolver.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Composer/DependencyResolver/DebugSolver.php b/src/Composer/DependencyResolver/DebugSolver.php index b3ff0ca63..d02679e8c 100644 --- a/src/Composer/DependencyResolver/DebugSolver.php +++ b/src/Composer/DependencyResolver/DebugSolver.php @@ -17,7 +17,7 @@ namespace Composer\DependencyResolver; */ class DebugSolver extends Solver { - private function printDecisionMap() + protected function printDecisionMap() { echo "\nDecisionMap: \n"; foreach ($this->decisionMap as $packageId => $level) { @@ -35,16 +35,16 @@ class DebugSolver extends Solver echo "\n"; } - private function printDecisionQueue() + protected function printDecisionQueue() { echo "DecisionQueue: \n"; foreach ($this->decisionQueue as $i => $literal) { - echo ' ' . $literal . ' ' . $this->decisionQueueWhy[$i]." level ".$this->decisionMap[$literal->getPackageId()]."\n"; + echo ' ' . $this->pool->literalToString($literal) . ' ' . $this->decisionQueueWhy[$i]." level ".$this->decisionMap[abs($literal)]."\n"; } echo "\n"; } - private function printWatches() + protected function printWatches() { echo "\nWatches:\n"; foreach ($this->watches as $literalId => $watch) { From c869566868591af804d55181eda1167e9a72e3d0 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 20 May 2012 15:57:38 +0200 Subject: [PATCH 20/27] Make ruleHash a protected member of rules --- src/Composer/DependencyResolver/Rule.php | 2 +- .../Test/DependencyResolver/RuleTest.php | 27 ++++--------------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/src/Composer/DependencyResolver/Rule.php b/src/Composer/DependencyResolver/Rule.php index f5cb4ed01..c0ba0de76 100644 --- a/src/Composer/DependencyResolver/Rule.php +++ b/src/Composer/DependencyResolver/Rule.php @@ -38,7 +38,7 @@ class Rule protected $job; - public $ruleHash; + protected $ruleHash; public function __construct(Pool $pool, array $literals, $reason, $reasonData, $job = null) { diff --git a/tests/Composer/Test/DependencyResolver/RuleTest.php b/tests/Composer/Test/DependencyResolver/RuleTest.php index 739e275ad..8d4c732a2 100644 --- a/tests/Composer/Test/DependencyResolver/RuleTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleTest.php @@ -28,10 +28,9 @@ class RuleTest extends TestCase public function testGetHash() { - $rule = new Rule($this->pool, array(), 'job1', null); - $rule->ruleHash = '123'; + $rule = new Rule($this->pool, array(123), 'job1', null); - $this->assertEquals('123', $rule->getHash()); + $this->assertEquals(substr(md5('123'), 0, 5), $rule->getHash()); } public function testSetAndGetId() @@ -44,22 +43,8 @@ class RuleTest extends TestCase public function testEqualsForRulesWithDifferentHashes() { - $rule = new Rule($this->pool, array(), 'job1', null); - $rule->ruleHash = '123'; - - $rule2 = new Rule($this->pool, array(), 'job1', null); - $rule2->ruleHash = '321'; - - $this->assertFalse($rule->equals($rule2)); - } - - public function testEqualsForRulesWithDifferentLiterals() - { - $rule = new Rule($this->pool, array(1), 'job1', null); - $rule->ruleHash = '123'; - - $rule2 = new Rule($this->pool, array(12), 'job1', null); - $rule2->ruleHash = '123'; + $rule = new Rule($this->pool, array(1, 2), 'job1', null); + $rule2 = new Rule($this->pool, array(1, 3), 'job1', null); $this->assertFalse($rule->equals($rule2)); } @@ -67,14 +52,12 @@ class RuleTest extends TestCase public function testEqualsForRulesWithDifferLiteralsQuantity() { $rule = new Rule($this->pool, array(1, 12), 'job1', null); - $rule->ruleHash = '123'; $rule2 = new Rule($this->pool, array(1), 'job1', null); - $rule2->ruleHash = '123'; $this->assertFalse($rule->equals($rule2)); } - public function testEqualsForRulesWithThisSameLiterals() + public function testEqualsForRulesWithSameLiterals() { $rule = new Rule($this->pool, array(1, 12), 'job1', null); $rule2 = new Rule($this->pool, array(1, 12), 'job1', null); From e817a2e2d709bec24fffc8fbe7f83c7d5a7eb48c Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Sun, 20 May 2012 15:58:55 +0200 Subject: [PATCH 21/27] Move public members above protected members in rule watch node --- src/Composer/DependencyResolver/RuleWatchNode.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php index 1c8cfdeab..c0a2258e6 100644 --- a/src/Composer/DependencyResolver/RuleWatchNode.php +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -17,11 +17,11 @@ namespace Composer\DependencyResolver; */ class RuleWatchNode { - protected $rule; - public $watch1; public $watch2; + protected $rule; + public function __construct($rule) { $this->rule = $rule; From 265533d3900da3b93c0ccb77550c07f5e20b31e4 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 12:39:04 +0200 Subject: [PATCH 22/27] Rename watches array to watchChains to make clearer what they are --- .../DependencyResolver/RuleWatchGraph.php | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index e4bd71417..1262722ee 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -17,7 +17,7 @@ namespace Composer\DependencyResolver; */ class RuleWatchGraph { - protected $watches = array(); + protected $watchChains = array(); /** * Alters watch chains for a rule. @@ -30,28 +30,30 @@ class RuleWatchGraph } foreach (array($node->watch1, $node->watch2) as $literal) { - if (!isset($this->watches[$literal])) { - $this->watches[$literal] = new RuleWatchChain; + if (!isset($this->watchChains[$literal])) { + $this->watchChains[$literal] = new RuleWatchChain; } - $this->watches[$literal]->unshift($node); + $this->watchChains[$literal]->unshift($node); } } public function contains($literal) { - return isset($this->watches[$literal]); + return isset($this->watchChains[$literal]); } public function propagateLiteral($literal, $level, $skipCallback, $conflictCallback, $decideCallback) { - if (!isset($this->watches[$literal])) { + if (!isset($this->watchChains[$literal])) { return null; } - $this->watches[$literal]->rewind(); - while ($this->watches[$literal]->valid()) { - $node = $this->watches[$literal]->current(); + $chain = $this->watchChains[$literal]; + + $chain->rewind(); + while ($chain->valid()) { + $node = $chain->current(); $otherWatch = $node->getOtherWatch($literal); if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { @@ -76,7 +78,7 @@ class RuleWatchGraph call_user_func($decideCallback, $otherWatch, $level, $node->getRule()); } - $this->watches[$literal]->next(); + $chain->next(); } return null; @@ -84,12 +86,12 @@ class RuleWatchGraph protected function moveWatch($fromLiteral, $toLiteral, $node) { - if (!isset($this->watches[$toLiteral])) { - $this->watches[$toLiteral] = new RuleWatchChain; + if (!isset($this->watchChains[$toLiteral])) { + $this->watchChains[$toLiteral] = new RuleWatchChain; } $node->moveWatch($fromLiteral, $toLiteral); - $this->watches[$fromLiteral]->remove(); - $this->watches[$toLiteral]->unshift($node); + $this->watchChains[$fromLiteral]->remove(); + $this->watchChains[$toLiteral]->unshift($node); } } From 76d395099295dfa66f52745b2fc63d55e2832cda Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 18:14:38 +0200 Subject: [PATCH 23/27] Document the RuleWatchGraph --- .../DependencyResolver/RuleWatchGraph.php | 66 +++++++++++++++++-- src/Composer/DependencyResolver/Solver.php | 10 +-- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchGraph.php b/src/Composer/DependencyResolver/RuleWatchGraph.php index 1262722ee..93b80f1ad 100644 --- a/src/Composer/DependencyResolver/RuleWatchGraph.php +++ b/src/Composer/DependencyResolver/RuleWatchGraph.php @@ -13,6 +13,13 @@ namespace Composer\DependencyResolver; /** + * The RuleWatchGraph efficiently propagates decisions to other rules + * + * All rules generated for solving a SAT problem should be inserted into the + * graph. When a decision on a literal is made, the graph can be used to + * propagate the decision to all other rules involving the rule, leading to + * other trivial decisions resulting from unit clauses. + * * @author Nils Adermann */ class RuleWatchGraph @@ -20,8 +27,16 @@ class RuleWatchGraph protected $watchChains = array(); /** - * Alters watch chains for a rule. + * Inserts a rule node into the appropriate chains within the graph * + * The node is prepended to the watch chains for each of the two literals it + * watches. + * + * Assertions are skipped because they only depend on a single package and + * have no alternative literal that could be true, so there is no need to + * watch chnages in any literals. + * + * @param RuleWatchNode $node The rule node to be inserted into the graph */ public function insert(RuleWatchNode $node) { @@ -38,13 +53,41 @@ class RuleWatchGraph } } - public function contains($literal) + /** + * Propagates a decision on a literal to all rules watching the literal + * + * If a decision, e.g. +A has been made, then all rules containing -A, e.g. + * (-A|+B|+C) now need to satisfy at least one of the other literals, so + * that the rule as a whole becomes true, since with +A applied the rule + * is now (false|+B|+C) so essentialy (+B|+C). + * + * This means that all rules watching the literal -A need to be updated to + * watch 2 other literals which can still be satisfied instead. So literals + * that conflict with previously made decisions are not an option. + * + * Alternatively it can occur that a unit clause results: e.g. if in the + * above example the rule was (-A|+B), then A turning true means that + * B must now be decided true as well. + * + * @param int $decidedLiteral The literal which was decided (A in our example) + * @param int $level The level at which the decision took place and at which + * all resulting decisions should be made. + * @param Callable $decisionsSatisfyCallback A callback which checks if a + * literal has already been positively decided and the rule is thus + * already true and can be skipped. + * @param Callable $conflictCallback A callback which checks if a literal + * would conflict with previously made decisions on the same package + * @param Callable $decideCallback A callback which is responsible for + * registering decided literals resulting from unit clauses + * @return Rule|null If a conflict is found the conflicting rule is returned + */ + public function propagateLiteral($decidedLiteral, $level, $decisionsSatisfyCallback, $conflictCallback, $decideCallback) { - return isset($this->watchChains[$literal]); - } + // we invert the decided literal here, example: + // A was decided => (-A|B) now requires B to be true, so we look for + // rules which are fulfilled by -A, rather than A. + $literal = -$decidedLiteral; - public function propagateLiteral($literal, $level, $skipCallback, $conflictCallback, $decideCallback) - { if (!isset($this->watchChains[$literal])) { return null; } @@ -56,7 +99,7 @@ class RuleWatchGraph $node = $chain->current(); $otherWatch = $node->getOtherWatch($literal); - if (!$node->getRule()->isDisabled() && !call_user_func($skipCallback, $otherWatch)) { + if (!$node->getRule()->isDisabled() && !call_user_func($decisionsSatisfyCallback, $otherWatch)) { $ruleLiterals = $node->getRule()->getLiterals(); $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $conflictCallback) { @@ -84,6 +127,15 @@ class RuleWatchGraph return null; } + /** + * Moves a rule node from one watch chain to another + * + * The rule node's watched literals are updated accordingly. + * + * @param $fromLiteral A literal the node used to watch + * @param $toLiteral A literal the node should watch now + * @param $node The rule node to be moved + */ protected function moveWatch($fromLiteral, $toLiteral, $node) { if (!isset($this->watchChains[$toLiteral])) { diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 9b0d12ad1..0b564c905 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -289,16 +289,8 @@ class Solver protected function propagate($level) { while ($this->propagateIndex < count($this->decisionQueue)) { - // we invert the decided literal here, example: - // A was decided => (-A|B) now requires B to be true, so we look for - // rules which are fulfilled by -A, rather than A. - - $literal = -$this->decisionQueue[$this->propagateIndex]; - - $this->propagateIndex++; - $conflict = $this->watchGraph->propagateLiteral( - $literal, + $this->decisionQueue[$this->propagateIndex++], $level, array($this, 'decisionsContain'), array($this, 'decisionsConflict'), From 7f9c5ffeef9cac0219e8d3f942b51df12e7808c2 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 18:26:18 +0200 Subject: [PATCH 24/27] Add documentation to RuleWatchChain and RuleWatchNode --- .../DependencyResolver/RuleWatchChain.php | 18 ++++++++++ .../DependencyResolver/RuleWatchNode.php | 33 ++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/Composer/DependencyResolver/RuleWatchChain.php b/src/Composer/DependencyResolver/RuleWatchChain.php index 00ff0bc56..2fea0d6ee 100644 --- a/src/Composer/DependencyResolver/RuleWatchChain.php +++ b/src/Composer/DependencyResolver/RuleWatchChain.php @@ -13,18 +13,36 @@ namespace Composer\DependencyResolver; /** + * An extension of SplDoublyLinkedList with seek and removal of current element + * + * SplDoublyLinkedList only allows deleting a particular offset and has no + * method to set the internal iterator to a particular offset. + * * @author Nils Adermann */ class RuleWatchChain extends \SplDoublyLinkedList { protected $offset = 0; + /** + * Moves the internal iterator to the specified offset + * + * @param int $offset The offset to seek to. + */ public function seek($offset) { $this->rewind(); for ($i = 0; $i < $offset; $i++, $this->next()); } + /** + * Removes the current element from the list + * + * As SplDoublyLinkedList only allows deleting a particular offset and + * incorrectly sets the internal iterator if you delete the current value + * this method sets the internal iterator back to the following element + * using the seek method. + */ public function remove() { $offset = $this->key(); diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php index c0a2258e6..4bdeaade2 100644 --- a/src/Composer/DependencyResolver/RuleWatchNode.php +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -13,6 +13,10 @@ namespace Composer\DependencyResolver; /** + * Wrapper around a Rule which keeps track of the two literals it watches + * + * Used by RuleWatchGraph to store rules in two RuleWatchChains. + * * @author Nils Adermann */ class RuleWatchNode @@ -22,6 +26,11 @@ class RuleWatchNode protected $rule; + /** + * Creates a new node watching the first and second literals of the rule. + * + * @param Rule $rule The rule to wrap + */ public function __construct($rule) { $this->rule = $rule; @@ -33,7 +42,12 @@ class RuleWatchNode } /** - * Put watch2 on rule's literal with highest level + * Places the second watch on the rule's literal, decided at the highest level + * + * Useful for learned rules where the literal for the highest rule is most + * likely to quickly lead to further decisions. + * + * @param SplFixedArray $decisionMap A package to decision lookup table */ public function watch2OnHighest($decisionMap) { @@ -56,11 +70,22 @@ class RuleWatchNode } } + /** + * Returns the rule this node wraps + * + * @return Rule + */ public function getRule() { return $this->rule; } + /** + * Given one watched literal, this method returns the other watched literal + * + * @param int The watched literal that should not be returned + * @return int A literal + */ public function getOtherWatch($literal) { if ($this->watch1 == $literal) { @@ -70,6 +95,12 @@ class RuleWatchNode } } + /** + * Moves a watch from one literal to another + * + * @param int $from The previously watched literal + * @param int $to The literal to be watched now + */ public function moveWatch($from, $to) { if ($this->watch1 == $from) { From f193d61dfe47e54fa6ac067020a75278b14f92e6 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 18:32:22 +0200 Subject: [PATCH 25/27] Remove unecessary brackets --- src/Composer/DependencyResolver/RuleWatchNode.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleWatchNode.php b/src/Composer/DependencyResolver/RuleWatchNode.php index 4bdeaade2..88856974f 100644 --- a/src/Composer/DependencyResolver/RuleWatchNode.php +++ b/src/Composer/DependencyResolver/RuleWatchNode.php @@ -37,8 +37,8 @@ class RuleWatchNode $literals = $rule->getLiterals(); - $this->watch1 = (count($literals) > 0) ? $literals[0] : 0; - $this->watch2 = (count($literals) > 1) ? $literals[1] : 0; + $this->watch1 = count($literals) > 0 ? $literals[0] : 0; + $this->watch2 = count($literals) > 1 ? $literals[1] : 0; } /** From 99200af51b4b4c9906eeaa09a62b25f223f589cd Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 18:33:21 +0200 Subject: [PATCH 26/27] Correctly indent break statements --- src/Composer/DependencyResolver/RuleSetGenerator.php | 4 ++-- src/Composer/DependencyResolver/Solver.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 1538e5695..efed14163 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -258,7 +258,7 @@ class RuleSetGenerator $rule = $this->createInstallOneOfRule($job['packages'], Rule::RULE_JOB_INSTALL, $job); $this->addRule(RuleSet::TYPE_JOB, $rule); } - break; + break; case 'remove': // remove all packages with this name including uninstalled // ones to make sure none of them are picked as replacements @@ -266,7 +266,7 @@ class RuleSetGenerator $rule = $this->createRemoveRule($package, Rule::RULE_JOB_REMOVE, $job); $this->addRule(RuleSet::TYPE_JOB, $rule); } - break; + break; } } } diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 0b564c905..64e31799e 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -154,13 +154,13 @@ class Solver $this->updateMap[$package->getId()] = true; } } - break; + break; case 'update-all': foreach ($this->installedMap as $package) { $this->updateMap[$package->getId()] = true; } - break; + break; case 'install': if (!$job['packages']) { @@ -168,7 +168,7 @@ class Solver $problem->addRule(new Rule($this->pool, array(), null, null, $job)); $this->problems[] = $problem; } - break; + break; } } } From 21c7e219e21c4d7f0814e29be940566c0e84dbe8 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 21 May 2012 18:34:12 +0200 Subject: [PATCH 27/27] Change "else if" to "elseif" --- src/Composer/DependencyResolver/RuleSetGenerator.php | 2 +- src/Composer/DependencyResolver/Solver.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index efed14163..67bf2fb8b 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -204,7 +204,7 @@ class RuleSetGenerator if (($package instanceof AliasPackage) && $package->getAliasOf() === $provider) { $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createRequireRule($package, array($provider), Rule::RULE_PACKAGE_ALIAS, (string) $package)); - } else if (!$this->obsoleteImpossibleForAlias($package, $provider)) { + } elseif (!$this->obsoleteImpossibleForAlias($package, $provider)) { $reason = ($package->getName() == $provider->getName()) ? Rule::RULE_PACKAGE_SAME_NAME : Rule::RULE_PACKAGE_IMPLICIT_OBSOLETES; $this->addRule(RuleSet::TYPE_PACKAGE, $rule = $this->createConflictRule($package, $provider, $reason, (string) $package)); } diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 64e31799e..5ec682d57 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -381,7 +381,7 @@ class Solver throw new SolverBugException( "Trying to revert to invalid level ".(int) $newLevel." from level ".(int) $level."." ); - } else if (!$newRule) { + } elseif (!$newRule) { throw new SolverBugException( "No rule was learned from analyzing $rule at level $level." ); @@ -451,7 +451,7 @@ class Solver if (1 === $l) { $l1num++; - } else if ($level === $l) { + } elseif ($level === $l) { $num++; } else { // not level1 or conflict level, add to new rule @@ -660,7 +660,7 @@ class Solver if ($foundDisabled && $rule->isEnabled()) { $rule->disable(); - } else if (!$foundDisabled && $rule->isDisabled()) { + } elseif (!$foundDisabled && $rule->isDisabled()) { $rule->enable(); } }