From ec61297122258f21a030d54844f3c9827bef1e3d Mon Sep 17 00:00:00 2001 From: Volker Dusch Date: Sat, 18 Feb 2012 18:31:19 +0100 Subject: [PATCH 1/2] Remove the allowUninstall policy option and clean up the only usage --- src/Composer/DependencyResolver/DefaultPolicy.php | 5 ----- src/Composer/DependencyResolver/PolicyInterface.php | 1 - src/Composer/DependencyResolver/Solver.php | 11 +++-------- 3 files changed, 3 insertions(+), 14 deletions(-) diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index f06b4104b..63699d883 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -21,11 +21,6 @@ use Composer\Package\LinkConstraint\VersionConstraint; */ class DefaultPolicy implements PolicyInterface { - public function allowUninstall() - { - return true; - } - public function versionCompare(PackageInterface $a, PackageInterface $b, $operator) { $constraint = new VersionConstraint($operator, $b->getVersion()); diff --git a/src/Composer/DependencyResolver/PolicyInterface.php b/src/Composer/DependencyResolver/PolicyInterface.php index 45309a081..7c26ab63f 100644 --- a/src/Composer/DependencyResolver/PolicyInterface.php +++ b/src/Composer/DependencyResolver/PolicyInterface.php @@ -20,7 +20,6 @@ use Composer\Package\PackageInterface; */ interface PolicyInterface { - function allowUninstall(); function versionCompare(PackageInterface $a, PackageInterface $b, $operator); function findUpdatePackages(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); function installable(Solver $solver, Pool $pool, array $installedMap, PackageInterface $package); diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 680b87591..25d186073 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -986,14 +986,9 @@ class Solver $updates = $this->policy->findUpdatePackages($this, $this->pool, $this->installedMap, $package); $rule = $this->createUpdateRule($package, $updates, self::RULE_INTERNAL_ALLOW_UPDATE, (string) $package); - if ($this->policy->allowUninstall()) { - $rule->setWeak(true); - $this->addRule(RuleSet::TYPE_FEATURE, $rule); - $this->packageToFeatureRule[$package->getId()] = $rule; - } else { - $this->addRule(RuleSet::TYPE_UPDATE, $rule); - $this->packageToUpdateRule[$package->getId()] = $rule; - } + $rule->setWeak(true); + $this->addRule(RuleSet::TYPE_FEATURE, $rule); + $this->packageToFeatureRule[$package->getId()] = $rule; } foreach ($this->jobs as $job) { From 3fb75faa7589e8967a63ca420319b19225e99aff Mon Sep 17 00:00:00 2001 From: Volker Dusch Date: Sat, 18 Feb 2012 19:19:49 +0100 Subject: [PATCH 2/2] Now that no more update rules are created the code that handles them can be removed too. Also adapted the tests that used TYPE_UPDATE exemplarily to use TYPE_FEATURE. --- src/Composer/DependencyResolver/RuleSet.php | 2 -- src/Composer/DependencyResolver/Solver.php | 17 ++--------------- .../RuleSetIteratorTest.php | 6 +++--- .../Test/DependencyResolver/RuleSetTest.php | 19 +++++++++---------- 4 files changed, 14 insertions(+), 30 deletions(-) diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php index 6d79a9678..1f444c788 100644 --- a/src/Composer/DependencyResolver/RuleSet.php +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -20,7 +20,6 @@ class RuleSet implements \IteratorAggregate, \Countable // highest priority => lowest number const TYPE_PACKAGE = 0; const TYPE_JOB = 1; - const TYPE_UPDATE = 2; const TYPE_FEATURE = 3; const TYPE_CHOICE = 4; const TYPE_LEARNED = 5; @@ -29,7 +28,6 @@ class RuleSet implements \IteratorAggregate, \Countable -1 => 'UNKNOWN', self::TYPE_PACKAGE => 'PACKAGE', self::TYPE_FEATURE => 'FEATURE', - self::TYPE_UPDATE => 'UPDATE', self::TYPE_JOB => 'JOB', self::TYPE_CHOICE => 'CHOICE', self::TYPE_LEARNED => 'LEARNED', diff --git a/src/Composer/DependencyResolver/Solver.php b/src/Composer/DependencyResolver/Solver.php index 25d186073..9ff6b0764 100644 --- a/src/Composer/DependencyResolver/Solver.php +++ b/src/Composer/DependencyResolver/Solver.php @@ -52,7 +52,6 @@ class Solver protected $decisionMap; protected $installedMap; - protected $packageToUpdateRule = array(); protected $packageToFeatureRule = array(); public function __construct(PolicyInterface $policy, Pool $pool, RepositoryInterface $installed) @@ -508,7 +507,7 @@ class Solver // push all of our rules (can only be feature or job rules) // asserting this literal on the problem stack - foreach ($this->rules->getIteratorFor(array(RuleSet::TYPE_JOB, RuleSet::TYPE_UPDATE, RuleSet::TYPE_FEATURE)) as $assertRule) { + foreach ($this->rules->getIteratorFor(array(RuleSet::TYPE_JOB, RuleSet::TYPE_FEATURE)) as $assertRule) { if ($assertRule->isDisabled() || !$assertRule->isAssertion() || $assertRule->isWeak()) { continue; } @@ -882,11 +881,6 @@ class Solver protected function disableUpdateRule($package) { - // find update & feature rule and disable - if (isset($this->packageToUpdateRule[$package->getId()])) { - $this->packageToUpdateRule[$package->getId()]->disable(); - } - if (isset($this->packageToFeatureRule[$package->getId()])) { $this->packageToFeatureRule[$package->getId()]->disable(); } @@ -1056,9 +1050,6 @@ class Solver 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()); - } if (isset($this->packageToFeatureRule[$package->getId()])) { $literals = array_merge($literals, $this->packageToFeatureRule[$package->getId()]->getLiterals()); } @@ -1808,11 +1799,7 @@ class Solver $rule = null; - if (isset($this->packageToUpdateRule[$literal->getPackageId()])) { - $rule = $this->packageToUpdateRule[$literal->getPackageId()]; - } - - if ((!$rule || $rule->isDisabled()) && isset($this->packageToFeatureRule[$literal->getPackageId()])) { + if (isset($this->packageToFeatureRule[$literal->getPackageId()])) { $rule = $this->packageToFeatureRule[$literal->getPackageId()]; } diff --git a/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php index 6340a7f68..28db18131 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php @@ -27,7 +27,7 @@ class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase new Rule(array(), 'job1', null), new Rule(array(), 'job2', null), ), - RuleSet::TYPE_UPDATE => array( + RuleSet::TYPE_FEATURE => array( new Rule(array(), 'update1', null), ), RuleSet::TYPE_PACKAGE => array(), @@ -46,7 +46,7 @@ class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase $expected = array( $this->rules[RuleSet::TYPE_JOB][0], $this->rules[RuleSet::TYPE_JOB][1], - $this->rules[RuleSet::TYPE_UPDATE][0], + $this->rules[RuleSet::TYPE_FEATURE][0], ); $this->assertEquals($expected, $result); @@ -64,7 +64,7 @@ class ResultSetIteratorTest extends \PHPUnit_Framework_TestCase $expected = array( RuleSet::TYPE_JOB, RuleSet::TYPE_JOB, - RuleSet::TYPE_UPDATE, + RuleSet::TYPE_FEATURE, ); $this->assertEquals($expected, $result); diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php index 7a5b018b4..54ad88a58 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -27,10 +27,9 @@ class RuleSetTest extends TestCase new Rule(array(), 'job1', null), new Rule(array(), 'job2', null), ), - RuleSet::TYPE_UPDATE => array( + RuleSet::TYPE_FEATURE => array( new Rule(array(), 'update1', null), ), - RuleSet::TYPE_FEATURE => array(), RuleSet::TYPE_LEARNED => array(), RuleSet::TYPE_CHOICE => array(), ); @@ -38,7 +37,7 @@ class RuleSetTest extends TestCase $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_FEATURE][0], RuleSet::TYPE_FEATURE); $ruleSet->add($rules[RuleSet::TYPE_JOB][1], RuleSet::TYPE_JOB); $this->assertEquals($rules, $ruleSet->getRules()); @@ -81,7 +80,7 @@ class RuleSetTest extends TestCase $rule1 = new Rule(array(), 'job1', null); $rule2 = new Rule(array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); - $ruleSet->add($rule2, RuleSet::TYPE_UPDATE); + $ruleSet->add($rule2, RuleSet::TYPE_FEATURE); $iterator = $ruleSet->getIterator(); @@ -97,9 +96,9 @@ class RuleSetTest extends TestCase $rule2 = new Rule(array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); - $ruleSet->add($rule2, RuleSet::TYPE_UPDATE); + $ruleSet->add($rule2, RuleSet::TYPE_FEATURE); - $iterator = $ruleSet->getIteratorFor(RuleSet::TYPE_UPDATE); + $iterator = $ruleSet->getIteratorFor(RuleSet::TYPE_FEATURE); $this->assertSame($rule2, $iterator->current()); } @@ -111,7 +110,7 @@ class RuleSetTest extends TestCase $rule2 = new Rule(array(), 'job1', null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); - $ruleSet->add($rule2, RuleSet::TYPE_UPDATE); + $ruleSet->add($rule2, RuleSet::TYPE_FEATURE); $iterator = $ruleSet->getIteratorWithout(RuleSet::TYPE_JOB); @@ -143,7 +142,7 @@ class RuleSetTest extends TestCase ->method('equal') ->will($this->returnValue(false)); - $ruleSet->add($rule, RuleSet::TYPE_UPDATE); + $ruleSet->add($rule, RuleSet::TYPE_FEATURE); $this->assertTrue($ruleSet->containsEqual($rule)); $this->assertFalse($ruleSet->containsEqual($rule2)); @@ -156,9 +155,9 @@ class RuleSetTest extends TestCase $literal = new Literal($this->getPackage('foo', '2.1'), true); $rule = new Rule(array($literal), 'job1', null); - $ruleSet->add($rule, RuleSet::TYPE_UPDATE); + $ruleSet->add($rule, RuleSet::TYPE_FEATURE); - $this->assertContains('UPDATE : (+foo-2.1.0.0)', $ruleSet->__toString()); + $this->assertContains('FEATURE : (+foo-2.1.0.0)', $ruleSet->__toString()); } private function getRuleMock()