From bc672deb32911fe4c2f903852b2652c0440c94d3 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 27 Jun 2011 00:11:57 +0200 Subject: [PATCH] Moving rule iteration logic to a separate RuleSet and RuleSetIterator class --- src/Composer/DependencyResolver/RuleSet.php | 129 ++++++++++ .../DependencyResolver/RuleSetIterator.php | 101 ++++++++ src/Composer/DependencyResolver/Solver.php | 237 +++++++++--------- .../RuleSetIteratorTest.php | 55 ++++ .../Test/DependencyResolver/RuleSetTest.php | 44 ++++ 5 files changed, 452 insertions(+), 114 deletions(-) create mode 100644 src/Composer/DependencyResolver/RuleSet.php create mode 100644 src/Composer/DependencyResolver/RuleSetIterator.php create mode 100644 tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php create mode 100644 tests/Composer/Test/DependencyResolver/RuleSetTest.php diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php new file mode 100644 index 000000000..dc21ccdb2 --- /dev/null +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -0,0 +1,129 @@ + + * 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 RuleSet +{ + // highest priority => lowest number + const TYPE_PACKAGE = 0; + const TYPE_JOB = 1; + const TYPE_UPDATE = 2; + const TYPE_FEATURE = 3; + const TYPE_WEAK = 4; + const TYPE_LEARNED = 5; + + protected static $types = array( + -1 => 'UNKNOWN', + self::TYPE_PACKAGE => 'PACKAGE', + self::TYPE_FEATURE => 'FEATURE', + self::TYPE_UPDATE => 'UPDATE', + self::TYPE_JOB => 'JOB', + self::TYPE_WEAK => 'WEAK', + self::TYPE_LEARNED => 'LEARNED', + ); + + protected $rules; + + public function __construct() + { + foreach ($this->getTypes() as $type) { + $this->rules[$type] = array(); + } + } + + public function add(Rule $rule, $type) + { + if (!isset(self::$types[$type])) + { + throw OutOfBoundsException('Unknown rule type: ' . $type); + } + + if (!isset($this->rules[$type])) + { + $this->rules[$type] = array(); + } + + $this->rules[$type][] = $rule; + $rule->setType($type); + } + + public function getRules() + { + return $this->rules; + } + + public function getIterator() + { + return new RuleSetIterator($this->getRules()); + } + + public function getIteratorFor($types) + { + if (!is_array($types)) + { + $types = array($types); + } + + $allRules = $this->getRules(); + $rules = array(); + + foreach ($types as $type) + { + $rules[$type] = $allRules[$type]; + } + + return new RuleSetIterator($rules); + } + + + public function getIteratorWithout($types) + { + if (!is_array($types)) + { + $types = array($types); + } + + $rules = $this->getRules(); + + foreach ($types as $type) + { + unset($rules[$type]); + } + + return new RuleSetIterator($rules); + } + + public function getTypes() + { + $types = self::$types; + unset($types[-1]); + return array_keys($types); + } + + public function __toString() + { + $string = "\n"; + foreach ($this->rules as $type => $rules) { + $string .= str_pad(self::$types[$type], 8, ' ') . ": "; + foreach ($rules as $rule) { + $string .= $rule; + } + $string .= "\n"; + } + + return $string; + } +} diff --git a/src/Composer/DependencyResolver/RuleSetIterator.php b/src/Composer/DependencyResolver/RuleSetIterator.php new file mode 100644 index 000000000..779175b25 --- /dev/null +++ b/src/Composer/DependencyResolver/RuleSetIterator.php @@ -0,0 +1,101 @@ + + * 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 RuleSetIterator implements \Iterator +{ + protected $rules; + protected $types; + + protected $currentOffset; + protected $currentTotalOffset; + protected $currentType; + protected $currentTypeOffset; + + public function __construct(array $rules) + { + $this->rules = $rules; + $this->types = array_keys($rules); + sort($this->types); + + $this->rewind(); + } + + public function current() + { + return $this->rules[$this->currentType][$this->currentOffset]; + } + + public function key() + { + return $this->currentType; + } + + public function getId() + { + return $this->currentTotalOffset; + } + + public function next() + { + $this->currentTotalOffset++; + $this->currentOffset++; + + if (!isset($this->rules[$this->currentType])) { + return; + } + + if ($this->currentOffset >= sizeof($this->rules[$this->currentType])) { + $this->currentOffset = 0; + + do { + $this->currentTypeOffset++; + + if (!isset($this->types[$this->currentTypeOffset])) { + $this->currentType = -1; + break; + } + + $this->currentType = $this->types[$this->currentTypeOffset]; + } while (isset($this->types[$this->currentTypeOffset]) && !sizeof($this->rules[$this->currentType])); + } + } + + public function rewind() + { + $this->currentOffset = 0; + $this->currentTotalOffset = 0; + + $this->currentTypeOffset = -1; + $this->currentType = -1; + + do { + $this->currentTypeOffset++; + + if (!isset($this->types[$this->currentTypeOffset])) { + $this->currentType = -1; + break; + } + + $this->currentType = $this->types[$this->currentTypeOffset]; + } while (isset($this->types[$this->currentTypeOffset]) && !sizeof($this->rules[$this->currentType])); + } + + public function valid() + { + return isset($this->rules[$this->currentType][$this->currentOffset]); + } +} diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 0bba115e1..87a131c0a 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -30,23 +30,6 @@ class Solver const RULE_PACKAGE_NOT_EXIST = 8; const RULE_PACKAGE_REQUIRES = 9; - const TYPE_PACKAGE = 0; - const TYPE_FEATURE = 1; - const TYPE_UPDATE = 2; - const TYPE_JOB = 3; - const TYPE_WEAK = 4; - const TYPE_LEARNED = 5; - - protected static $types = array( - -1 => 'UNKNOWN', - self::TYPE_PACKAGE => 'PACKAGE', - self::TYPE_FEATURE => 'FEATURE', - self::TYPE_UPDATE => 'UPDATE', - self::TYPE_JOB => 'JOB', - self::TYPE_WEAK => 'WEAK', - self::TYPE_LEARNED => 'LEARNED', - ); - protected $policy; protected $pool; protected $installed; @@ -64,15 +47,7 @@ class Solver $this->policy = $policy; $this->pool = $pool; $this->installed = $installed; - $this->rules = array( - // order matters here! further down => higher priority - self::TYPE_LEARNED => array(), - self::TYPE_WEAK => array(), - self::TYPE_FEATURE => array(), - self::TYPE_UPDATE => array(), - self::TYPE_JOB => array(), - self::TYPE_PACKAGE => array(), - ); + $this->rules = new RuleSet; } /** @@ -241,16 +216,13 @@ class Solver */ private function addRule($type, Rule $newRule = null) { if ($newRule) { - foreach ($this->rules as $rules) { - foreach ($rules as $rule) { - if ($rule->equals($newRule)) { - return; - } + foreach ($this->rules->getIterator() as $rule) { + if ($rule->equals($newRule)) { + return; } } - $newRule->setType($type); - $this->rules[$type][] = $newRule; + $this->rules->add($newRule, $type); } } @@ -273,7 +245,7 @@ class Solver } if (!$dontFix && !$this->policy->installable($this, $this->pool, $this->installed, $package)) { - $this->addRule(self::TYPE_PACKAGE, $this->createRemoveRule($package, self::RULE_NOT_INSTALLABLE, (string) $package)); + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRemoveRule($package, self::RULE_NOT_INSTALLABLE, (string) $package)); continue; } @@ -299,7 +271,7 @@ class Solver } } - $this->addRule(self::TYPE_PACKAGE, $this->createRequireRule($package, $possibleRequires, self::RULE_PACKAGE_REQUIRES, (string) $link)); + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRequireRule($package, $possibleRequires, self::RULE_PACKAGE_REQUIRES, (string) $link)); foreach ($possibleRequires as $require) { $workQueue->enqueue($require); @@ -314,7 +286,7 @@ class Solver continue; } - $this->addRule(self::TYPE_PACKAGE, $this->createConflictRule($package, $conflict, self::RULE_PACKAGE_CONFLICT, (string) $link)); + $this->addRule(RuleSet::TYPE_PACKAGE, $this->createConflictRule($package, $conflict, self::RULE_PACKAGE_CONFLICT, (string) $link)); } } @@ -359,29 +331,25 @@ class Solver */ private function makeWatches() { - foreach ($this->rules as $type => $rules) { - foreach ($rules as $i => $rule) { - - // skip simple assertions of the form (A) or (-A) - if ($rule->isAssertion()) { - continue; - } - - 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; - + foreach ($this->rules as $rule) { + // skip simple assertions of the form (A) or (-A) + if ($rule->isAssertion()) { + continue; } + + 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; } } @@ -400,44 +368,39 @@ class Solver { // do we need to decide a SYSTEMSOLVABLE at level 1? - foreach ($this->rules as $type => $rules) { - if (self::TYPE_WEAK === $type) { + foreach ($this->rules->getIteratorWithout(RuleSet::TYPE_WEAK) as $rule) { + if (!$rule->isAssertion() || $rule->isDisabled()) { continue; } - foreach ($rules as $rule) { - if (!$rule->isAssertion() || $rule->isDisabled()) { - continue; - } - $literals = $rule->getLiterals(); - $literal = $literals[0]; + $literals = $rule->getLiterals(); + $literal = $literals[0]; - if (!$this->decided($literal->getPackage())) { - $this->decisionQueue[] = $literal; - $this->decisionQueueWhy[] = $rule; - $this->addDecision($literal, 1); - continue; - } + if (!$this->decided($literal->getPackage())) { + $this->decisionQueue[] = $literal; + $this->decisionQueueWhy[] = $rule; + $this->addDecision($literal, 1); + continue; + } - if ($this->decisionsSatisfy($literal)) { - continue; - } + if ($this->decisionsSatisfy($literal)) { + continue; + } - // found a conflict - if (self::TYPE_LEARNED === $type) { - $rule->disable(); - } + // found a conflict + if (RuleSet::TYPE_LEARNED === $rule->getType()) { + $rule->disable(); + } - $conflict = $this->findDecisionRule($literal->getPackage()); - // todo: handle conflict with systemsolvable? + $conflict = $this->findDecisionRule($literal->getPackage()); + // todo: handle conflict with systemsolvable? - if ($conflict && self::TYPE_PACKAGE === $conflict->getType()) { + if ($conflict && RuleSet::TYPE_PACKAGE === $conflict->getType()) { - } } } - foreach ($this->rules[self::TYPE_WEAK] as $rule) { + foreach ($this->rules->getIteratorFor(RuleSet::TYPE_WEAK) as $rule) { if (!$rule->isAssertion() || $rule->isDisabled()) { continue; } @@ -520,13 +483,13 @@ class Solver if ($rule->equals($featureRule)) { if ($this->policy->allowUninstall()) { - $this->addRule(self::TYPE_WEAK, $featureRule); + $this->addRule(RuleSet::TYPE_WEAK, $featureRule); } else { - $this->addRule(self::TYPE_UPDATE, $rule); + $this->addRule(RuleSet::TYPE_UPDATE, $rule); } } else if ($this->policy->allowUninstall()) { - $this->addRule(self::TYPE_WEAK, $featureRule); - $this->addRule(self::TYPE_WEAK, $rule); + $this->addRule(RuleSet::TYPE_WEAK, $featureRule); + $this->addRule(RuleSet::TYPE_WEAK, $rule); } } @@ -534,7 +497,7 @@ class Solver switch ($job['cmd']) { case 'install': $rule = $this->createInstallOneOfRule($job['packages'], self::RULE_JOB_INSTALL, $job['packageName']); - $this->addRule(self::TYPE_JOB, $rule); + $this->addRule(RuleSet::TYPE_JOB, $rule); //$this->ruleToJob[$rule] = $job; break; case 'remove': @@ -544,7 +507,7 @@ class Solver // todo: cleandeps foreach ($job['packages'] as $package) { $rule = $this->createRemoveRule($package, self::RULE_JOB_REMOVE); - $this->addRule(self::TYPE_JOB, $rule); + $this->addRule(RuleSet::TYPE_JOB, $rule); //$this->ruleToJob[$rule] = $job; } break; @@ -555,7 +518,7 @@ class Solver } else { $rule = $this->createRemoveRule($package, self::RULE_JOB_LOCK); } - $this->addRule(self::TYPE_JOB, $rule); + $this->addRule(RuleSet::TYPE_JOB, $rule); //$this->ruleToJob[$rule] = $job; } break; @@ -597,18 +560,6 @@ class Solver return $transaction; } - public function printRules() - { - print "\n"; - foreach ($this->rules as $type => $rules) { - print str_pad(self::$types[$type], 8, ' ') . ": "; - foreach ($rules as $rule) { - print $rule; - } - print "\n"; - } - } - protected $decisionQueue = array(); protected $propagateIndex; protected $decisionMap = array(); @@ -718,6 +669,10 @@ class Solver // /* foreach rule where 'pkg' is now FALSE */ //for (rp = watches + pkg; *rp; rp = next_rp) + if (!isset($this->watches[$literal->getId()])) { + continue; + } + for ($rule = $this->watches[$literal->getId()]; $rule !== null; $rule = $nextRule) { $nextRule = $rule->getNext($literal); @@ -790,9 +745,63 @@ class Solver return $this->setPropagateLearn($level, $literals[0], $disableRules, $rule); } + private function analyzeUnsolvableRule($rule, &$lastWeak) + { + //if ($this->learntRules && + +/* + Pool *pool = solv->pool; + int i; + Id why = r - solv->rules; + + IF_POOLDEBUG (SAT_DEBUG_UNSOLVABLE) + solver_printruleclass(solv, SAT_DEBUG_UNSOLVABLE, r); + if (solv->learntrules && why >= solv->learntrules) + { + for (i = solv->learnt_why.elements[why - solv->learntrules]; solv->learnt_pool.elements[i]; i++) + if (solv->learnt_pool.elements[i] > 0) + analyze_unsolvable_rule(solv, solv->rules + solv->learnt_pool.elements[i], lastweakp); + return; + } + if (MAPTST(&solv->weakrulemap, why)) + if (!*lastweakp || why > *lastweakp) + *lastweakp = why; + /* do not add rpm rules to problem * + if (why < solv->rpmrules_end) + return; + /* turn rule into problem * + if (why >= solv->jobrules && why < solv->jobrules_end) + why = -(solv->ruletojob.elements[why - solv->jobrules] + 1); + /* normalize dup/infarch rules * + if (why > solv->infarchrules && why < solv->infarchrules_end) + { + Id name = pool->solvables[-solv->rules[why].p].name; + while (why > solv->infarchrules && pool->solvables[-solv->rules[why - 1].p].name == name) + why--; + } + if (why > solv->duprules && why < solv->duprules_end) + { + Id name = pool->solvables[-solv->rules[why].p].name; + while (why > solv->duprules && pool->solvables[-solv->rules[why - 1].p].name == name) + why--; + } + + /* return if problem already countains our rule * + if (solv->problems.count) + { + for (i = solv->problems.count - 1; i >= 0; i--) + if (solv->problems.elements[i] == 0) /* end of last problem reached? * + break; + else if (solv->problems.elements[i] == why) + return; + } + queue_push(&solv->problems, why); +*/ + } + private function analyzeUnsolvable($conflictRule, $disableRules) { - return false; + $this->analyzeUnsolvableRule($conflictRule, $lastWeak); } private function runSat($disableRules = true, $installRecommended = false) @@ -836,8 +845,8 @@ class Solver // handle job rules if ($level < $systemLevel) { - $ruleIndex = 0; - foreach ($this->rules[self::TYPE_JOB] as $ruleIndex => $rule) { + $iterator = $this->rules->getIteratorFor(RuleSet::TYPE_JOB); + foreach ($iterator as $rule) { if ($rule->isEnabled()) { $decisionQueue = array(); $noneSatisfied = true; @@ -886,7 +895,8 @@ class Solver $systemLevel = $level + 1; // jobs left - if ($ruleIndex + 1 < count($this->rules[Solver::TYPE_JOB])) { + $iterator->next(); + if ($iterator->valid()) { continue; } } @@ -916,13 +926,14 @@ class Solver } $rule = null; + /** TODO: huh at package id?!? if (isset($this->rules[Solver::TYPE_UPDATE][$literal->getPackageId()])) { $rule = $this->rules[Solver::TYPE_UPDATE][$literal->getPackageId()]; } if ((!$rule || $rule->isDisabled()) && isset($this->rules[Solver::TYPE_FEATURE][$literal->getPackageId()])) { $rule = $this->rules[Solver::TYPE_FEATURE][$literal->getPackageId()]; - } + }**/ if (!$rule || $rule->isDisabled()) { continue; @@ -1016,14 +1027,12 @@ class Solver $systemLevel = $level; } - foreach ($this->rules as $ruleType => $rules) { - foreach ($rules as $rule) { - if ($rule->isEnabled()) { - $decisionQueue = array(); - } + foreach ($this->rules->getIterator() as $rule) { + if ($rule->isEnabled()) { + $decisionQueue = array(); } } - +echo $this->rules; // // /* // * decide diff --git a/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php new file mode 100644 index 000000000..2daa64eb6 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php @@ -0,0 +1,55 @@ + + * 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\Rule; +use Composer\DependencyResolver\RuleSet; +use Composer\DependencyResolver\RuleSetIterator; + +class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase +{ + protected $rules; + + protected function setUp() + { + $this->rules = array( + RuleSet::TYPE_JOB => array( + new Rule(array(), 'job1', null), + new Rule(array(), 'job2', null), + ), + RuleSet::TYPE_UPDATE => array( + new Rule(array(), 'update1', null), + ), + RuleSet::TYPE_PACKAGE => array(), + ); + } + + public function testForeach() + { + $ruleSetIterator = new RuleSetIterator($this->rules); + + $result = array(); + foreach ($ruleSetIterator as $rule) + { + $result[] = $rule; + } + + $expected = array( + $this->rules[RuleSet::TYPE_JOB][0], + $this->rules[RuleSet::TYPE_JOB][1], + $this->rules[RuleSet::TYPE_UPDATE][0], + ); + + $this->assertEquals($expected, $result); + } +} diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php new file mode 100644 index 000000000..08cd39080 --- /dev/null +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -0,0 +1,44 @@ + + * 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\Rule; +use Composer\DependencyResolver\RuleSet; + +class RuleSetTest extends \PHPUnit_Framework_TestCase +{ + public function testAdd() + { + $rules = array( + RuleSet::TYPE_PACKAGE => array(), + RuleSet::TYPE_JOB => array( + new Rule(array(), 'job1', null), + new Rule(array(), 'job2', null), + ), + RuleSet::TYPE_UPDATE => array( + new Rule(array(), 'update1', null), + ), + RuleSet::TYPE_FEATURE => array(), + RuleSet::TYPE_WEAK => array(), + RuleSet::TYPE_LEARNED => array(), + ); + + $ruleSet = new RuleSet; + + $ruleSet->add($rules[RuleSet::TYPE_JOB][0], RuleSet::TYPE_JOB); + $ruleSet->add($rules[RuleSet::TYPE_UPDATE][0], RuleSet::TYPE_UPDATE); + $ruleSet->add($rules[RuleSet::TYPE_JOB][1], RuleSet::TYPE_JOB); + + $this->assertEquals($rules, $ruleSet->getRules()); + } +}