From e848c76cbc19adf1fc483c42037a287f63e43b86 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 23 May 2013 18:12:54 +0200 Subject: [PATCH] Only compare branches as versions in the policy to sort packages, but not in the solver, fixes #1817 --- .../DependencyResolver/DefaultPolicy.php | 2 +- .../LinkConstraint/VersionConstraint.php | 17 ++++++++++++----- .../DependencyResolver/DefaultPolicyTest.php | 14 ++++++++++++++ .../LinkConstraint/VersionConstraintTest.php | 17 +++++++++++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/Composer/DependencyResolver/DefaultPolicy.php b/src/Composer/DependencyResolver/DefaultPolicy.php index 4d2c25855..190829213 100644 --- a/src/Composer/DependencyResolver/DefaultPolicy.php +++ b/src/Composer/DependencyResolver/DefaultPolicy.php @@ -39,7 +39,7 @@ class DefaultPolicy implements PolicyInterface $constraint = new VersionConstraint($operator, $b->getVersion()); $version = new VersionConstraint('==', $a->getVersion()); - return $constraint->matchSpecific($version); + return $constraint->matchSpecific($version, true); } public function findUpdatePackages(Pool $pool, array $installedMap, PackageInterface $package) diff --git a/src/Composer/Package/LinkConstraint/VersionConstraint.php b/src/Composer/Package/LinkConstraint/VersionConstraint.php index 8eb69dfd3..1fc37bf85 100644 --- a/src/Composer/Package/LinkConstraint/VersionConstraint.php +++ b/src/Composer/Package/LinkConstraint/VersionConstraint.php @@ -44,12 +44,19 @@ class VersionConstraint extends SpecificConstraint $this->version = $version; } - public function versionCompare($a, $b, $operator) + public function versionCompare($a, $b, $operator, $compareBranches = false) { - if ('dev-' === substr($a, 0, 4) && 'dev-' === substr($b, 0, 4)) { + $aIsBranch = 'dev-' === substr($a, 0, 4); + $bIsBranch = 'dev-' === substr($b, 0, 4); + if ($aIsBranch && $bIsBranch) { return $operator == '==' && $a === $b; } + // when branches are not comparable, we make sure dev branches never match anything + if (!$compareBranches && ($aIsBranch || $bIsBranch)) { + return false; + } + return version_compare($a, $b, $operator); } @@ -57,7 +64,7 @@ class VersionConstraint extends SpecificConstraint * * @param VersionConstraint $provider */ - public function matchSpecific(VersionConstraint $provider) + public function matchSpecific(VersionConstraint $provider, $compareBranches = false) { $noEqualOp = str_replace('=', '', $this->operator); $providerNoEqualOp = str_replace('=', '', $provider->operator); @@ -71,7 +78,7 @@ class VersionConstraint extends SpecificConstraint // these kinds of comparisons always have a solution if ($isNonEqualOp || $isProviderNonEqualOp) { return !$isEqualOp && !$isProviderEqualOp - || $this->versionCompare($provider->version, $this->version, '!='); + || $this->versionCompare($provider->version, $this->version, '!=', $compareBranches); } // an example for the condition is <= 2.0 & < 1.0 @@ -80,7 +87,7 @@ class VersionConstraint extends SpecificConstraint return true; } - if ($this->versionCompare($provider->version, $this->version, $this->operator)) { + if ($this->versionCompare($provider->version, $this->version, $this->operator, $compareBranches)) { // special case, e.g. require >= 1.0 and provide < 1.0 // 1.0 >= 1.0 but 1.0 is outside of the provided interval if ($provider->version == $this->version && $provider->operator == $providerNoEqualOp && $this->operator != $noEqualOp) { diff --git a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php index af0c0f99e..4d50fa6c5 100644 --- a/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php +++ b/tests/Composer/Test/DependencyResolver/DefaultPolicyTest.php @@ -94,6 +94,20 @@ class DefaultPolicyTest extends TestCase $this->assertEquals($expected, $selected); } + public function testSelectNewestWithDevPicksNonDev() + { + $this->repo->addPackage($packageA1 = $this->getPackage('A', 'dev-foo')); + $this->repo->addPackage($packageA2 = $this->getPackage('A', '1.0.0')); + $this->pool->addRepository($this->repo); + + $literals = array($packageA1->getId(), $packageA2->getId()); + $expected = array($packageA2->getId()); + + $selected = $this->policy->selectPreferedPackages($this->pool, array(), $literals); + + $this->assertEquals($expected, $selected); + } + public function testSelectNewestOverInstalled() { $this->repo->addPackage($packageA = $this->getPackage('A', '2.0')); diff --git a/tests/Composer/Test/Package/LinkConstraint/VersionConstraintTest.php b/tests/Composer/Test/Package/LinkConstraint/VersionConstraintTest.php index eb6663822..e2adda282 100644 --- a/tests/Composer/Test/Package/LinkConstraint/VersionConstraintTest.php +++ b/tests/Composer/Test/Package/LinkConstraint/VersionConstraintTest.php @@ -72,6 +72,8 @@ class VersionConstraintTest extends \PHPUnit_Framework_TestCase array('==', 'dev-foo-bist', '==', 'dev-foo-aist'), array('<=', 'dev-foo-bist', '>=', 'dev-foo-aist'), array('>=', 'dev-foo-bist', '<', 'dev-foo-aist'), + array('<', '0.12', '==', 'dev-foo'), // branches are not comparable + array('>', '0.12', '==', 'dev-foo'), // branches are not comparable ); } @@ -85,4 +87,19 @@ class VersionConstraintTest extends \PHPUnit_Framework_TestCase $this->assertFalse($versionRequire->matches($versionProvide)); } + + public function testComparableBranches() + { + $versionRequire = new VersionConstraint('>', '0.12'); + $versionProvide = new VersionConstraint('==', 'dev-foo'); + + $this->assertFalse($versionRequire->matches($versionProvide)); + $this->assertFalse($versionRequire->matchSpecific($versionProvide, true)); + + $versionRequire = new VersionConstraint('<', '0.12'); + $versionProvide = new VersionConstraint('==', 'dev-foo'); + + $this->assertFalse($versionRequire->matches($versionProvide)); + $this->assertTrue($versionRequire->matchSpecific($versionProvide, true)); + } }