From 53d2ab2253c5f73579ea4b845bfd99f28c15a6cf Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 29 Aug 2019 11:37:23 +0200 Subject: [PATCH] Split up steps on VCS downloaders to allow doing network operations before touching the filesystem on GitDownloader, fixes #7903 --- src/Composer/Downloader/DownloadManager.php | 72 ++++-- .../Downloader/DownloaderInterface.php | 37 ++- src/Composer/Downloader/FileDownloader.php | 18 +- src/Composer/Downloader/FossilDownloader.php | 12 +- src/Composer/Downloader/GitDownloader.php | 103 +++++--- src/Composer/Downloader/HgDownloader.php | 12 +- src/Composer/Downloader/PathDownloader.php | 2 +- .../Downloader/PerforceDownloader.php | 10 +- src/Composer/Downloader/SvnDownloader.php | 12 +- src/Composer/Downloader/VcsDownloader.php | 132 +++++++--- src/Composer/Downloader/ZipDownloader.php | 4 +- .../Installer/InstallationManager.php | 58 ++++- src/Composer/Installer/InstallerInterface.php | 50 +++- src/Composer/Installer/LibraryInstaller.php | 25 ++ .../Installer/MetapackageInstaller.php | 16 ++ src/Composer/Installer/NoopInstaller.php | 14 ++ src/Composer/Installer/ProjectInstaller.php | 16 ++ src/Composer/Util/Git.php | 8 +- .../Test/Downloader/FossilDownloaderTest.php | 8 +- .../Test/Downloader/GitDownloaderTest.php | 235 +++++++++--------- .../Test/Downloader/HgDownloaderTest.php | 6 + .../Test/Downloader/ZipDownloaderTest.php | 2 +- 22 files changed, 618 insertions(+), 234 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index a23c167b5..794998bb2 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -165,9 +165,9 @@ class DownloadManager /** * Downloads package into target dir. * - * @param PackageInterface $package package instance - * @param string $targetDir target dir - * @param PackageInterface $prevPackage previous package instance in case of updates + * @param PackageInterface $package package instance + * @param string $targetDir target dir + * @param PackageInterface|null $prevPackage previous package instance in case of updates * * @return PromiseInterface * @throws \InvalidArgumentException if package have no urls to download from @@ -182,7 +182,7 @@ class DownloadManager $io = $this->io; $self = $this; - $download = function ($retry = false) use (&$sources, $io, $package, $self, $targetDir, &$download) { + $download = function ($retry = false) use (&$sources, $io, $package, $self, $targetDir, &$download, $prevPackage) { $source = array_shift($sources); if ($retry) { $io->writeError(' Now trying to download from ' . $source . ''); @@ -214,7 +214,7 @@ class DownloadManager }; try { - $result = $downloader->download($package, $targetDir); + $result = $downloader->download($package, $targetDir, $prevPackage); } catch (\Exception $e) { return $handleError($e); } @@ -232,12 +232,31 @@ class DownloadManager return $download(); } + /** + * Prepares an operation execution + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param string $targetDir target dir + * @param PackageInterface|null $prevPackage previous package instance in case of updates + * + * @return PromiseInterface|null + */ + public function prepare($type, PackageInterface $package, $targetDir, PackageInterface $prevPackage = null) + { + $downloader = $this->getDownloaderForPackage($package); + if ($downloader) { + return $downloader->prepare($type, $package, $targetDir, $prevPackage); + } + } + /** * Installs package into target dir. * * @param PackageInterface $package package instance * @param string $targetDir target dir * + * @return PromiseInterface|null * @throws \InvalidArgumentException if package have no urls to download from * @throws \RuntimeException */ @@ -245,7 +264,7 @@ class DownloadManager { $downloader = $this->getDownloaderForPackage($package); if ($downloader) { - $downloader->install($package, $targetDir); + return $downloader->install($package, $targetDir); } } @@ -256,6 +275,7 @@ class DownloadManager * @param PackageInterface $target target package version * @param string $targetDir target dir * + * @return PromiseInterface|null * @throws \InvalidArgumentException if initial package is not installed */ public function update(PackageInterface $initial, PackageInterface $target, $targetDir) @@ -270,17 +290,14 @@ class DownloadManager // if we have a downloader present before, but not after, the package became a metapackage and its files should be removed if (!$downloader) { - $initialDownloader->remove($initial, $targetDir); - return; + return $initialDownloader->remove($initial, $targetDir); } $initialType = $this->getDownloaderType($initialDownloader); $targetType = $this->getDownloaderType($downloader); if ($initialType === $targetType) { try { - $downloader->update($initial, $target, $targetDir); - - return; + return $downloader->update($initial, $target, $targetDir); } catch (\RuntimeException $e) { if (!$this->io->isInteractive()) { throw $e; @@ -294,8 +311,15 @@ class DownloadManager // if downloader type changed, or update failed and user asks for reinstall, // we wipe the dir and do a new install instead of updating it - $initialDownloader->remove($initial, $targetDir); - $this->install($target, $targetDir); + $promise = $initialDownloader->remove($initial, $targetDir); + if ($promise) { + $self = $this; + return $promise->then(function ($res) use ($self, $target, $targetDir) { + return $self->install($target, $targetDir); + }); + } + + return $this->install($target, $targetDir); } /** @@ -303,12 +327,32 @@ class DownloadManager * * @param PackageInterface $package package instance * @param string $targetDir target dir + * + * @return PromiseInterface|null */ public function remove(PackageInterface $package, $targetDir) { $downloader = $this->getDownloaderForPackage($package); if ($downloader) { - $downloader->remove($package, $targetDir); + return $downloader->remove($package, $targetDir); + } + } + + /** + * Cleans up a failed operation + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param string $targetDir target dir + * @param PackageInterface|null $prevPackage previous package instance in case of updates + * + * @return PromiseInterface|null + */ + public function cleanup($type, PackageInterface $package, $targetDir, PackageInterface $prevPackage = null) + { + $downloader = $this->getDownloaderForPackage($package); + if ($downloader) { + return $downloader->cleanup($type, $package, $targetDir, $prevPackage); } } diff --git a/src/Composer/Downloader/DownloaderInterface.php b/src/Composer/Downloader/DownloaderInterface.php index 2074b16da..01e7f95c8 100644 --- a/src/Composer/Downloader/DownloaderInterface.php +++ b/src/Composer/Downloader/DownloaderInterface.php @@ -31,14 +31,30 @@ interface DownloaderInterface public function getInstallationSource(); /** - * This should do any network-related tasks to prepare for install/update + * This should do any network-related tasks to prepare for an upcoming install/update * * @return PromiseInterface|null */ - public function download(PackageInterface $package, $path); + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null); /** - * Downloads specific package into specific folder. + * Do anything that needs to be done between all downloads have been completed and the actual operation is executed + * + * All packages get first downloaded, then all together prepared, then all together installed/updated/uninstalled. Therefore + * for error recovery it is important to avoid failing during install/update/uninstall as much as possible, and risky things or + * user prompts should happen in the prepare step rather. In case of failure, cleanup() will be called so that changes can + * be undone as much as possible. + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param string $path download path + * @param PackageInterface $prevPackage previous package instance in case of an update + * @return PromiseInterface|null + */ + public function prepare($type, PackageInterface $package, $path, PackageInterface $prevPackage = null); + + /** + * Installs specific package into specific folder. * * @param PackageInterface $package package instance * @param string $path download path @@ -61,4 +77,19 @@ interface DownloaderInterface * @param string $path download path */ public function remove(PackageInterface $package, $path); + + /** + * Do anything to cleanup changes applied in the prepare or install/update/uninstall steps + * + * Note that cleanup will be called for all packages regardless if they failed an operation or not, to give + * all installers a change to cleanup things they did previously, so you need to keep track of changes + * applied in the installer/downloader themselves. + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param string $path download path + * @param PackageInterface $prevPackage previous package instance in case of an update + * @return PromiseInterface|null + */ + public function cleanup($type, PackageInterface $package, $path, PackageInterface $prevPackage = null); } diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index ab72bbaed..20d21804d 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -84,7 +84,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface /** * {@inheritDoc} */ - public function download(PackageInterface $package, $path, $output = true) + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null, $output = true) { if (!$package->getDistUrl()) { throw new \InvalidArgumentException('The given package is missing url information'); @@ -222,6 +222,20 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface return $download(); } + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, $path, PackageInterface $prevPackage = null) + { + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, $path, PackageInterface $prevPackage = null) + { + } + /** * {@inheritDoc} */ @@ -336,7 +350,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $e = null; try { - $res = $this->download($package, $targetDir.'_compare', false); + $res = $this->download($package, $targetDir.'_compare', null, false); $this->httpDownloader->wait(); $res = $this->install($package, $targetDir.'_compare', false); diff --git a/src/Composer/Downloader/FossilDownloader.php b/src/Composer/Downloader/FossilDownloader.php index a814f89b7..be7c987b3 100644 --- a/src/Composer/Downloader/FossilDownloader.php +++ b/src/Composer/Downloader/FossilDownloader.php @@ -23,7 +23,15 @@ class FossilDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doInstall(PackageInterface $package, $path, $url) + protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null) + { + + } + + /** + * {@inheritDoc} + */ + protected function doInstall(PackageInterface $package, $path, $url) { // Ensure we are allowed to use this URL by config $this->config->prohibitUrlByConfig($url, $this->io); @@ -49,7 +57,7 @@ class FossilDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) + protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { // Ensure we are allowed to use this URL by config $this->config->prohibitUrlByConfig($url, $this->io); diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php index f698981fe..300749ca8 100644 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -29,6 +29,7 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface private $hasStashedChanges = false; private $hasDiscardedChanges = false; private $gitUtil; + private $cachedPackages = array(); public function __construct(IOInterface $io, Config $config, ProcessExecutor $process = null, Filesystem $fs = null) { @@ -39,34 +40,49 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface /** * {@inheritDoc} */ - public function doInstall(PackageInterface $package, $path, $url) + protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null) { GitUtil::cleanEnv(); - $path = $this->normalizePath($path); - $cachePath = $this->config->get('cache-vcs-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $url).'/'; - $ref = $package->getSourceReference(); - $flag = Platform::isWindows() ? '/D ' : ''; - // --dissociate option is only available since git 2.3.0-rc0 + $cachePath = $this->config->get('cache-vcs-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $url).'/'; $gitVersion = $this->gitUtil->getVersion(); - $msg = "Cloning ".$this->getShortHash($ref); - $command = 'git clone --no-checkout %url% %path% && cd '.$flag.'%path% && git remote add composer %url% && git fetch composer'; + // --dissociate option is only available since git 2.3.0-rc0 if ($gitVersion && version_compare($gitVersion, '2.3.0-rc0', '>=') && Cache::isUsable($cachePath)) { - $this->io->writeError('', true, IOInterface::DEBUG); + $this->io->writeError(" - Syncing " . $package->getName() . " (" . $package->getFullPrettyVersion() . ") into cache"); $this->io->writeError(sprintf(' Cloning to cache at %s', ProcessExecutor::escape($cachePath)), true, IOInterface::DEBUG); - try { - $this->gitUtil->fetchRefOrSyncMirror($url, $cachePath, $ref); - if (is_dir($cachePath)) { - $command = - 'git clone --no-checkout %cachePath% %path% --dissociate --reference %cachePath% ' - . '&& cd '.$flag.'%path% ' - . '&& git remote set-url origin %url% && git remote add composer %url%'; - $msg = "Cloning ".$this->getShortHash($ref).' from cache'; - } - } catch (\RuntimeException $e) { + $ref = $package->getSourceReference(); + if ($this->gitUtil->fetchRefOrSyncMirror($url, $cachePath, $ref) && is_dir($cachePath)) { + $this->cachedPackages[$package->getId()][$ref] = true; } } + } + + /** + * {@inheritDoc} + */ + protected function doInstall(PackageInterface $package, $path, $url) + { + GitUtil::cleanEnv(); + $path = $this->normalizePath($path); + $cachePath = $this->config->get('cache-vcs-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $url).'/'; + $ref = $package->getSourceReference(); + $flag = Platform::isWindows() ? '/D ' : ''; + + if (!empty($this->cachedPackages[$package->getId()][$ref])) { + $msg = "Cloning ".$this->getShortHash($ref).' from cache'; + $command = + 'git clone --no-checkout %cachePath% %path% --dissociate --reference %cachePath% ' + . '&& cd '.$flag.'%path% ' + . '&& git remote set-url origin %url% && git remote add composer %url%'; + } else { + $msg = "Cloning ".$this->getShortHash($ref); + $command = 'git clone --no-checkout %url% %path% && cd '.$flag.'%path% && git remote add composer %url% && git fetch composer'; + if (getenv('COMPOSER_DISABLE_NETWORK')) { + throw new \RuntimeException('The required git reference for '.$package->getName().' is not in cache and network is disabled, aborting'); + } + } + $this->io->writeError($msg); $commandCallable = function ($url) use ($path, $command, $cachePath) { @@ -99,30 +115,41 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface /** * {@inheritDoc} */ - public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) + protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { GitUtil::cleanEnv(); + $path = $this->normalizePath($path); if (!$this->hasMetadataRepository($path)) { throw new \RuntimeException('The .git directory is missing from '.$path.', see https://getcomposer.org/commit-deps for more information'); } - $updateOriginUrl = false; - if ( - 0 === $this->process->execute('git remote -v', $output, $path) - && preg_match('{^origin\s+(?P\S+)}m', $output, $originMatch) - && preg_match('{^composer\s+(?P\S+)}m', $output, $composerMatch) - ) { - if ($originMatch['url'] === $composerMatch['url'] && $composerMatch['url'] !== $target->getSourceUrl()) { - $updateOriginUrl = true; + $cachePath = $this->config->get('cache-vcs-dir').'/'.preg_replace('{[^a-z0-9.]}i', '-', $url).'/'; + $ref = $target->getSourceReference(); + $flag = Platform::isWindows() ? '/D ' : ''; + + if (!empty($this->cachedPackages[$target->getId()][$ref])) { + $msg = "Checking out ".$this->getShortHash($ref).' from cache'; + $command = 'git rev-parse --quiet --verify %ref% || (git remote set-url composer %cachePath% && git fetch composer && git fetch --tags composer); git remote set-url composer %url%'; + } else { + $msg = "Checking out ".$this->getShortHash($ref); + $command = 'git remote set-url composer %url% && git rev-parse --quiet --verify %ref% || (git fetch composer && git fetch --tags composer)'; + if (getenv('COMPOSER_DISABLE_NETWORK')) { + throw new \RuntimeException('The required git reference for '.$package->getName().' is not in cache and network is disabled, aborting'); } } - $ref = $target->getSourceReference(); - $this->io->writeError(" Checking out ".$this->getShortHash($ref)); - $command = 'git remote set-url composer %s && git rev-parse --quiet --verify %s || (git fetch composer && git fetch --tags composer)'; + $this->io->writeError($msg); - $commandCallable = function ($url) use ($command, $ref) { - return sprintf($command, ProcessExecutor::escape($url), ProcessExecutor::escape($ref.'^{commit}')); + $commandCallable = function ($url) use ($ref, $command, $cachePath) { + return str_replace( + array('%url%', '%ref%', '%cachePath%'), + array( + ProcessExecutor::escape($url), + ProcessExecutor::escape($ref.'^{commit}'), + ProcessExecutor::escape($cachePath), + ), + $command + ); }; $this->gitUtil->runCommand($commandCallable, $url, $path); @@ -133,6 +160,16 @@ class GitDownloader extends VcsDownloader implements DvcsDownloaderInterface $target->setSourceReference($newRef); } + $updateOriginUrl = false; + if ( + 0 === $this->process->execute('git remote -v', $output, $path) + && preg_match('{^origin\s+(?P\S+)}m', $output, $originMatch) + && preg_match('{^composer\s+(?P\S+)}m', $output, $composerMatch) + ) { + if ($originMatch['url'] === $composerMatch['url'] && $composerMatch['url'] !== $target->getSourceUrl()) { + $updateOriginUrl = true; + } + } if ($updateOriginUrl) { $this->updateOriginUrl($path, $target->getSourceUrl()); } diff --git a/src/Composer/Downloader/HgDownloader.php b/src/Composer/Downloader/HgDownloader.php index add381a75..91144a13d 100644 --- a/src/Composer/Downloader/HgDownloader.php +++ b/src/Composer/Downloader/HgDownloader.php @@ -24,7 +24,15 @@ class HgDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doInstall(PackageInterface $package, $path, $url) + protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null) + { + + } + + /** + * {@inheritDoc} + */ + protected function doInstall(PackageInterface $package, $path, $url) { $hgUtils = new HgUtils($this->io, $this->config, $this->process); @@ -44,7 +52,7 @@ class HgDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) + protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { $hgUtils = new HgUtils($this->io, $this->config, $this->process); diff --git a/src/Composer/Downloader/PathDownloader.php b/src/Composer/Downloader/PathDownloader.php index e289c0173..b84396416 100644 --- a/src/Composer/Downloader/PathDownloader.php +++ b/src/Composer/Downloader/PathDownloader.php @@ -37,7 +37,7 @@ class PathDownloader extends FileDownloader implements VcsCapableDownloaderInter /** * {@inheritdoc} */ - public function download(PackageInterface $package, $path, $output = true) + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null, $output = true) { $url = $package->getDistUrl(); $realUrl = realpath($url); diff --git a/src/Composer/Downloader/PerforceDownloader.php b/src/Composer/Downloader/PerforceDownloader.php index 777866714..8be866929 100644 --- a/src/Composer/Downloader/PerforceDownloader.php +++ b/src/Composer/Downloader/PerforceDownloader.php @@ -24,6 +24,14 @@ class PerforceDownloader extends VcsDownloader /** @var Perforce */ protected $perforce; + /** + * {@inheritDoc} + */ + protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null) + { + + } + /** * {@inheritDoc} */ @@ -76,7 +84,7 @@ class PerforceDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) + protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { $this->doInstall($target, $path, $url); } diff --git a/src/Composer/Downloader/SvnDownloader.php b/src/Composer/Downloader/SvnDownloader.php index 0aae163c6..35f01eb68 100644 --- a/src/Composer/Downloader/SvnDownloader.php +++ b/src/Composer/Downloader/SvnDownloader.php @@ -28,7 +28,15 @@ class SvnDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doInstall(PackageInterface $package, $path, $url) + protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null) + { + + } + + /** + * {@inheritDoc} + */ + protected function doInstall(PackageInterface $package, $path, $url) { SvnUtil::cleanEnv(); $ref = $package->getSourceReference(); @@ -48,7 +56,7 @@ class SvnDownloader extends VcsDownloader /** * {@inheritDoc} */ - public function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) + protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url) { SvnUtil::cleanEnv(); $ref = $target->getSourceReference(); diff --git a/src/Composer/Downloader/VcsDownloader.php b/src/Composer/Downloader/VcsDownloader.php index b87f6433a..15bf55072 100644 --- a/src/Composer/Downloader/VcsDownloader.php +++ b/src/Composer/Downloader/VcsDownloader.php @@ -54,9 +54,57 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa /** * {@inheritDoc} */ - public function download(PackageInterface $package, $path) + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null) { - // noop for now, ideally we would do a git fetch already here, or make sure the cached git repo is synced, etc. + if (!$package->getSourceReference()) { + throw new \InvalidArgumentException('Package '.$package->getPrettyName().' is missing reference information'); + } + + $urls = $this->prepareUrls($package->getSourceUrls()); + + while ($url = array_shift($urls)) { + try { + return $this->doDownload($package, $path, $url, $prevPackage); + } catch (\Exception $e) { + // rethrow phpunit exceptions to avoid hard to debug bug failures + if ($e instanceof \PHPUnit_Framework_Exception) { + throw $e; + } + if ($this->io->isDebug()) { + $this->io->writeError('Failed: ['.get_class($e).'] '.$e->getMessage()); + } elseif (count($urls)) { + $this->io->writeError(' Failed, trying the next URL'); + } + if (!count($urls)) { + throw $e; + } + } + } + } + + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, $path, PackageInterface $prevPackage = null) + { + if ($type === 'update') { + $this->cleanChanges($prevPackage, $path, true); + } elseif ($type === 'install') { + $this->filesystem->emptyDirectory($path); + } elseif ($type === 'uninstall') { + $this->cleanChanges($package, $path, false); + } + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, $path, PackageInterface $prevPackage = null) + { + if ($type === 'update') { + // TODO keep track of whether prepare was called for this package + $this->reapplyChanges($path); + } } /** @@ -69,32 +117,10 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa } $this->io->writeError(" - Installing " . $package->getName() . " (" . $package->getFullPrettyVersion() . "): ", false); - $this->filesystem->emptyDirectory($path); - $urls = $package->getSourceUrls(); + $urls = $this->prepareUrls($package->getSourceUrls()); while ($url = array_shift($urls)) { try { - if (Filesystem::isLocalPath($url)) { - // realpath() below will not understand - // url that starts with "file://" - $needle = 'file://'; - $isFileProtocol = false; - if (0 === strpos($url, $needle)) { - $url = substr($url, strlen($needle)); - $isFileProtocol = true; - } - - // realpath() below will not understand %20 spaces etc. - if (false !== strpos($url, '%')) { - $url = rawurldecode($url); - } - - $url = realpath($url); - - if ($isFileProtocol) { - $url = $needle . $url; - } - } $this->doInstall($package, $path, $url); break; } catch (\Exception $e) { @@ -141,15 +167,11 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Updating' : 'Downgrading'; $this->io->writeError(" - " . $actionName . " " . $name . " (" . $from . " => " . $to . "): ", false); - $this->cleanChanges($initial, $path, true); - $urls = $target->getSourceUrls(); + $urls = $this->prepareUrls($target->getSourceUrls()); $exception = null; while ($url = array_shift($urls)) { try { - if (Filesystem::isLocalPath($url)) { - $url = realpath($url); - } $this->doUpdate($initial, $target, $path, $url); $exception = null; @@ -167,8 +189,6 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa } } - $this->reapplyChanges($path); - // print the commit logs if in verbose mode and VCS metadata is present // because in case of missing metadata code would trigger another exception if (!$exception && $this->io->isVerbose() && $this->hasMetadataRepository($path)) { @@ -204,7 +224,6 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa public function remove(PackageInterface $package, $path) { $this->io->writeError(" - Removing " . $package->getName() . " (" . $package->getPrettyVersion() . ")"); - $this->cleanChanges($package, $path, false); if (!$this->filesystem->removeDirectory($path)) { throw new \RuntimeException('Could not completely delete '.$path.', aborting.'); } @@ -243,7 +262,7 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa } /** - * Guarantee that no changes have been made to the local copy + * Reapply previously stashes changes if applicable, only called after an update (regardless if successful or not) * * @param string $path * @throws \RuntimeException in case the operation must be aborted or the patch does not apply cleanly @@ -252,12 +271,26 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa { } + /** + * Downloads data needed to run an install/update later + * + * @param PackageInterface $package package instance + * @param string $path download path + * @param string $url package url + * @param PackageInterface|null $prevPackage previous package (in case of an update) + * + * @return PromiseInterface|null + */ + abstract protected function doDownload(PackageInterface $package, $path, $url, PackageInterface $prevPackage = null); + /** * Downloads specific package into specific folder. * * @param PackageInterface $package package instance * @param string $path download path * @param string $url package url + * + * @return PromiseInterface|null */ abstract protected function doInstall(PackageInterface $package, $path, $url); @@ -268,6 +301,8 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa * @param PackageInterface $target updated package * @param string $path download path * @param string $url package url + * + * @return PromiseInterface|null */ abstract protected function doUpdate(PackageInterface $initial, PackageInterface $target, $path, $url); @@ -289,4 +324,33 @@ abstract class VcsDownloader implements DownloaderInterface, ChangeReportInterfa * @return bool */ abstract protected function hasMetadataRepository($path); + + private function prepareUrls(array $urls) + { + foreach ($urls as $index => $url) { + if (Filesystem::isLocalPath($url)) { + // realpath() below will not understand + // url that starts with "file://" + $fileProtocol = 'file://'; + $isFileProtocol = false; + if (0 === strpos($url, $fileProtocol)) { + $url = substr($url, strlen($fileProtocol)); + $isFileProtocol = true; + } + + // realpath() below will not understand %20 spaces etc. + if (false !== strpos($url, '%')) { + $url = rawurldecode($url); + } + + $urls[$index] = realpath($url); + + if ($isFileProtocol) { + $urls[$index] = $fileProtocol . $urls[$index]; + } + } + } + + return $urls; + } } diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index bd8d3b499..160bae1d6 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -47,7 +47,7 @@ class ZipDownloader extends ArchiveDownloader /** * {@inheritDoc} */ - public function download(PackageInterface $package, $path, $output = true) + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null, $output = true) { if (null === self::$hasSystemUnzip) { $finder = new ExecutableFinder; @@ -76,7 +76,7 @@ class ZipDownloader extends ArchiveDownloader } } - return parent::download($package, $path, $output); + return parent::download($package, $path, $prevPackage, $output); } /** diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index ce10dc4da..f018c0a31 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -177,11 +177,52 @@ class InstallationManager $promise = $installer->download($target, $operation->getInitialPackage()); } - if (isset($promise)) { + if (!empty($promise)) { $this->loop->wait(array($promise)); } - $this->$method($repo, $operation); + $e = null; + try { + if ($method === 'install' || $method === 'uninstall') { + $package = $operation->getPackage(); + $installer = $this->getInstaller($package->getType()); + $promise = $installer->prepare($method, $package); + } elseif ($method === 'update') { + $target = $operation->getTargetPackage(); + $targetType = $target->getType(); + $installer = $this->getInstaller($targetType); + $promise = $installer->prepare('update', $target, $operation->getInitialPackage()); + } + + if (!empty($promise)) { + $this->loop->wait(array($promise)); + } + + $promise = $this->$method($repo, $operation); + if (!empty($promise)) { + $this->loop->wait(array($promise)); + } + } catch (\Exception $e) { + } + + if ($method === 'install' || $method === 'uninstall') { + $package = $operation->getPackage(); + $installer = $this->getInstaller($package->getType()); + $promise = $installer->cleanup($method, $package); + } elseif ($method === 'update') { + $target = $operation->getTargetPackage(); + $targetType = $target->getType(); + $installer = $this->getInstaller($targetType); + $promise = $installer->cleanup('update', $target, $operation->getInitialPackage()); + } + + if (!empty($promise)) { + $this->loop->wait(array($promise)); + } + + if ($e) { + throw $e; + } } /** @@ -194,8 +235,10 @@ class InstallationManager { $package = $operation->getPackage(); $installer = $this->getInstaller($package->getType()); - $installer->install($repo, $package); + $promise = $installer->install($repo, $package); $this->markForNotification($package); + + return $promise; } /** @@ -214,13 +257,15 @@ class InstallationManager if ($initialType === $targetType) { $installer = $this->getInstaller($initialType); - $installer->update($repo, $initial, $target); + $promise = $installer->update($repo, $initial, $target); $this->markForNotification($target); } else { $this->getInstaller($initialType)->uninstall($repo, $initial); $installer = $this->getInstaller($targetType); - $installer->install($repo, $target); + $promise = $installer->install($repo, $target); } + + return $promise; } /** @@ -233,7 +278,8 @@ class InstallationManager { $package = $operation->getPackage(); $installer = $this->getInstaller($package->getType()); - $installer->uninstall($repo, $package); + + return $installer->uninstall($repo, $package); } /** diff --git a/src/Composer/Installer/InstallerInterface.php b/src/Composer/Installer/InstallerInterface.php index 310c5fcfc..cc4bef7e9 100644 --- a/src/Composer/Installer/InstallerInterface.php +++ b/src/Composer/Installer/InstallerInterface.php @@ -46,26 +46,43 @@ interface InstallerInterface /** * Downloads the files needed to later install the given package. * - * @param PackageInterface $package package instance - * @param PackageInterface $prevPackage previous package instance in case of an update + * @param PackageInterface $package package instance + * @param PackageInterface $prevPackage previous package instance in case of an update * @return PromiseInterface|null */ public function download(PackageInterface $package, PackageInterface $prevPackage = null); + /** + * Do anything that needs to be done between all downloads have been completed and the actual operation is executed + * + * All packages get first downloaded, then all together prepared, then all together installed/updated/uninstalled. Therefore + * for error recovery it is important to avoid failing during install/update/uninstall as much as possible, and risky things or + * user prompts should happen in the prepare step rather. In case of failure, cleanup() will be called so that changes can + * be undone as much as possible. + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param PackageInterface $prevPackage previous package instance in case of an update + * @return PromiseInterface|null + */ + public function prepare($type, PackageInterface $package, PackageInterface $prevPackage = null); + /** * Installs specific package. * - * @param InstalledRepositoryInterface $repo repository in which to check - * @param PackageInterface $package package instance + * @param InstalledRepositoryInterface $repo repository in which to check + * @param PackageInterface $package package instance + * @return PromiseInterface|null */ public function install(InstalledRepositoryInterface $repo, PackageInterface $package); /** * Updates specific package. * - * @param InstalledRepositoryInterface $repo repository in which to check - * @param PackageInterface $initial already installed package version - * @param PackageInterface $target updated version + * @param InstalledRepositoryInterface $repo repository in which to check + * @param PackageInterface $initial already installed package version + * @param PackageInterface $target updated version + * @return PromiseInterface|null * * @throws InvalidArgumentException if $initial package is not installed */ @@ -74,11 +91,26 @@ interface InstallerInterface /** * Uninstalls specific package. * - * @param InstalledRepositoryInterface $repo repository in which to check - * @param PackageInterface $package package instance + * @param InstalledRepositoryInterface $repo repository in which to check + * @param PackageInterface $package package instance + * @return PromiseInterface|null */ public function uninstall(InstalledRepositoryInterface $repo, PackageInterface $package); + /** + * Do anything to cleanup changes applied in the prepare or install/update/uninstall steps + * + * Note that cleanup will be called for all packages regardless if they failed an operation or not, to give + * all installers a change to cleanup things they did previously, so you need to keep track of changes + * applied in the installer/downloader themselves. + * + * @param string $type one of install/update/uninstall + * @param PackageInterface $package package instance + * @param PackageInterface $prevPackage previous package instance in case of an update + * @return PromiseInterface|null + */ + public function cleanup($type, PackageInterface $package, PackageInterface $prevPackage = null); + /** * Returns the installation path of a package * diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index a89553b1b..5e99e1f47 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -85,6 +85,9 @@ class LibraryInstaller implements InstallerInterface, BinaryPresenceInterface return (Platform::isWindows() && $this->filesystem->isJunction($installPath)) || is_link($installPath); } + /** + * {@inheritDoc} + */ public function download(PackageInterface $package, PackageInterface $prevPackage = null) { $this->initializeVendorDir(); @@ -93,6 +96,28 @@ class LibraryInstaller implements InstallerInterface, BinaryPresenceInterface return $this->downloadManager->download($package, $downloadPath, $prevPackage); } + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + $this->initializeVendorDir(); + $downloadPath = $this->getInstallPath($package); + + return $this->downloadManager->prepare($type, $package, $downloadPath, $prevPackage); + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + $this->initializeVendorDir(); + $downloadPath = $this->getInstallPath($package); + + return $this->downloadManager->cleanup($type, $package, $downloadPath, $prevPackage); + } + /** * {@inheritDoc} */ diff --git a/src/Composer/Installer/MetapackageInstaller.php b/src/Composer/Installer/MetapackageInstaller.php index b47c00740..1ed6beb71 100644 --- a/src/Composer/Installer/MetapackageInstaller.php +++ b/src/Composer/Installer/MetapackageInstaller.php @@ -55,6 +55,22 @@ class MetapackageInstaller implements InstallerInterface // noop } + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + // noop + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + // noop + } + /** * {@inheritDoc} */ diff --git a/src/Composer/Installer/NoopInstaller.php b/src/Composer/Installer/NoopInstaller.php index 51df3c305..4fe581ff5 100644 --- a/src/Composer/Installer/NoopInstaller.php +++ b/src/Composer/Installer/NoopInstaller.php @@ -47,6 +47,20 @@ class NoopInstaller implements InstallerInterface { } + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + } + /** * {@inheritDoc} */ diff --git a/src/Composer/Installer/ProjectInstaller.php b/src/Composer/Installer/ProjectInstaller.php index 350b220f5..069c741ec 100644 --- a/src/Composer/Installer/ProjectInstaller.php +++ b/src/Composer/Installer/ProjectInstaller.php @@ -71,6 +71,22 @@ class ProjectInstaller implements InstallerInterface return $this->downloadManager->download($package, $installPath, $prevPackage); } + /** + * {@inheritDoc} + */ + public function prepare($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + $this->downloadManager->prepare($type, $package, $this->installPath, $prevPackage); + } + + /** + * {@inheritDoc} + */ + public function cleanup($type, PackageInterface $package, PackageInterface $prevPackage = null) + { + $this->downloadManager->cleanup($type, $package, $this->installPath, $prevPackage); + } + /** * {@inheritDoc} */ diff --git a/src/Composer/Util/Git.php b/src/Composer/Util/Git.php index 74e5c286f..48e91ba84 100644 --- a/src/Composer/Util/Git.php +++ b/src/Composer/Util/Git.php @@ -224,6 +224,10 @@ class Git public function syncMirror($url, $dir) { + if (getenv('COMPOSER_DISABLE_NETWORK')) { + return false; + } + // update the repo if it is a valid git repository if (is_dir($dir) && 0 === $this->process->execute('git rev-parse --git-dir', $output, $dir) && trim($output) === '.') { try { @@ -260,9 +264,7 @@ class Git } } - $this->syncMirror($url, $dir); - - return false; + return $this->syncMirror($url, $dir); } private function isAuthenticationFailure($url, &$match) diff --git a/tests/Composer/Test/Downloader/FossilDownloaderTest.php b/tests/Composer/Test/Downloader/FossilDownloaderTest.php index 9ab7b6b84..4ec8fed45 100644 --- a/tests/Composer/Test/Downloader/FossilDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FossilDownloaderTest.php @@ -48,7 +48,7 @@ class FossilDownloaderTest extends TestCase /** * @expectedException \InvalidArgumentException */ - public function testDownloadForPackageWithoutSourceReference() + public function testInstallForPackageWithoutSourceReference() { $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->once()) @@ -59,7 +59,7 @@ class FossilDownloaderTest extends TestCase $downloader->install($packageMock, '/path'); } - public function testDownload() + public function testInstall() { $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -104,7 +104,9 @@ class FossilDownloaderTest extends TestCase ->will($this->returnValue(null)); $downloader = $this->getDownloaderMock(); + $downloader->prepare('update', $sourcePackageMock, '/path', $initialPackageMock); $downloader->update($initialPackageMock, $sourcePackageMock, '/path'); + $downloader->cleanup('update', $sourcePackageMock, '/path', $initialPackageMock); } public function testUpdate() @@ -140,7 +142,9 @@ class FossilDownloaderTest extends TestCase ->will($this->returnValue(0)); $downloader = $this->getDownloaderMock(null, null, $processExecutor); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); } public function testRemove() diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index b9a85a666..bf1402186 100644 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -17,6 +17,7 @@ use Composer\Config; use Composer\Test\TestCase; use Composer\Util\Filesystem; use Composer\Util\Platform; +use Prophecy\Argument; class GitDownloaderTest extends TestCase { @@ -79,7 +80,10 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue(null)); $downloader = $this->getDownloaderMock(); + $downloader->download($packageMock, '/path'); + $downloader->prepare('install', $packageMock, '/path'); $downloader->install($packageMock, '/path'); + $downloader->cleanup('install', $packageMock, '/path'); } public function testDownload() @@ -130,7 +134,10 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue(0)); $downloader = $this->getDownloaderMock(null, null, $processExecutor); + $downloader->download($packageMock, 'composerPath'); + $downloader->prepare('install', $packageMock, 'composerPath'); $downloader->install($packageMock, 'composerPath'); + $downloader->cleanup('install', $packageMock, 'composerPath'); } public function testDownloadWithCache() @@ -195,7 +202,10 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue(0)); $downloader = $this->getDownloaderMock(null, $config, $processExecutor); + $downloader->download($packageMock, 'composerPath'); + $downloader->prepare('install', $packageMock, 'composerPath'); $downloader->install($packageMock, 'composerPath'); + $downloader->cleanup('install', $packageMock, 'composerPath'); @rmdir($cachePath); } @@ -265,7 +275,10 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue(0)); $downloader = $this->getDownloaderMock(null, new Config(), $processExecutor); + $downloader->download($packageMock, 'composerPath'); + $downloader->prepare('install', $packageMock, 'composerPath'); $downloader->install($packageMock, 'composerPath'); + $downloader->cleanup('install', $packageMock, 'composerPath'); } public function pushUrlProvider() @@ -329,12 +342,12 @@ class GitDownloaderTest extends TestCase $config->merge(array('config' => array('github-protocols' => $protocols))); $downloader = $this->getDownloaderMock(null, $config, $processExecutor); + $downloader->download($packageMock, 'composerPath'); + $downloader->prepare('install', $packageMock, 'composerPath'); $downloader->install($packageMock, 'composerPath'); + $downloader->cleanup('install', $packageMock, 'composerPath'); } - /** - * @expectedException \RuntimeException - */ public function testDownloadThrowsRuntimeExceptionIfGitCommandFails() { $expectedGitCommand = $this->winCompat("git clone --no-checkout 'https://example.com/composer/composer' 'composerPath' && cd 'composerPath' && git remote add composer 'https://example.com/composer/composer' && git fetch composer"); @@ -359,8 +372,20 @@ class GitDownloaderTest extends TestCase ->with($this->equalTo($expectedGitCommand)) ->will($this->returnValue(1)); - $downloader = $this->getDownloaderMock(null, null, $processExecutor); - $downloader->install($packageMock, 'composerPath'); + // not using PHPUnit's expected exception because Prophecy exceptions extend from RuntimeException too so it is not safe + try { + $downloader = $this->getDownloaderMock(null, null, $processExecutor); + $downloader->download($packageMock, 'composerPath'); + $downloader->prepare('install', $packageMock, 'composerPath'); + $downloader->install($packageMock, 'composerPath'); + $downloader->cleanup('install', $packageMock, 'composerPath'); + $this->fail('This test should throw'); + } catch (\RuntimeException $e) { + if ('RuntimeException' !== get_class($e)) { + throw $e; + } + $this->assertEquals('RuntimeException', get_class($e)); + } } /** @@ -375,7 +400,10 @@ class GitDownloaderTest extends TestCase ->will($this->returnValue(null)); $downloader = $this->getDownloaderMock(); + $downloader->download($sourcePackageMock, '/path', $initialPackageMock); + $downloader->prepare('update', $sourcePackageMock, '/path', $initialPackageMock); $downloader->update($initialPackageMock, $sourcePackageMock, '/path'); + $downloader->cleanup('update', $sourcePackageMock, '/path', $initialPackageMock); } public function testUpdate() @@ -392,39 +420,22 @@ class GitDownloaderTest extends TestCase $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') - ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) - ->method('execute') - ->with($this->equalTo($this->winCompat($expectedGitUpdateCommand)), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(5)) - ->method('execute') - ->with($this->equalTo('git branch -r')) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(6)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) - ->will($this->returnValue(0)); + + $process = $this->prophesize('Composer\Util\ProcessExecutor'); + $process->execute($this->winCompat('git --version'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git show-ref --head -d'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git status --porcelain --untracked-files=no'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git remote -v'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git branch -r'), Argument::cetera())->willReturn(0); + $process->execute($expectedGitUpdateCommand, null, $this->winCompat($this->workingDir))->willReturn(0)->shouldBeCalled(); + $process->execute($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --"), null, $this->winCompat($this->workingDir))->willReturn(0)->shouldBeCalled(); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); - $downloader = $this->getDownloaderMock(null, new Config(), $processExecutor); + $downloader = $this->getDownloaderMock(null, new Config(), $process->reveal()); + $downloader->download($packageMock, $this->workingDir, $packageMock); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); } public function testUpdateWithNewRepoUrl() @@ -444,27 +455,20 @@ class GitDownloaderTest extends TestCase $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') - ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) + ->with($this->equalTo($this->winCompat("git --version"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) ->method('execute') - ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) + ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(2)) ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnCallback(function ($cmd, &$output, $cwd) { - $output = 'origin https://github.com/old/url (fetch) -origin https://github.com/old/url (push) -composer https://github.com/old/url (fetch) -composer https://github.com/old/url (push) -'; - - return 0; - })); + ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) + ->will($this->returnValue(0)); $processExecutor->expects($this->at(3)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote -v"))) @@ -482,26 +486,41 @@ composer https://github.com/old/url (push) ->with($this->equalTo($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); $processExecutor->expects($this->at(7)) + ->method('execute') + ->with($this->equalTo($this->winCompat("git remote -v"))) + ->will($this->returnCallback(function ($cmd, &$output, $cwd) { + $output = 'origin https://github.com/old/url (fetch) +origin https://github.com/old/url (push) +composer https://github.com/old/url (fetch) +composer https://github.com/old/url (push) +'; + + return 0; + })); + $processExecutor->expects($this->at(8)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote set-url origin 'https://github.com/composer/composer'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); - $processExecutor->expects($this->at(8)) + $processExecutor->expects($this->at(9)) ->method('execute') ->with($this->equalTo($this->winCompat("git remote set-url --push origin 'git@github.com:composer/composer.git'")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) ->will($this->returnValue(0)); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); $downloader = $this->getDownloaderMock(null, new Config(), $processExecutor); + $downloader->download($packageMock, $this->workingDir, $packageMock); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); } /** * @group failing - * @expectedException \RuntimeException */ public function testUpdateThrowsRuntimeExceptionIfGitCommandFails() { $expectedGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedGitUpdateCommand2 = $this->winCompat("git remote set-url composer 'git@github.com:composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); $packageMock->expects($this->any()) @@ -513,36 +532,38 @@ composer https://github.com/old/url (push) $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') - ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) - ->method('execute') - ->with($this->equalTo($expectedGitUpdateCommand)) - ->will($this->returnValue(1)); + + $process = $this->prophesize('Composer\Util\ProcessExecutor'); + $process->execute($this->winCompat('git --version'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git show-ref --head -d'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git status --porcelain --untracked-files=no'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git remote -v'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git branch -r'), Argument::cetera())->willReturn(0); + $process->execute($expectedGitUpdateCommand, null, $this->winCompat($this->workingDir))->willReturn(1)->shouldBeCalled(); + $process->execute($expectedGitUpdateCommand2, null, $this->winCompat($this->workingDir))->willReturn(1)->shouldBeCalled(); + $process->getErrorOutput()->willReturn(''); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); - $downloader = $this->getDownloaderMock(null, new Config(), $processExecutor); - $downloader->update($packageMock, $packageMock, $this->workingDir); + + // not using PHPUnit's expected exception because Prophecy exceptions extend from RuntimeException too so it is not safe + try { + $downloader = $this->getDownloaderMock(null, new Config(), $process->reveal()); + $downloader->download($packageMock, $this->workingDir, $packageMock); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); + $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); + $this->fail('This test should throw'); + } catch (\RuntimeException $e) { + if ('RuntimeException' !== get_class($e)) { + throw $e; + } + $this->assertEquals('RuntimeException', get_class($e)); + } } public function testUpdateDoesntThrowsRuntimeExceptionIfGitCommandFailsAtFirstButIsAbleToRecover() { - $expectedFirstGitUpdateCommand = $this->winCompat("git remote set-url composer '' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); + $expectedFirstGitUpdateCommand = $this->winCompat("git remote set-url composer '".(Platform::isWindows() ? 'C:\\' : '/')."' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); $expectedSecondGitUpdateCommand = $this->winCompat("git remote set-url composer 'https://github.com/composer/composer' && git rev-parse --quiet --verify 'ref^{commit}' || (git fetch composer && git fetch --tags composer)"); $packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock(); @@ -554,52 +575,24 @@ composer https://github.com/old/url (push) ->will($this->returnValue('1.0.0.0')); $packageMock->expects($this->any()) ->method('getSourceUrls') - ->will($this->returnValue(array('/foo/bar', 'https://github.com/composer/composer'))); - $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); - $processExecutor->expects($this->at(0)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git show-ref --head -d"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(1)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git status --porcelain --untracked-files=no"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(2)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(3)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(4)) - ->method('execute') - ->with($this->equalTo($expectedFirstGitUpdateCommand)) - ->will($this->returnValue(1)); - $processExecutor->expects($this->at(6)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git --version"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(7)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(8)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git remote -v"))) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(9)) - ->method('execute') - ->with($this->equalTo($expectedSecondGitUpdateCommand)) - ->will($this->returnValue(0)); - $processExecutor->expects($this->at(11)) - ->method('execute') - ->with($this->equalTo($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --")), $this->equalTo(null), $this->equalTo($this->winCompat($this->workingDir))) - ->will($this->returnValue(0)); + ->will($this->returnValue(array(Platform::isWindows() ? 'C:\\' : '/', 'https://github.com/composer/composer'))); + + $process = $this->prophesize('Composer\Util\ProcessExecutor'); + $process->execute($this->winCompat('git --version'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git show-ref --head -d'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git status --porcelain --untracked-files=no'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git remote -v'), Argument::cetera())->willReturn(0); + $process->execute($this->winCompat('git branch -r'), Argument::cetera())->willReturn(0); + $process->execute($expectedFirstGitUpdateCommand, Argument::cetera())->willReturn(1)->shouldBeCalled(); + $process->execute($expectedSecondGitUpdateCommand, Argument::cetera())->willReturn(0)->shouldBeCalled(); + $process->execute($this->winCompat("git checkout 'ref' -- && git reset --hard 'ref' --"), null, $this->winCompat($this->workingDir))->willReturn(0)->shouldBeCalled(); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); - $downloader = $this->getDownloaderMock(null, new Config(), $processExecutor); + $downloader = $this->getDownloaderMock(null, new Config(), $process->reveal()); + $downloader->download($packageMock, $this->workingDir, $packageMock); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); } public function testDowngradeShowsAppropriateMessage() @@ -644,7 +637,10 @@ composer https://github.com/old/url (push) $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor); + $downloader->download($newPackage, $this->workingDir, $oldPackage); + $downloader->prepare('update', $newPackage, $this->workingDir, $oldPackage); $downloader->update($oldPackage, $newPackage, $this->workingDir); + $downloader->cleanup('update', $newPackage, $this->workingDir, $oldPackage); } public function testNotUsingDowngradingWithReferences() @@ -679,11 +675,14 @@ composer https://github.com/old/url (push) $ioMock = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(); $ioMock->expects($this->at(0)) ->method('writeError') - ->with($this->stringContains('updating')); + ->with($this->stringContains('Updating')); $this->fs->ensureDirectoryExists($this->workingDir.'/.git'); $downloader = $this->getDownloaderMock($ioMock, null, $processExecutor); + $downloader->download($newPackage, $this->workingDir, $oldPackage); + $downloader->prepare('update', $newPackage, $this->workingDir, $oldPackage); $downloader->update($oldPackage, $newPackage, $this->workingDir); + $downloader->cleanup('update', $newPackage, $this->workingDir, $oldPackage); } public function testRemove() @@ -703,7 +702,9 @@ composer https://github.com/old/url (push) ->will($this->returnValue(true)); $downloader = $this->getDownloaderMock(null, null, $processExecutor, $filesystem); + $downloader->prepare('uninstall', $packageMock, 'composerPath'); $downloader->remove($packageMock, 'composerPath'); + $downloader->cleanup('uninstall', $packageMock, 'composerPath'); } public function testGetInstallationSource() diff --git a/tests/Composer/Test/Downloader/HgDownloaderTest.php b/tests/Composer/Test/Downloader/HgDownloaderTest.php index a4219d143..c69d497ce 100644 --- a/tests/Composer/Test/Downloader/HgDownloaderTest.php +++ b/tests/Composer/Test/Downloader/HgDownloaderTest.php @@ -98,7 +98,9 @@ class HgDownloaderTest extends TestCase ->will($this->returnValue(null)); $downloader = $this->getDownloaderMock(); + $downloader->prepare('update', $sourcePackageMock, '/path', $initialPackageMock); $downloader->update($initialPackageMock, $sourcePackageMock, '/path'); + $downloader->cleanup('update', $sourcePackageMock, '/path', $initialPackageMock); } public function testUpdate() @@ -129,7 +131,9 @@ class HgDownloaderTest extends TestCase ->will($this->returnValue(0)); $downloader = $this->getDownloaderMock(null, null, $processExecutor); + $downloader->prepare('update', $packageMock, $this->workingDir, $packageMock); $downloader->update($packageMock, $packageMock, $this->workingDir); + $downloader->cleanup('update', $packageMock, $this->workingDir, $packageMock); } public function testRemove() @@ -148,7 +152,9 @@ class HgDownloaderTest extends TestCase ->will($this->returnValue(true)); $downloader = $this->getDownloaderMock(null, null, $processExecutor, $filesystem); + $downloader->prepare('uninstall', $packageMock, 'composerPath'); $downloader->remove($packageMock, 'composerPath'); + $downloader->cleanup('uninstall', $packageMock, 'composerPath'); } public function testGetInstallationSource() diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index e69149271..63c407218 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -338,7 +338,7 @@ class ZipDownloaderTest extends TestCase class MockedZipDownloader extends ZipDownloader { - public function download(PackageInterface $package, $path, $output = true) + public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null, $output = true) { return; }