Merge remote-tracking branch 'naderman/solver-refactor'

src/Composer/DependencyResolver/Decisions.php

@@ -17,7 +17,7 @@ namespace Composer\DependencyResolver;
  *
  * @author Nils Adermann <naderman@naderman.de>
  */
-class Decisions implements \Iterator
+class Decisions implements \Iterator, \Countable
 {
     const DECISION_LITERAL = 0;
     const DECISION_REASON = 1;
@@ -25,7 +25,6 @@ class Decisions implements \Iterator
     protected $pool;
     protected $decisionMap;
     protected $decisionQueue = array();
-    protected $decisionQueueFree = array();
 
     public function __construct($pool)
     {
@@ -38,47 +37,13 @@ class Decisions implements \Iterator
         }
     }
 
-    protected function addDecision($literal, $level)
-    {
-        $packageId = abs($literal);
-
-        $previousDecision = $this->decisionMap[$packageId];
-        if ($previousDecision != 0) {
-            $literalString = $this->pool->literalToString($literal);
-            $package = $this->pool->literalToPackage($literal);
-            throw new SolverBugException(
-                "Trying to decide $literalString on level $level, even though $package was previously decided as ".(int) $previousDecision."."
-            );
-        }
-
-        if ($literal > 0) {
-            $this->decisionMap[$packageId] = $level;
-        } else {
-            $this->decisionMap[$packageId] = -$level;
-        }
-    }
-
-    public function decide($literal, $level, $why, $addToFreeQueue = false)
+    public function decide($literal, $level, $why)
     {
         $this->addDecision($literal, $level);
         $this->decisionQueue[] = array(
             self::DECISION_LITERAL => $literal,
             self::DECISION_REASON => $why,
         );
-
-        if ($addToFreeQueue) {
-            $this->decisionQueueFree[count($this->decisionQueue) - 1] = true;
-        }
-    }
-
-    public function contain($literal)
-    {
-        $packageId = abs($literal);
-
-        return (
-            $this->decisionMap[$packageId] > 0 && $literal > 0 ||
-            $this->decisionMap[$packageId] < 0 && $literal < 0
-        );
     }
 
     public function satisfy($literal)
@@ -159,15 +124,12 @@ class Decisions implements \Iterator
         while ($decision = array_pop($this->decisionQueue)) {
             $this->decisionMap[abs($decision[self::DECISION_LITERAL])] = 0;
         }
-
-        $this->decisionQueueFree = array();
     }
 
     public function resetToOffset($offset)
     {
         while (count($this->decisionQueue) > $offset + 1) {
             $decision = array_pop($this->decisionQueue);
-            unset($this->decisionQueueFree[count($this->decisionQueue)]);
             $this->decisionMap[abs($decision[self::DECISION_LITERAL])] = 0;
         }
     }
@@ -178,9 +140,9 @@ class Decisions implements \Iterator
         array_pop($this->decisionQueue);
     }
 
-    public function getMaxOffset()
+    public function count()
     {
-        return count($this->decisionQueue) - 1;
+        return count($this->decisionQueue);
     }
 
     public function rewind()
@@ -212,4 +174,24 @@ class Decisions implements \Iterator
     {
         return count($this->decisionQueue) === 0;
     }
+
+    protected function addDecision($literal, $level)
+    {
+        $packageId = abs($literal);
+
+        $previousDecision = $this->decisionMap[$packageId];
+        if ($previousDecision != 0) {
+            $literalString = $this->pool->literalToString($literal);
+            $package = $this->pool->literalToPackage($literal);
+            throw new SolverBugException(
+                "Trying to decide $literalString on level $level, even though $package was previously decided as ".(int) $previousDecision."."
+            );
+        }
+
+        if ($literal > 0) {
+            $this->decisionMap[$packageId] = $level;
+        } else {
+            $this->decisionMap[$packageId] = -$level;
+        }
+    }
 }

src/Composer/DependencyResolver/Problem.php

@@ -66,10 +66,10 @@ class Problem
                     $ext = substr($job['packageName'], 4);
                     $error = extension_loaded($ext) ? 'has the wrong version ('.phpversion($ext).') installed' : 'is missing from your system';
 
-                    return 'The requested PHP extension "'.$job['packageName'].'" '.$this->constraintToText($job['constraint']).$error.'.';
+                    return 'The requested PHP extension '.$job['packageName'].$this->constraintToText($job['constraint']).' '.$error.'.';
                 }
 
-                return 'The requested package "'.$job['packageName'].'" '.$this->constraintToText($job['constraint']).'could not be found.';
+                return 'The requested package '.$job['packageName'].$this->constraintToText($job['constraint']).' could not be found.';
             }
         }
 
@@ -115,14 +115,27 @@ class Problem
     {
         switch ($job['cmd']) {
             case 'install':
-                return 'Installation of package "'.$job['packageName'].'" '.$this->constraintToText($job['constraint']).'was requested. Satisfiable by packages ['.implode(', ', $job['packages']).'].';
+                if (!$job['packages']) {
+                    return 'No package found to satisfy install request for '.$job['packageName'].$this->constraintToText($job['constraint']);
+                }
+
+                return 'Installation request for '.$job['packageName'].$this->constraintToText($job['constraint']).': Satisfiable by ['.$this->getPackageList($job['packages']).'].';
             case 'update':
-                return 'Update of package "'.$job['packageName'].'" '.$this->constraintToText($job['constraint']).'was requested.';
+                return 'Update request for '.$job['packageName'].$this->constraintToText($job['constraint']).'.';
             case 'remove':
-                return 'Removal of package "'.$job['packageName'].'" '.$this->constraintToText($job['constraint']).'was requested.';
+                return 'Removal request for '.$job['packageName'].$this->constraintToText($job['constraint']).'';
         }
 
-        return 'Job(cmd='.$job['cmd'].', target='.$job['packageName'].', packages=['.implode(', ', $job['packages']).'])';
+        return 'Job(cmd='.$job['cmd'].', target='.$job['packageName'].', packages=['.$this->packageList($job['packages']).'])';
+    }
+
+    protected function getPackageList($packages)
+    {
+        return implode(', ', array_map(function ($package) {
+                return $package->getPrettyString();
+            },
+            $packages
+        ));
     }
 
     /**
@@ -133,6 +146,6 @@ class Problem
      */
     protected function constraintToText($constraint)
     {
-        return ($constraint) ? 'with constraint '.$constraint.' ' : '';
+        return ($constraint) ? ' '.$constraint : '';
     }
 }

src/Composer/DependencyResolver/Rule.php

@@ -171,7 +171,7 @@ class Rule
                 $package1 = $this->pool->literalToPackage($this->literals[0]);
                 $package2 = $this->pool->literalToPackage($this->literals[1]);
 
-                return 'Package "'.$package1.'" conflicts with "'.$package2.'"';
+                return 'Package '.$package1->getPrettyString().' conflicts with '.$package2->getPrettyString().'"';
 
             case self::RULE_PACKAGE_REQUIRES:
                 $literals = $this->literals;

src/Composer/DependencyResolver/RuleWatchGraph.php

@@ -94,7 +94,7 @@ class RuleWatchGraph
             $node = $chain->current();
             $otherWatch = $node->getOtherWatch($literal);
 
-            if (!$node->getRule()->isDisabled() && !$decisions->contain($otherWatch)) {
+            if (!$node->getRule()->isDisabled() && !$decisions->satisfy($otherWatch)) {
                 $ruleLiterals = $node->getRule()->getLiterals();
 
                 $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $decisions) {

src/Composer/DependencyResolver/Solver.php

@@ -19,6 +19,9 @@ use Composer\Repository\RepositoryInterface;
  */
 class Solver
 {
+    const BRANCH_LITERALS = 0;
+    const BRANCH_LEVEL = 1;
+
     protected $policy;
     protected $pool;
     protected $installed;
@@ -48,7 +51,7 @@ class Solver
     // aka solver_makeruledecisions
     private function makeAssertionRuleDecisions()
     {
-        $decisionStart = $this->decisions->getMaxOffset();
+        $decisionStart = count($this->decisions) - 1;
 
         for ($ruleIndex = 0; $ruleIndex < count($this->rules); $ruleIndex++) {
             $rule = $this->rules->ruleById($ruleIndex);
@@ -242,16 +245,10 @@ class Solver
             }
 
             $this->decisions->revertLast();
-            $this->propagateIndex = $this->decisions->getMaxOffset() + 1;
+            $this->propagateIndex = count($this->decisions);
         }
 
-        while (!empty($this->branches)) {
-            list($literals, $branchLevel) = $this->branches[count($this->branches) - 1];
-
-            if ($branchLevel < $level) {
-                break;
-            }
-
+        while (!empty($this->branches) && $this->branches[count($this->branches) - 1][self::BRANCH_LEVEL] >= $level) {
             array_pop($this->branches);
         }
     }
@@ -275,7 +272,7 @@ class Solver
     {
         $level++;
 
-        $this->decisions->decide($literal, $level, $rule, true);
+        $this->decisions->decide($literal, $level, $rule);
 
         while (true) {
             $rule = $this->propagate($level);
@@ -343,7 +340,7 @@ class Solver
         $seen = array();
         $learnedLiterals = array(null);
 
-        $decisionId = $this->decisions->getMaxOffset() + 1;
+        $decisionId = count($this->decisions);
 
         $this->learnedPool[] = array();
 
@@ -483,12 +480,7 @@ class Solver
             $seen[abs($literal)] = true;
         }
 
-        $decisionId = $this->decisions->getMaxOffset() + 1;
-
-        while ($decisionId > 0) {
-            $decisionId--;
-
-            $decision = $this->decisions->atOffset($decisionId);
+        foreach ($this->decisions as $decision) {
             $literal = $decision[Decisions::DECISION_LITERAL];
 
             // skip literals that are not in this rule
@@ -601,7 +593,6 @@ class Solver
 
         $level = 1;
         $systemLevel = $level + 1;
-        $minimizationSteps = 0;
         $installedPos = 0;
 
         while (true) {
@@ -759,9 +750,8 @@ class Solver
                 }
 
                 if ($lastLiteral) {
-                    unset($this->branches[$lastBranchIndex][0][$lastBranchOffset]);
-                    $this->branches[$lastBranchIndex][0] = array_values($this->branches[$lastBranchIndex][0]);
-                    $minimizationSteps++;
+                    unset($this->branches[$lastBranchIndex][self::BRANCH_LITERALS][$lastBranchOffset]);
+                    array_values($this->branches[$lastBranchIndex][self::BRANCH_LITERALS]);
 
                     $level = $lastLevel;
                     $this->revert($level);

src/Composer/Package/BasePackage.php

@@ -201,6 +201,11 @@ abstract class BasePackage implements PackageInterface
         return $this->getUniqueName();
     }
 
+    public function getPrettyString()
+    {
+        return $this->getPrettyName().'-'.$this->getPrettyVersion();
+    }
+
     public function __clone()
     {
         $this->repository = null;

src/Composer/Package/PackageInterface.php

@@ -356,4 +356,11 @@ interface PackageInterface
      * @return string
      */
     public function __toString();
+
+    /**
+     * Converts the package into a pretty readable string
+     *
+     * @return string
+     */
+    public function getPrettyString();
 }

tests/Composer/Test/DependencyResolver/SolverTest.php

@@ -77,7 +77,7 @@ class SolverTest extends TestCase
         } catch (SolverProblemsException $e) {
             $problems = $e->getProblems();
             $this->assertEquals(1, count($problems));
-            $this->assertEquals('The requested package "b" with constraint == 1.0.0.0 could not be found.', (string) $problems[0]);
+            $this->assertEquals('The requested package b == 1.0.0.0 could not be found.', (string) $problems[0]);
         }
     }