From ee60df708d0c02e83bb62fec9077d7d45d4eaeaa Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sun, 10 Mar 2013 19:55:26 +0100 Subject: [PATCH] Handle stability changes correctly, fixes #877 On update, packages that are less stable than the minimum-stability allows will now be downgraded to their correct versions, even if they were installed as unstable already. --- src/Composer/Installer.php | 93 +++++++++++-------- ...e-downgrades-non-whitelisted-unstable.test | 65 +++++++++++++ .../installer/partial-update-from-lock.test | 66 +++++++++++++ .../partial-update-without-lock.test | 49 ++++++++++ .../update-downgrades-unstable-packages.test | 49 ++++++++++ .../Test/Mock/InstallationManagerMock.php | 5 + 6 files changed, 286 insertions(+), 41 deletions(-) create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-from-lock.test create mode 100644 tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test create mode 100644 tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 992a7c69f..897e67d56 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -325,6 +325,20 @@ class Installer // creating requirements request $request = $this->createRequest($pool, $this->package, $platformRepo); + if (!$installFromLock) { + // remove unstable packages from the localRepo if they don't match the current stability settings + $removedUnstablePackages = array(); + foreach ($localRepo->getPackages() as $package) { + if ( + !$pool->isPackageAcceptable($package->getName(), $package->getStability()) + && $this->installationManager->isPackageInstalled($localRepo, $package) + ) { + $removedUnstablePackages[$package->getName()] = true; + $request->remove($package->getName(), new VersionConstraint('=', $package->getVersion())); + } + } + } + if ($this->update) { $this->io->write('Updating dependencies'.($withDevReqs?' (including require-dev)':'').''); @@ -339,6 +353,43 @@ class Installer foreach ($links as $link) { $request->install($link->getTarget(), $link->getConstraint()); } + + // if the updateWhitelist is enabled, packages not in it are also fixed + // to the version specified in the lock, or their currently installed version + if ($this->updateWhitelist) { + if ($this->locker->isLocked()) { + try { + $currentPackages = $this->locker->getLockedRepository($withDevReqs)->getPackages(); + } catch (\RuntimeException $e) { + // fetch only non-dev packages from lock if doing a dev update fails due to a previously incomplete lock file + $currentPackages = $this->locker->getLockedRepository()->getPackages(); + } + } else { + $currentPackages = $installedRepo->getPackages(); + } + + // collect packages to fixate from root requirements as well as installed packages + $candidates = array(); + foreach ($links as $link) { + $candidates[$link->getTarget()] = true; + } + foreach ($localRepo->getPackages() as $package) { + $candidates[$package->getName()] = true; + } + + // fix them to the version in lock (or currently installed) if they are not updateable + foreach ($candidates as $candidate => $dummy) { + foreach ($currentPackages as $curPackage) { + if ($curPackage->getName() === $candidate) { + if (!$this->isUpdateable($curPackage) && !isset($removedUnstablePackages[$curPackage->getName()])) { + $constraint = new VersionConstraint('=', $curPackage->getVersion()); + $request->install($curPackage->getName(), $constraint); + } + break; + } + } + } + } } elseif ($installFromLock) { $this->io->write('Installing dependencies'.($withDevReqs?' (including require-dev)':'').' from lock file'); @@ -377,44 +428,6 @@ class Installer } } - // if the updateWhitelist is enabled, packages not in it are also fixed - // to the version specified in the lock, or their currently installed version - if ($this->update && $this->updateWhitelist) { - if ($this->locker->isLocked()) { - try { - $currentPackages = $this->locker->getLockedRepository($withDevReqs)->getPackages(); - } catch (\RuntimeException $e) { - // fetch only non-dev packages from lock if doing a dev update fails due to a previously incomplete lock file - $currentPackages = $this->locker->getLockedRepository()->getPackages(); - } - } else { - $currentPackages = $installedRepo->getPackages(); - } - - // collect links from composer as well as installed packages - $candidates = array(); - foreach ($links as $link) { - $candidates[$link->getTarget()] = true; - } - foreach ($localRepo->getPackages() as $package) { - $candidates[$package->getName()] = true; - } - - // fix them to the version in lock (or currently installed) if they are not updateable - foreach ($candidates as $candidate => $dummy) { - foreach ($currentPackages as $curPackage) { - if ($curPackage->getName() === $candidate) { - if ($this->isUpdateable($curPackage)) { - break; - } - - $constraint = new VersionConstraint('=', $curPackage->getVersion()); - $request->install($curPackage->getName(), $constraint); - } - } - } - } - // force dev packages to have the latest links if we update or install from a (potentially new) lock $this->processDevPackages($localRepo, $pool, $policy, $repositories, $lockedRepository, $installFromLock, 'force-links'); @@ -512,9 +525,7 @@ class Installer $constraint->setPrettyString($rootPackage->getPrettyVersion()); $request->install($rootPackage->getName(), $constraint); - // fix the version of all installed packages (+ platform) that are not - // in the current local repo to prevent rogue updates (e.g. non-dev - // updating when in dev) + // fix the version of all platform packages to prevent the solver trying to remove those foreach ($platformRepo->getPackages() as $package) { $constraint = new VersionConstraint('=', $package->getVersion()); $constraint->setPrettyString($package->getPrettyVersion()); diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test b/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test new file mode 100644 index 000000000..fb618ebe3 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-downgrades-non-whitelisted-unstable.test @@ -0,0 +1,65 @@ +--TEST-- +Partial update from lock file should apply lock file and downgrade unstable packages even if not whitelisted +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/old", "version": "1.0.0" }, + { "name": "a/old", "version": "2.0.0" }, + { "name": "b/unstable", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } + ] + } + ], + "require": { + "a/old": "*", + "b/unstable": "*", + "c/uptodate": "*" + } +} +--LOCK-- +{ + "packages": [ + { "name": "a/old", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "b/unstable": 15 + }, + "platform": [], + "platform-dev": [] +} +--INSTALLED-- +[ + { "name": "a/old", "version": "0.9.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "2.0.0" } +] +--RUN-- +update c/uptodate +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "a/old", "version": "1.0.0", "type": "library" }, + { "name": "b/unstable", "version": "1.0.0", "type": "library" }, + { "name": "c/uptodate", "version": "2.0.0", "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Updating a/old (0.9.0) to a/old (1.0.0) +Updating b/unstable (1.1.0-alpha) to b/unstable (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-from-lock.test b/tests/Composer/Test/Fixtures/installer/partial-update-from-lock.test new file mode 100644 index 000000000..51368b861 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-from-lock.test @@ -0,0 +1,66 @@ +--TEST-- +Partial update from lock file should update everything to the state of the lock, remove overly unstable packages +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/old", "version": "1.0.0" }, + { "name": "a/old", "version": "2.0.0" }, + { "name": "b/unstable", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } + ] + } + ], + "require": { + "a/old": "*", + "b/unstable": "*", + "c/uptodate": "*" + } +} +--LOCK-- +{ + "packages": [ + { "name": "a/old", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": { + "b/unstable": 15 + }, + "platform": [], + "platform-dev": [] +} +--INSTALLED-- +[ + { "name": "a/old", "version": "0.9.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "2.0.0" } +] +--RUN-- +update b/unstable +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "a/old", "version": "1.0.0", "type": "library" }, + { "name": "b/unstable", "version": "1.0.0", "type": "library" }, + { "name": "c/uptodate", "version": "1.0.0", "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Updating a/old (0.9.0) to a/old (1.0.0) +Updating c/uptodate (2.0.0) to c/uptodate (1.0.0) +Updating b/unstable (1.1.0-alpha) to b/unstable (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test b/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test new file mode 100644 index 000000000..146277d02 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/partial-update-without-lock.test @@ -0,0 +1,49 @@ +--TEST-- +Partial update without lock file should update everything whitelisted, remove overly unstable packages +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/old", "version": "1.0.0" }, + { "name": "a/old", "version": "2.0.0" }, + { "name": "b/unstable", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } + ] + } + ], + "require": { + "a/old": "*", + "b/unstable": "*", + "c/uptodate": "*" + } +} +--INSTALLED-- +[ + { "name": "a/old", "version": "1.0.0" }, + { "name": "b/unstable", "version": "1.1.0-alpha" }, + { "name": "c/uptodate", "version": "1.0.0" }, + { "name": "d/removed", "version": "1.0.0" } +] +--RUN-- +update b/unstable +--EXPECT-LOCK-- +{ + "packages": [ + { "name": "a/old", "version": "1.0.0", "type": "library" }, + { "name": "b/unstable", "version": "1.0.0", "type": "library" }, + { "name": "c/uptodate", "version": "1.0.0", "type": "library" }, + { "name": "d/removed", "version": "1.0.0", "type": "library" } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "stable", + "stability-flags": [], + "platform": [], + "platform-dev": [] +} +--EXPECT-- +Updating b/unstable (1.1.0-alpha) to b/unstable (1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test b/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test new file mode 100644 index 000000000..1b6e55ef9 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test @@ -0,0 +1,49 @@ +--TEST-- +Downgrading from unstable to more stable package should work even if already installed +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "a/a", "version": "dev-master", + "source": { "reference": "abcd", "url": "", "type": "git" } + }, + { + "name": "a/a", "version": "1.0.0", + "source": { "reference": "1.0.0", "url": "", "type": "git" }, + "dist": { "reference": "1.0.0", "url": "", "type": "zip", "shasum": "" } + }, + { + "name": "b/b", "version": "dev-master", + "source": { "reference": "abcd", "url": "", "type": "git" } + }, + { + "name": "b/b", "version": "1.0.0", + "source": { "reference": "1.0.0", "url": "", "type": "git" }, + "dist": { "reference": "1.0.0", "url": "", "type": "zip", "shasum": "" } + } + ] + } + ], + "require": { + "a/a": "*", + "b/b": "*@dev" + } +} +--INSTALLED-- +[ + { + "name": "a/a", "version": "dev-master", + "source": { "reference": "abcd", "url": "", "type": "git" } + }, + { + "name": "b/b", "version": "dev-master", + "source": { "reference": "abcd", "url": "", "type": "git" } + } +] +--RUN-- +update +--EXPECT-- +Updating a/a (dev-master abcd) to a/a (1.0.0) diff --git a/tests/Composer/Test/Mock/InstallationManagerMock.php b/tests/Composer/Test/Mock/InstallationManagerMock.php index 0ecca1e2a..985b85879 100644 --- a/tests/Composer/Test/Mock/InstallationManagerMock.php +++ b/tests/Composer/Test/Mock/InstallationManagerMock.php @@ -33,6 +33,11 @@ class InstallationManagerMock extends InstallationManager return ''; } + public function isPackageInstalled(InstalledRepositoryInterface $repo, PackageInterface $package) + { + return $repo->hasPackage($package); + } + public function install(RepositoryInterface $repo, InstallOperation $operation) { $this->installed[] = $operation->getPackage();