From c917865fe9f62fd0a7fa6db910597ec4259fc7bf Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 13 Apr 2018 13:48:30 +0200 Subject: [PATCH] Fix handling of dev versions and consolidate logic, refs #7119 --- src/Composer/Downloader/FileDownloader.php | 3 +- src/Composer/Downloader/VcsDownloader.php | 19 +--------- .../Package/Version/VersionParser.php | 11 ++++++ .../Test/Downloader/FileDownloaderTest.php | 22 +++++++---- .../Test/Downloader/FossilDownloaderTest.php | 3 ++ .../Test/Downloader/GitDownloaderTest.php | 38 +++++++++++-------- .../Test/Downloader/HgDownloaderTest.php | 3 ++ .../Package/Version/VersionParserTest.php | 22 +++++++++++ 8 files changed, 78 insertions(+), 43 deletions(-) diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index da4695f57..11d20eb99 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -19,6 +19,7 @@ use Composer\IO\IOInterface; use Composer\IO\NullIO; use Composer\Package\Comparer\Comparer; use Composer\Package\PackageInterface; +use Composer\Package\Version\VersionParser; use Composer\Plugin\PluginEvents; use Composer\Plugin\PreFileDownloadEvent; use Composer\EventDispatcher\EventDispatcher; @@ -218,7 +219,7 @@ class FileDownloader implements DownloaderInterface $from = $initial->getPrettyVersion(); $to = $target->getPrettyVersion(); - $actionName = version_compare($from, $to, '<') ? 'Updating' : 'Downgrading'; + $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Updating' : 'Downgrading'; $this->io->writeError(" - " . $actionName . " " . $name . " (" . $from . " => " . $to . "): ", false); $this->remove($initial, $path, false); diff --git a/src/Composer/Downloader/VcsDownloader.php b/src/Composer/Downloader/VcsDownloader.php index a0ccacdf1..aa666058e 100644 --- a/src/Composer/Downloader/VcsDownloader.php +++ b/src/Composer/Downloader/VcsDownloader.php @@ -130,7 +130,7 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa $to = $target->getFullPrettyVersion(); } - $actionName = $this->packageCompare($initial, $target, '>') ? 'Downgrading' : 'Updating'; + $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Updating' : 'Downgrading'; $this->io->writeError(" - " . $actionName . " " . $name . " (" . $from . " => " . $to . "): ", false); $this->cleanChanges($initial, $path, true); @@ -243,23 +243,6 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa } } - /** - * Compare two packages. Always false if both versions are references - * - * @param PackageInterface $initial - * @param PackageInterface $target - * @param string $operation - * @return bool - */ - protected function packageCompare(PackageInterface $initial, PackageInterface $target, $operation = '<') - { - if ($initial->getPrettyVersion() == $target->getPrettyVersion()) { - return false; - } - - return version_compare($initial->getFullPrettyVersion(), $target->getFullPrettyVersion(), $operation); - } - /** * Guarantee that no changes have been made to the local copy * diff --git a/src/Composer/Package/Version/VersionParser.php b/src/Composer/Package/Version/VersionParser.php index 22b6dad50..636ccd4da 100644 --- a/src/Composer/Package/Version/VersionParser.php +++ b/src/Composer/Package/Version/VersionParser.php @@ -14,6 +14,7 @@ namespace Composer\Package\Version; use Composer\Repository\PlatformRepository; use Composer\Semver\VersionParser as SemverVersionParser; +use Composer\Semver\Semver; class VersionParser extends SemverVersionParser { @@ -63,4 +64,14 @@ class VersionParser extends SemverVersionParser return $result; } + + /** + * @return bool + */ + public static function isUpgrade($normalizedFrom, $normalizedTo) + { + $sorted = Semver::sort(array($normalizedTo, $normalizedFrom)); + + return $sorted[0] === $normalizedFrom; + } } diff --git a/tests/Composer/Test/Downloader/FileDownloaderTest.php b/tests/Composer/Test/Downloader/FileDownloaderTest.php index fab0c43d1..d186e982e 100644 --- a/tests/Composer/Test/Downloader/FileDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FileDownloaderTest.php @@ -211,18 +211,24 @@ class FileDownloaderTest extends TestCase $oldPackage = $this->getMock('Composer\Package\PackageInterface'); $oldPackage->expects($this->once()) ->method('getPrettyVersion') - ->will($this->returnValue('1.0.0')); - $oldPackage->expects($this->any()) - ->method('getDistUrl') - ->will($this->returnValue($distUrl = 'http://example.com/script.js')); + ->will($this->returnValue('1.2.0')); $oldPackage->expects($this->once()) - ->method('getDistUrls') - ->will($this->returnValue(array($distUrl))); + ->method('getVersion') + ->will($this->returnValue('1.2.0.0')); $newPackage = $this->getMock('Composer\Package\PackageInterface'); $newPackage->expects($this->once()) ->method('getPrettyVersion') - ->will($this->returnValue('1.2.0')); + ->will($this->returnValue('1.0.0')); + $newPackage->expects($this->once()) + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); + $newPackage->expects($this->any()) + ->method('getDistUrl') + ->will($this->returnValue($distUrl = 'http://example.com/script.js')); + $newPackage->expects($this->once()) + ->method('getDistUrls') + ->will($this->returnValue(array($distUrl))); $ioMock = $this->getMock('Composer\IO\IOInterface'); $ioMock->expects(($this->at(0))) @@ -237,6 +243,6 @@ class FileDownloaderTest extends TestCase ->will($this->returnValue(true)); $downloader = $this->getDownloader($ioMock, null, null, null, null, $filesystem); - $downloader->update($newPackage, $oldPackage, $path); + $downloader->update($oldPackage, $newPackage, $path); } } diff --git a/tests/Composer/Test/Downloader/FossilDownloaderTest.php b/tests/Composer/Test/Downloader/FossilDownloaderTest.php index 961686bef..ca941fe20 100644 --- a/tests/Composer/Test/Downloader/FossilDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FossilDownloaderTest.php @@ -123,6 +123,9 @@ class FossilDownloaderTest extends TestCase $packageMock->expects($this->any()) ->method('getSourceUrls') ->will($this->returnValue(array('http://fossil.kd2.org/kd2fw/'))); + $packageMock->expects($this->any()) + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $expectedFossilCommand = $this->getCmd("fossil changes"); diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 2400fdb56..f487a5e28 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -390,8 +390,8 @@ class GitDownloaderTest extends TestCase ->method('getSourceUrls') ->will($this->returnValue(array('https://github.com/composer/composer'))); $packageMock->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('1.0.0')); + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) ->method('execute') @@ -442,8 +442,8 @@ class GitDownloaderTest extends TestCase ->method('getSourceUrl') ->will($this->returnValue('https://github.com/composer/composer')); $packageMock->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('1.0.0')); + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) ->method('execute') @@ -510,6 +510,9 @@ composer https://github.com/old/url (push) $packageMock->expects($this->any()) ->method('getSourceUrls') ->will($this->returnValue(array('https://github.com/composer/composer'))); + $packageMock->expects($this->any()) + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) ->method('execute') @@ -546,6 +549,9 @@ composer https://github.com/old/url (push) $packageMock->expects($this->any()) ->method('getSourceReference') ->will($this->returnValue('ref')); + $packageMock->expects($this->any()) + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $packageMock->expects($this->any()) ->method('getSourceUrls') ->will($this->returnValue(array('/foo/bar', 'https://github.com/composer/composer'))); @@ -600,11 +606,11 @@ composer https://github.com/old/url (push) { $oldPackage = $this->getMock('Composer\Package\PackageInterface'); $oldPackage->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('1.0.0')); + ->method('getVersion') + ->will($this->returnValue('1.2.0.0')); $oldPackage->expects($this->any()) ->method('getFullPrettyVersion') - ->will($this->returnValue('1.0.0')); + ->will($this->returnValue('1.2.0')); $oldPackage->expects($this->any()) ->method('getSourceReference') ->will($this->returnValue('ref')); @@ -620,11 +626,11 @@ composer https://github.com/old/url (push) ->method('getSourceUrls') ->will($this->returnValue(array('https://github.com/composer/composer'))); $newPackage->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('1.2.0')); + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $newPackage->expects($this->any()) ->method('getFullPrettyVersion') - ->will($this->returnValue('1.2.0')); + ->will($this->returnValue('1.0.0')); $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->any()) @@ -638,15 +644,15 @@ composer https://github.com/old/url (push) $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor); - $downloader->update($newPackage, $oldPackage, $this->workingDir); + $downloader->update($oldPackage, $newPackage, $this->workingDir); } public function testNotUsingDowngradingWithReferences() { $oldPackage = $this->getMock('Composer\Package\PackageInterface'); $oldPackage->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('ref')); + ->method('getVersion') + ->will($this->returnValue('dev-ref')); $oldPackage->expects($this->any()) ->method('getSourceReference') ->will($this->returnValue('ref')); @@ -662,8 +668,8 @@ composer https://github.com/old/url (push) ->method('getSourceUrls') ->will($this->returnValue(array('https://github.com/composer/composer'))); $newPackage->expects($this->any()) - ->method('getPrettyVersion') - ->will($this->returnValue('ref')); + ->method('getVersion') + ->will($this->returnValue('dev-ref2')); $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->any()) @@ -677,7 +683,7 @@ composer https://github.com/old/url (push) $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor); - $downloader->update($newPackage, $oldPackage, $this->workingDir); + $downloader->update($oldPackage, $newPackage, $this->workingDir); } public function testRemove() diff --git a/tests/Composer/Test/Downloader/HgDownloaderTest.php b/tests/Composer/Test/Downloader/HgDownloaderTest.php index 537d98775..714388f2c 100644 --- a/tests/Composer/Test/Downloader/HgDownloaderTest.php +++ b/tests/Composer/Test/Downloader/HgDownloaderTest.php @@ -109,6 +109,9 @@ class HgDownloaderTest extends TestCase $packageMock->expects($this->any()) ->method('getSourceReference') ->will($this->returnValue('ref')); + $packageMock->expects($this->any()) + ->method('getVersion') + ->will($this->returnValue('1.0.0.0')); $packageMock->expects($this->any()) ->method('getSourceUrls') ->will($this->returnValue(array('https://github.com/l3l0/composer'))); diff --git a/tests/Composer/Test/Package/Version/VersionParserTest.php b/tests/Composer/Test/Package/Version/VersionParserTest.php index 93728fd61..80dceb822 100644 --- a/tests/Composer/Test/Package/Version/VersionParserTest.php +++ b/tests/Composer/Test/Package/Version/VersionParserTest.php @@ -35,4 +35,26 @@ class VersionParserTest extends TestCase array(array('php', 'ext-apcu'), array(array('name' => 'php'), array('name' => 'ext-apcu'))), ); } + + /** + * @dataProvider getIsUpgradeTests + */ + public function testIsUpgrade($from, $to, $expected) + { + $this->assertSame($expected, VersionParser::isUpgrade($from, $to)); + } + + public function getIsUpgradeTests() + { + return array( + array('0.9.0.0', '1.0.0.0', true), + array('1.0.0.0', '0.9.0.0', false), + array('1.0.0.0', '9999999-dev', true), + array('9999999-dev', '9999999-dev', true), + array('9999999-dev', '1.0.0.0', false), + array('1.0.0.0', 'dev-foo', true), + array('dev-foo', 'dev-foo', true), + array('dev-foo', '1.0.0.0', true), + ); + } }