From d5286d0cb8216d2a76f5e63c03e4bcb6f0fa0c0a Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 08:30:59 +0200 Subject: [PATCH 01/11] Add a way for FileDownloader subclasses to add paths to the cleanup stage --- src/Composer/Downloader/FileDownloader.php | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index d7c4bcabe..f5674cf90 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -55,6 +55,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface * @private this is only public for php 5.3 support in closures */ public $lastCacheWrites = array(); + private $additionalCleanupPaths = array(); /** * Constructor. @@ -258,6 +259,12 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $path, ); + if (isset($this->additionalCleanupPaths[$package->getName()])) { + foreach ($this->additionalCleanupPaths[$package->getName()] as $path) { + $this->filesystem->remove($path); + } + } + foreach ($dirsToCleanUp as $dir) { if (is_dir($dir) && $this->filesystem->isDirEmpty($dir)) { $this->filesystem->removeDirectory($dir); @@ -291,6 +298,29 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface } } + /** + * TODO mark private in v3 + * @protected This is public due to PHP 5.3 + */ + public function addCleanupPath(PackageInterface $package, $path) + { + $this->additionalCleanupPaths[$package->getName()][] = $path; + } + + /** + * TODO mark private in v3 + * @protected This is public due to PHP 5.3 + */ + public function removeCleanupPath(PackageInterface $package, $path) + { + if (isset($this->additionalCleanupPaths[$package->getName()])) { + $idx = array_search($path, $this->additionalCleanupPaths[$package->getName()]); + if (false !== $idx) { + unset($this->additionalCleanupPaths[$package->getName()][$idx]); + } + } + } + /** * {@inheritDoc} */ From 0dad963cd89be409c729830dea4128a021129090 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 08:33:44 +0200 Subject: [PATCH 02/11] Add executeAsync to ProcessExecutor and allow Loop class to wait on it in addition to HttpDownloader --- src/Composer/Factory.php | 2 +- src/Composer/Util/HttpDownloader.php | 73 ++++--- src/Composer/Util/Loop.php | 35 ++- src/Composer/Util/ProcessExecutor.php | 204 ++++++++++++++++++ .../Test/Downloader/FileDownloaderTest.php | 6 +- .../Test/Downloader/XzDownloaderTest.php | 2 +- .../Test/Downloader/ZipDownloaderTest.php | 2 +- .../Repository/ComposerRepositoryTest.php | 10 +- 8 files changed, 294 insertions(+), 40 deletions(-) diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index 04da08e31..5bc41ee6b 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -336,7 +336,7 @@ class Factory $httpDownloader = self::createHttpDownloader($io, $config); $process = new ProcessExecutor($io); - $loop = new Loop($httpDownloader); + $loop = new Loop($httpDownloader, $process); $composer->setLoop($loop); // initialize event dispatcher diff --git a/src/Composer/Util/HttpDownloader.php b/src/Composer/Util/HttpDownloader.php index 2fa8fa716..41ced41e3 100644 --- a/src/Composer/Util/HttpDownloader.php +++ b/src/Composer/Util/HttpDownloader.php @@ -44,6 +44,7 @@ class HttpDownloader private $rfs; private $idGen = 0; private $disabled; + private $allowAsync = false; /** * @param IOInterface $io The IO instance @@ -139,6 +140,10 @@ class HttpDownloader 'origin' => Url::getOrigin($this->config, $request['url']), ); + if (!$sync && !$this->allowAsync) { + throw new \LogicException('You must use the HttpDownloader instance which is part of a Composer\Loop instance to be able to run async http requests'); + } + // capture username/password from URL if there is one if (preg_match('{^https?://([^:/]+):([^@/]+)@([^/]+)}i', $request['url'], $match)) { $this->io->setAuthentication($job['origin'], rawurldecode($match[1]), rawurldecode($match[2])); @@ -189,7 +194,6 @@ class HttpDownloader // TODO 3.0 this should be done directly on $this when PHP 5.3 is dropped $downloader->markJobDone(); - $downloader->scheduleNextJob(); return $response; }, function ($e) use (&$job, $downloader) { @@ -197,7 +201,6 @@ class HttpDownloader $job['exception'] = $e; $downloader->markJobDone(); - $downloader->scheduleNextJob(); throw $e; }); @@ -251,13 +254,7 @@ class HttpDownloader public function markJobDone() { $this->runningJobs--; - } - /** - * @private - */ - public function scheduleNextJob() - { foreach ($this->jobs as $job) { if ($job['status'] === self::STATUS_QUEUED) { $this->startJob($job['id']); @@ -268,36 +265,52 @@ class HttpDownloader } } - public function wait($index = null, $progress = false) + public function wait($index = null) { while (true) { - if ($this->curl) { - $this->curl->tick(); - } - - if (null !== $index) { - if ($this->jobs[$index]['status'] === self::STATUS_COMPLETED || $this->jobs[$index]['status'] === self::STATUS_FAILED) { - return; - } - } else { - $done = true; - foreach ($this->jobs as $job) { - if (!in_array($job['status'], array(self::STATUS_COMPLETED, self::STATUS_FAILED), true)) { - $done = false; - break; - } elseif (!$job['sync']) { - unset($this->jobs[$job['id']]); - } - } - if ($done) { - return; - } + if (!$this->hasActiveJob($index)) { + return; } usleep(1000); } } + /** + * @internal + */ + public function enableAsync() + { + $this->allowAsync = true; + } + + /** + * @internal + */ + public function hasActiveJob($index = null) + { + if ($this->curl) { + $this->curl->tick(); + } + + if (null !== $index) { + if ($this->jobs[$index]['status'] === self::STATUS_COMPLETED || $this->jobs[$index]['status'] === self::STATUS_FAILED) { + return false; + } + return true; + } + + foreach ($this->jobs as $job) { + if (!in_array($job['status'], array(self::STATUS_COMPLETED, self::STATUS_FAILED), true)) { + return true; + } elseif (!$job['sync']) { + unset($this->jobs[$job['id']]); + } + } + + return false; + } + private function getResponse($index) { if (!isset($this->jobs[$index])) { diff --git a/src/Composer/Util/Loop.php b/src/Composer/Util/Loop.php index dfaa2ac53..b0061ba2d 100644 --- a/src/Composer/Util/Loop.php +++ b/src/Composer/Util/Loop.php @@ -21,10 +21,19 @@ use React\Promise\Promise; class Loop { private $httpDownloader; + private $processExecutor; + private $currentPromises; - public function __construct(HttpDownloader $httpDownloader) + public function __construct(HttpDownloader $httpDownloader = null, ProcessExecutor $processExecutor = null) { $this->httpDownloader = $httpDownloader; + if ($this->httpDownloader) { + $this->httpDownloader->enableAsync(); + } + $this->processExecutor = $processExecutor; + if ($this->processExecutor) { + $this->processExecutor->enableAsync(); + } } public function wait(array $promises) @@ -39,8 +48,30 @@ class Loop } ); - $this->httpDownloader->wait(); + $this->currentPromises = $promises; + while (true) { + $hasActiveJob = false; + + if ($this->httpDownloader) { + if ($this->httpDownloader->hasActiveJob()) { + $hasActiveJob = true; + } + } + if ($this->processExecutor) { + if ($this->processExecutor->hasActiveJob()) { + $hasActiveJob = true; + } + } + + if (!$hasActiveJob) { + break; + } + + usleep(5000); + } + + $this->currentPromises = null; if ($uncaught) { throw $uncaught; } diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index a30a04d15..b443e541d 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -16,18 +16,32 @@ use Composer\IO\IOInterface; use Symfony\Component\Process\Process; use Symfony\Component\Process\ProcessUtils; use Symfony\Component\Process\Exception\RuntimeException; +use React\Promise\Promise; /** * @author Robert Schönthal + * @author Jordi Boggiano */ class ProcessExecutor { + const STATUS_QUEUED = 1; + const STATUS_STARTED = 2; + const STATUS_COMPLETED = 3; + const STATUS_FAILED = 4; + const STATUS_ABORTED = 5; + protected static $timeout = 300; protected $captureOutput; protected $errorOutput; protected $io; + private $jobs = array(); + private $runningJobs = 0; + private $maxJobs = 10; + private $idGen = 0; + private $allowAsync = false; + public function __construct(IOInterface $io = null) { $this->io = $io; @@ -112,6 +126,196 @@ class ProcessExecutor return $process->getExitCode(); } + /** + * starts a process on the commandline in async mode + * + * @param string $command the command to execute + * @param mixed $output the output will be written into this var if passed by ref + * if a callable is passed it will be used as output handler + * @param string $cwd the working directory + * @return int statuscode + */ + public function executeAsync($command, $cwd = null) + { + if (!$this->allowAsync) { + throw new \LogicException('You must use the ProcessExecutor instance which is part of a Composer\Loop instance to be able to run async processes'); + } + + $job = array( + 'id' => $this->idGen++, + 'status' => self::STATUS_QUEUED, + 'command' => $command, + 'cwd' => $cwd, + ); + + $resolver = function ($resolve, $reject) use (&$job) { + $job['status'] = ProcessExecutor::STATUS_QUEUED; + $job['resolve'] = $resolve; + $job['reject'] = $reject; + }; + + $self = $this; + $io = $this->io; + + $canceler = function () use (&$job) { + if ($job['status'] === self::STATUS_QUEUED) { + $job['status'] = self::STATUS_ABORTED; + } + if ($job['status'] !== self::STATUS_STARTED) { + return; + } + $job['status'] = self::STATUS_ABORTED; + try { + if (defined('SIGINT')) { + $job['process']->signal(SIGINT); + } + } catch (\Exception $e) { + // signal can throw in various conditions, but we don't care if it fails + } + $job['process']->stop(1); + }; + + $promise = new Promise($resolver, $canceler); + $promise = $promise->then(function () use (&$job, $self) { + if ($job['process']->isSuccessful()) { + $job['status'] = ProcessExecutor::STATUS_COMPLETED; + } else { + $job['status'] = ProcessExecutor::STATUS_FAILED; + } + + // TODO 3.0 this should be done directly on $this when PHP 5.3 is dropped + $self->markJobDone(); + + return $job['process']; + }, function () use (&$job, $self) { + $job['status'] = ProcessExecutor::STATUS_FAILED; + + $self->markJobDone(); + + return \React\Promise\reject($job['process']); + }); + $this->jobs[$job['id']] =& $job; + + if ($this->runningJobs < $this->maxJobs) { + $this->startJob($job['id']); + } + + return $promise; + } + + private function startJob($id) + { + $job =& $this->jobs[$id]; + if ($job['status'] !== self::STATUS_QUEUED) { + return; + } + + // start job + $job['status'] = self::STATUS_STARTED; + $this->runningJobs++; + + $command = $job['command']; + $cwd = $job['cwd']; + + if ($this->io && $this->io->isDebug()) { + $safeCommand = preg_replace_callback('{://(?P[^:/\s]+):(?P[^@\s/]+)@}i', function ($m) { + if (preg_match('{^[a-f0-9]{12,}$}', $m['user'])) { + return '://***:***@'; + } + + return '://'.$m['user'].':***@'; + }, $command); + $safeCommand = preg_replace("{--password (.*[^\\\\]\') }", '--password \'***\' ', $safeCommand); + $this->io->writeError('Executing async command ('.($cwd ?: 'CWD').'): '.$safeCommand); + } + + // make sure that null translate to the proper directory in case the dir is a symlink + // and we call a git command, because msysgit does not handle symlinks properly + if (null === $cwd && Platform::isWindows() && false !== strpos($command, 'git') && getcwd()) { + $cwd = realpath(getcwd()); + } + + // TODO in v3, commands should be passed in as arrays of cmd + args + if (method_exists('Symfony\Component\Process\Process', 'fromShellCommandline')) { + $process = Process::fromShellCommandline($command, $cwd, null, null, static::getTimeout()); + } else { + $process = new Process($command, $cwd, null, null, static::getTimeout()); + } + + $job['process'] = $process; + + $process->start(); + } + + public function wait($index = null) + { + while (true) { + if (!$this->hasActiveJob($index)) { + return; + } + + usleep(1000); + } + } + + /** + * @internal + */ + public function enableAsync() + { + $this->allowAsync = true; + } + + /** + * @internal + */ + public function hasActiveJob($index = null) + { + // tick + foreach ($this->jobs as &$job) { + if ($job['status'] === self::STATUS_STARTED) { + if (!$job['process']->isRunning()) { + call_user_func($job['resolve'], $job['process']); + } + } + } + + if (null !== $index) { + if ($this->jobs[$index]['status'] === self::STATUS_COMPLETED || $this->jobs[$index]['status'] === self::STATUS_FAILED || $this->jobs[$index]['status'] === self::STATUS_ABORTED) { + return false; + } + + return true; + } + + foreach ($this->jobs as $job) { + if (!in_array($job['status'], array(self::STATUS_COMPLETED, self::STATUS_FAILED, self::STATUS_ABORTED), true)) { + return true; + } else { + unset($this->jobs[$job['id']]); + } + } + + return false; + } + + /** + * @private + */ + public function markJobDone() + { + $this->runningJobs--; + + foreach ($this->jobs as $job) { + if ($job['status'] === self::STATUS_QUEUED) { + $this->startJob($job['id']); + if ($this->runningJobs >= $this->maxJobs) { + return; + } + } + } + } + public function splitLines($output) { $output = trim($output); diff --git a/tests/Composer/Test/Downloader/FileDownloaderTest.php b/tests/Composer/Test/Downloader/FileDownloaderTest.php index c86ffa2f7..ba8f95db9 100644 --- a/tests/Composer/Test/Downloader/FileDownloaderTest.php +++ b/tests/Composer/Test/Downloader/FileDownloaderTest.php @@ -139,8 +139,8 @@ class FileDownloaderTest extends TestCase ->will($this->returnValue($path.'/vendor')); try { - $promise = $downloader->download($packageMock, $path); $loop = new Loop($this->httpDownloader); + $promise = $downloader->download($packageMock, $path); $loop->wait(array($promise)); $this->fail('Download was expected to throw'); @@ -225,8 +225,8 @@ class FileDownloaderTest extends TestCase touch($dlFile); try { - $promise = $downloader->download($packageMock, $path); $loop = new Loop($this->httpDownloader); + $promise = $downloader->download($packageMock, $path); $loop->wait(array($promise)); $this->fail('Download was expected to throw'); @@ -296,8 +296,8 @@ class FileDownloaderTest extends TestCase mkdir(dirname($dlFile), 0777, true); touch($dlFile); - $promise = $downloader->download($newPackage, $path, $oldPackage); $loop = new Loop($this->httpDownloader); + $promise = $downloader->download($newPackage, $path, $oldPackage); $loop->wait(array($promise)); $downloader->update($oldPackage, $newPackage, $path); diff --git a/tests/Composer/Test/Downloader/XzDownloaderTest.php b/tests/Composer/Test/Downloader/XzDownloaderTest.php index f770b0d35..6996d67f6 100644 --- a/tests/Composer/Test/Downloader/XzDownloaderTest.php +++ b/tests/Composer/Test/Downloader/XzDownloaderTest.php @@ -70,8 +70,8 @@ class XzDownloaderTest extends TestCase $downloader = new XzDownloader($io, $config, $httpDownloader = new HttpDownloader($io, $this->getMockBuilder('Composer\Config')->getMock()), null, null, null); try { - $promise = $downloader->download($packageMock, $this->testDir.'/install-path'); $loop = new Loop($httpDownloader); + $promise = $downloader->download($packageMock, $this->testDir.'/install-path'); $loop->wait(array($promise)); $downloader->install($packageMock, $this->testDir.'/install-path'); diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index 4436c6ad7..764af8feb 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -92,8 +92,8 @@ class ZipDownloaderTest extends TestCase $this->setPrivateProperty('hasSystemUnzip', false); try { - $promise = $downloader->download($this->package, $path = sys_get_temp_dir().'/composer-zip-test'); $loop = new Loop($this->httpDownloader); + $promise = $downloader->download($this->package, $path = sys_get_temp_dir().'/composer-zip-test'); $loop->wait(array($promise)); $downloader->install($this->package, $path); diff --git a/tests/Composer/Test/Repository/ComposerRepositoryTest.php b/tests/Composer/Test/Repository/ComposerRepositoryTest.php index 4fcbbb431..01e3be4ce 100644 --- a/tests/Composer/Test/Repository/ComposerRepositoryTest.php +++ b/tests/Composer/Test/Repository/ComposerRepositoryTest.php @@ -189,16 +189,19 @@ class ComposerRepositoryTest extends TestCase ->getMock(); $httpDownloader->expects($this->at(0)) + ->method('enableAsync'); + + $httpDownloader->expects($this->at(1)) ->method('get') ->with($url = 'http://example.org/packages.json') ->willReturn(new \Composer\Util\Http\Response(array('url' => $url), 200, array(), json_encode(array('search' => '/search.json?q=%query%&type=%type%')))); - $httpDownloader->expects($this->at(1)) + $httpDownloader->expects($this->at(2)) ->method('get') ->with($url = 'http://example.org/search.json?q=foo&type=composer-plugin') ->willReturn(new \Composer\Util\Http\Response(array('url' => $url), 200, array(), json_encode($result))); - $httpDownloader->expects($this->at(2)) + $httpDownloader->expects($this->at(3)) ->method('get') ->with($url = 'http://example.org/search.json?q=foo&type=library') ->willReturn(new \Composer\Util\Http\Response(array('url' => $url), 200, array(), json_encode(array()))); @@ -291,6 +294,9 @@ class ComposerRepositoryTest extends TestCase ->getMock(); $httpDownloader->expects($this->at(0)) + ->method('enableAsync'); + + $httpDownloader->expects($this->at(1)) ->method('get') ->with($url = 'http://example.org/packages.json') ->willReturn(new \Composer\Util\Http\Response(array('url' => $url), 200, array(), json_encode(array( From 8f6e82f562ca2ba06c16d6325f079c386cdfdde0 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 09:06:49 +0200 Subject: [PATCH 03/11] Add support for aborting running promises cleanly --- .../Installer/InstallationManager.php | 2 ++ src/Composer/Util/Http/CurlDownloader.php | 34 ++++++++++++++++--- src/Composer/Util/HttpDownloader.php | 26 +++++++++----- src/Composer/Util/Loop.php | 9 +++++ src/Composer/Util/ProcessExecutor.php | 8 ++--- src/Composer/Util/RemoteFilesystem.php | 1 + 6 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index 06e8d65b9..9e55c4ac7 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -184,6 +184,8 @@ class InstallationManager $runCleanup = function () use (&$cleanupPromises, $loop) { $promises = array(); + $loop->abortJobs(); + foreach ($cleanupPromises as $cleanup) { $promises[] = new \React\Promise\Promise(function ($resolve, $reject) use ($cleanup) { $promise = $cleanup(); diff --git a/src/Composer/Util/Http/CurlDownloader.php b/src/Composer/Util/Http/CurlDownloader.php index 365caf899..257a29115 100644 --- a/src/Composer/Util/Http/CurlDownloader.php +++ b/src/Composer/Util/Http/CurlDownloader.php @@ -23,6 +23,7 @@ use Composer\Util\HttpDownloader; use React\Promise\Promise; /** + * @internal * @author Jordi Boggiano * @author Nicolas Grekas */ @@ -90,6 +91,9 @@ class CurlDownloader $this->authHelper = new AuthHelper($io, $config); } + /** + * @return int internal job id + */ public function download($resolve, $reject, $origin, $url, $options, $copyTo = null) { $attributes = array(); @@ -101,6 +105,9 @@ class CurlDownloader return $this->initDownload($resolve, $reject, $origin, $url, $options, $copyTo, $attributes); } + /** + * @return int internal job id + */ private function initDownload($resolve, $reject, $origin, $url, $options, $copyTo = null, array $attributes = array()) { $attributes = array_merge(array( @@ -199,8 +206,29 @@ class CurlDownloader } $this->checkCurlResult(curl_multi_add_handle($this->multiHandle, $curlHandle)); -// TODO progress + // TODO progress //$params['notification'](STREAM_NOTIFY_RESOLVE, STREAM_NOTIFY_SEVERITY_INFO, '', 0, 0, 0, false); + + return (int) $curlHandle; + } + + public function abortRequest($id) + { + if (isset($this->jobs[$id]) && isset($this->jobs[$id]['handle'])) { + $job = $this->jobs[$id]; + curl_multi_remove_handle($this->multiHandle, $job['handle']); + curl_close($job['handle']); + if (is_resource($job['headerHandle'])) { + fclose($job['headerHandle']); + } + if (is_resource($job['bodyHandle'])) { + fclose($job['bodyHandle']); + } + if ($job['filename']) { + @unlink($job['filename'].'~'); + } + unset($this->jobs[$id]); + } } public function tick() @@ -235,7 +263,7 @@ class CurlDownloader $statusCode = null; $response = null; try { -// TODO progress + // TODO progress //$this->onProgress($curlHandle, $job['callback'], $progress, $job['progress']); if (CURLE_OK !== $errno || $error) { throw new TransportException($error); @@ -285,8 +313,6 @@ class CurlDownloader // fail 4xx and 5xx responses and capture the response if ($statusCode >= 400 && $statusCode <= 599) { throw $this->failResponse($job, $response, $response->getStatusMessage()); -// TODO progress -// $this->io->overwriteError("Downloading (failed)", false); } if ($job['attributes']['storeAuth']) { diff --git a/src/Composer/Util/HttpDownloader.php b/src/Composer/Util/HttpDownloader.php index 41ced41e3..6fe53390e 100644 --- a/src/Composer/Util/HttpDownloader.php +++ b/src/Composer/Util/HttpDownloader.php @@ -31,6 +31,7 @@ class HttpDownloader const STATUS_STARTED = 2; const STATUS_COMPLETED = 3; const STATUS_FAILED = 4; + const STATUS_ABORTED = 5; private $io; private $config; @@ -184,8 +185,20 @@ class HttpDownloader $downloader = $this; $io = $this->io; + $curl = $this->curl; - $canceler = function () {}; + $canceler = function () use (&$job, $curl) { + if ($job['status'] === self::STATUS_QUEUED) { + $job['status'] = self::STATUS_ABORTED; + } + if ($job['status'] !== self::STATUS_STARTED) { + return; + } + $job['status'] = self::STATUS_ABORTED; + if (isset($job['curl_id'])) { + $curl->abortRequest($job['curl_id']); + } + }; $promise = new Promise($resolver, $canceler); $promise->then(function ($response) use (&$job, $downloader) { @@ -242,9 +255,9 @@ class HttpDownloader } if ($job['request']['copyTo']) { - $this->curl->download($resolve, $reject, $origin, $url, $options, $job['request']['copyTo']); + $job['curl_id'] = $this->curl->download($resolve, $reject, $origin, $url, $options, $job['request']['copyTo']); } else { - $this->curl->download($resolve, $reject, $origin, $url, $options); + $job['curl_id'] = $this->curl->download($resolve, $reject, $origin, $url, $options); } } @@ -294,14 +307,11 @@ class HttpDownloader } if (null !== $index) { - if ($this->jobs[$index]['status'] === self::STATUS_COMPLETED || $this->jobs[$index]['status'] === self::STATUS_FAILED) { - return false; - } - return true; + return $this->jobs[$index]['status'] < self::STATUS_COMPLETED; } foreach ($this->jobs as $job) { - if (!in_array($job['status'], array(self::STATUS_COMPLETED, self::STATUS_FAILED), true)) { + if ($job['status'] < self::STATUS_COMPLETED) { return true; } elseif (!$job['sync']) { unset($this->jobs[$job['id']]); diff --git a/src/Composer/Util/Loop.php b/src/Composer/Util/Loop.php index b0061ba2d..b7382f1ed 100644 --- a/src/Composer/Util/Loop.php +++ b/src/Composer/Util/Loop.php @@ -76,4 +76,13 @@ class Loop throw $uncaught; } } + + public function abortJobs() + { + if ($this->currentPromises) { + foreach ($this->currentPromises as $promise) { + $promise->cancel(); + } + } + } } diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index b443e541d..59db0c0c0 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -281,15 +281,11 @@ class ProcessExecutor } if (null !== $index) { - if ($this->jobs[$index]['status'] === self::STATUS_COMPLETED || $this->jobs[$index]['status'] === self::STATUS_FAILED || $this->jobs[$index]['status'] === self::STATUS_ABORTED) { - return false; - } - - return true; + return $this->jobs[$index]['status'] < self::STATUS_COMPLETED; } foreach ($this->jobs as $job) { - if (!in_array($job['status'], array(self::STATUS_COMPLETED, self::STATUS_FAILED, self::STATUS_ABORTED), true)) { + if ($job['status'] < self::STATUS_COMPLETED) { return true; } else { unset($this->jobs[$job['id']]); diff --git a/src/Composer/Util/RemoteFilesystem.php b/src/Composer/Util/RemoteFilesystem.php index b0dfbea94..4bac6a88f 100644 --- a/src/Composer/Util/RemoteFilesystem.php +++ b/src/Composer/Util/RemoteFilesystem.php @@ -20,6 +20,7 @@ use Composer\Util\HttpDownloader; use Composer\Util\Http\Response; /** + * @internal * @author François Pluchino * @author Jordi Boggiano * @author Nils Adermann From 3af617efe892647ca1d2cfa59fa5c23215b82ba3 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 09:07:40 +0200 Subject: [PATCH 04/11] Parallelize zip extraction using async unzip processes --- src/Composer/Downloader/ArchiveDownloader.php | 97 +++++++++++-------- src/Composer/Downloader/ZipDownloader.php | 72 ++++++++++---- .../Test/Downloader/ZipDownloaderTest.php | 73 ++++++++++---- 3 files changed, 166 insertions(+), 76 deletions(-) diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index 283d0103e..a9091dbe6 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/src/Composer/Downloader/ArchiveDownloader.php @@ -16,6 +16,7 @@ use Composer\Package\PackageInterface; use Symfony\Component\Finder\Finder; use Composer\IO\IOInterface; use Composer\Exception\IrrecoverableDownloadException; +use React\Promise\PromiseInterface; /** * Base downloader for archives @@ -60,26 +61,62 @@ abstract class ArchiveDownloader extends FileDownloader $temporaryDir = $this->config->get('vendor-dir').'/composer/'.substr(md5(uniqid('', true)), 0, 8); } while (is_dir($temporaryDir)); + $this->addCleanupPath($package, $temporaryDir); + + $this->filesystem->ensureDirectoryExists($temporaryDir); $fileName = $this->getFileName($package, $path); - try { - $this->filesystem->ensureDirectoryExists($temporaryDir); - try { - $this->extract($package, $fileName, $temporaryDir); - } catch (\Exception $e) { - // remove cache if the file was corrupted - parent::clearLastCacheWrite($package); - throw $e; - } + $filesystem = $this->filesystem; + $self = $this; - $this->filesystem->unlink($fileName); + $cleanup = function () use ($path, $filesystem, $temporaryDir, $package, $self) { + // remove cache if the file was corrupted + $self->clearLastCacheWrite($package); + + // clean up + $filesystem->removeDirectory($path); + $filesystem->removeDirectory($temporaryDir); + $self->removeCleanupPath($package, $temporaryDir); + }; + + $promise = null; + try { + $promise = $this->extract($package, $fileName, $temporaryDir); + } catch (\Exception $e) { + $cleanup(); + throw $e; + } + + if (!$promise instanceof PromiseInterface) { + $promise = \React\Promise\resolve(); + } + + return $promise->then(function () use ($self, $package, $filesystem, $fileName, $temporaryDir, $path) { + $filesystem->unlink($fileName); + + /** + * Returns the folder content, excluding dotfiles + * + * @param string $dir Directory + * @return \SplFileInfo[] + */ + $getFolderContent = function ($dir) { + $finder = Finder::create() + ->ignoreVCS(false) + ->ignoreDotFiles(false) + ->notName('.DS_Store') + ->depth(0) + ->in($dir); + + return iterator_to_array($finder); + }; $renameAsOne = false; - if (!file_exists($path) || ($this->filesystem->isDirEmpty($path) && $this->filesystem->removeDirectory($path))) { + if (!file_exists($path) || ($filesystem->isDirEmpty($path) && $filesystem->removeDirectory($path))) { $renameAsOne = true; } - $contentDir = $this->getFolderContent($temporaryDir); + $contentDir = $getFolderContent($temporaryDir); $singleDirAtTopLevel = 1 === count($contentDir) && is_dir(reset($contentDir)); if ($renameAsOne) { @@ -89,28 +126,27 @@ abstract class ArchiveDownloader extends FileDownloader } else { $extractedDir = $temporaryDir; } - $this->filesystem->rename($extractedDir, $path); + $filesystem->rename($extractedDir, $path); } else { // only one dir in the archive, extract its contents out of it if ($singleDirAtTopLevel) { - $contentDir = $this->getFolderContent((string) reset($contentDir)); + $contentDir = $getFolderContent((string) reset($contentDir)); } // move files back out of the temp dir foreach ($contentDir as $file) { $file = (string) $file; - $this->filesystem->rename($file, $path . '/' . basename($file)); + $filesystem->rename($file, $path . '/' . basename($file)); } } - $this->filesystem->removeDirectory($temporaryDir); - } catch (\Exception $e) { - // clean up - $this->filesystem->removeDirectory($path); - $this->filesystem->removeDirectory($temporaryDir); + $filesystem->removeDirectory($temporaryDir); + $self->removeCleanupPath($package, $temporaryDir); + }, function ($e) use ($cleanup) { + $cleanup(); throw $e; - } + }); } /** @@ -119,25 +155,8 @@ abstract class ArchiveDownloader extends FileDownloader * @param string $file Extracted file * @param string $path Directory * + * @return PromiseInterface|null * @throws \UnexpectedValueException If can not extract downloaded file to path */ abstract protected function extract(PackageInterface $package, $file, $path); - - /** - * Returns the folder content, excluding dotfiles - * - * @param string $dir Directory - * @return \SplFileInfo[] - */ - private function getFolderContent($dir) - { - $finder = Finder::create() - ->ignoreVCS(false) - ->ignoreDotFiles(false) - ->notName('.DS_Store') - ->depth(0) - ->in($dir); - - return iterator_to_array($finder); - } } diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index d0b4e8255..c5fd2b11b 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -86,9 +86,8 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file * @param bool $isLastChance If true it is called as a fallback and should throw an exception - * @return bool Success status */ - protected function extractWithSystemUnzip($file, $path, $isLastChance) + private function extractWithSystemUnzip(PackageInterface $package, $file, $path, $isLastChance, $async = false) { if (!self::$hasZipArchive) { // Force Exception throwing if the Other alternative is not available @@ -98,18 +97,47 @@ class ZipDownloader extends ArchiveDownloader if (!self::$hasSystemUnzip && !$isLastChance) { // This was call as the favorite extract way, but is not available // We switch to the alternative - return $this->extractWithZipArchive($file, $path, true); + return $this->extractWithZipArchive($package, $file, $path, true); + } + + // When called after a ZipArchive failed, perhaps there is some files to overwrite + $overwrite = $isLastChance ? '-o' : ''; + $command = 'unzip -qq '.$overwrite.' '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); + + if ($async) { + $self = $this; + $io = $this->io; + $tryFallback = function ($processError) use ($isLastChance, $io, $self, $file, $path, $package) { + if ($isLastChance) { + throw $processError; + } + + $io->writeError(' '.$processError->getMessage().''); + $io->writeError(' The archive may contain identical file names with different capitalization (which fails on case insensitive filesystems)'); + $io->writeError(' Unzip with unzip command failed, falling back to ZipArchive class'); + + return $self->extractWithZipArchive($package, $file, $path, true); + }; + + try { + $promise = $this->process->executeAsync($command); + + return $promise->then(function ($process) use ($tryFallback, $command, $package) { + if (!$process->isSuccessful()) { + return $tryFallback(new \RuntimeException('Failed to extract '.$package->getName().': ('.$process->getExitCode().') '.$command."\n\n".$process->getErrorOutput())); + } + }); + } catch (\Exception $e) { + return $tryFallback($e); + } catch (\Throwable $e) { + return $tryFallback($e); + } } $processError = null; - // When called after a ZipArchive failed, perhaps there is some files to overwrite - $overwrite = $isLastChance ? '-o' : ''; - - $command = 'unzip -qq '.$overwrite.' '.ProcessExecutor::escape($file).' -d '.ProcessExecutor::escape($path); - try { if (0 === $exitCode = $this->process->execute($command, $ignoredOutput)) { - return true; + return \React\Promise\resolve(); } $processError = new \RuntimeException('Failed to execute ('.$exitCode.') '.$command."\n\n".$this->process->getErrorOutput()); @@ -121,11 +149,11 @@ class ZipDownloader extends ArchiveDownloader throw $processError; } - $this->io->writeError(' '.$processError->getMessage()); + $this->io->writeError(' '.$processError->getMessage().''); $this->io->writeError(' The archive may contain identical file names with different capitalization (which fails on case insensitive filesystems)'); $this->io->writeError(' Unzip with unzip command failed, falling back to ZipArchive class'); - return $this->extractWithZipArchive($file, $path, true); + return $this->extractWithZipArchive($package, $file, $path, true); } /** @@ -134,9 +162,11 @@ class ZipDownloader extends ArchiveDownloader * @param string $file File to extract * @param string $path Path where to extract file * @param bool $isLastChance If true it is called as a fallback and should throw an exception - * @return bool Success status + * + * TODO v3 should make this private once we can drop PHP 5.3 support + * @protected */ - protected function extractWithZipArchive($file, $path, $isLastChance) + public function extractWithZipArchive(PackageInterface $package, $file, $path, $isLastChance) { if (!self::$hasSystemUnzip) { // Force Exception throwing if the Other alternative is not available @@ -146,7 +176,7 @@ class ZipDownloader extends ArchiveDownloader if (!self::$hasZipArchive && !$isLastChance) { // This was call as the favorite extract way, but is not available // We switch to the alternative - return $this->extractWithSystemUnzip($file, $path, true); + return $this->extractWithSystemUnzip($package, $file, $path, true); } $processError = null; @@ -159,7 +189,7 @@ class ZipDownloader extends ArchiveDownloader if (true === $extractResult) { $zipArchive->close(); - return true; + return \React\Promise\resolve(); } $processError = new \RuntimeException(rtrim("There was an error extracting the ZIP file, it is either corrupted or using an invalid format.\n")); @@ -170,16 +200,18 @@ class ZipDownloader extends ArchiveDownloader $processError = new \RuntimeException('The archive may contain identical file names with different capitalization (which fails on case insensitive filesystems): '.$e->getMessage(), 0, $e); } catch (\Exception $e) { $processError = $e; + } catch (\Throwable $e) { + $processError = $e; } if ($isLastChance) { throw $processError; } - $this->io->writeError(' '.$processError->getMessage()); + $this->io->writeError(' '.$processError->getMessage().''); $this->io->writeError(' Unzip with ZipArchive class failed, falling back to unzip command'); - return $this->extractWithSystemUnzip($file, $path, true); + return $this->extractWithSystemUnzip($package, $file, $path, true); } /** @@ -192,10 +224,10 @@ class ZipDownloader extends ArchiveDownloader { // Each extract calls its alternative if not available or fails if (self::$isWindows) { - $this->extractWithZipArchive($file, $path, false); - } else { - $this->extractWithSystemUnzip($file, $path, false); + return $this->extractWithZipArchive($package, $file, $path, false); } + + return $this->extractWithSystemUnzip($package, $file, $path, false, true); } /** diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index 764af8feb..d86d2cdf1 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -179,37 +179,65 @@ class ZipDownloaderTest extends TestCase /** * @expectedException \Exception - * @expectedExceptionMessage Failed to execute (1) unzip + * @expectedExceptionMessage Failed to extract : (1) unzip */ public function testSystemUnzipOnlyFailed() { - if (!class_exists('ZipArchive')) { - $this->markTestSkipped('zip extension missing'); - } - + $this->setPrivateProperty('isWindows', false); $this->setPrivateProperty('hasSystemUnzip', true); $this->setPrivateProperty('hasZipArchive', false); + + $procMock = $this->getMockBuilder('Symfony\Component\Process\Process')->disableOriginalConstructor()->getMock(); + $procMock->expects($this->any()) + ->method('getExitCode') + ->will($this->returnValue(1)); + $procMock->expects($this->any()) + ->method('isSuccessful') + ->will($this->returnValue(false)); + $procMock->expects($this->any()) + ->method('getErrorOutput') + ->will($this->returnValue('output')); + $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) - ->method('execute') - ->will($this->returnValue(1)); + ->method('executeAsync') + ->will($this->returnValue(\React\Promise\resolve($procMock))); $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $e = null; + $promise->then(function () { + // noop + }, function ($ex) use (&$e) { + $e = $ex; + }); + + if ($e) { + throw $e; + } } public function testSystemUnzipOnlyGood() { - if (!class_exists('ZipArchive')) { - $this->markTestSkipped('zip extension missing'); - } - + $this->setPrivateProperty('isWindows', false); $this->setPrivateProperty('hasSystemUnzip', true); $this->setPrivateProperty('hasZipArchive', false); + + $procMock = $this->getMockBuilder('Symfony\Component\Process\Process')->disableOriginalConstructor()->getMock(); + $procMock->expects($this->any()) + ->method('getExitCode') + ->will($this->returnValue(0)); + $procMock->expects($this->any()) + ->method('isSuccessful') + ->will($this->returnValue(true)); + $procMock->expects($this->any()) + ->method('getErrorOutput') + ->will($this->returnValue('output')); + $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) - ->method('execute') - ->will($this->returnValue(0)); + ->method('executeAsync') + ->will($this->returnValue(\React\Promise\resolve($procMock))); $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); @@ -225,10 +253,21 @@ class ZipDownloaderTest extends TestCase $this->setPrivateProperty('hasSystemUnzip', true); $this->setPrivateProperty('hasZipArchive', true); + $procMock = $this->getMockBuilder('Symfony\Component\Process\Process')->disableOriginalConstructor()->getMock(); + $procMock->expects($this->any()) + ->method('getExitCode') + ->will($this->returnValue(1)); + $procMock->expects($this->any()) + ->method('isSuccessful') + ->will($this->returnValue(false)); + $procMock->expects($this->any()) + ->method('getErrorOutput') + ->will($this->returnValue('output')); + $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) - ->method('execute') - ->will($this->returnValue(1)); + ->method('executeAsync') + ->will($this->returnValue(\React\Promise\resolve($procMock))); $zipArchive = $this->getMockBuilder('ZipArchive')->getMock(); $zipArchive->expects($this->at(0)) @@ -350,6 +389,6 @@ class MockedZipDownloader extends ZipDownloader public function extract(PackageInterface $package, $file, $path) { - parent::extract($package, $file, $path); + return parent::extract($package, $file, $path); } } From b1e15c77256dd5e05ceaef207dd5c6225061c4fa Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 09:39:11 +0200 Subject: [PATCH 05/11] Fix a couple async bugs --- src/Composer/Util/ProcessExecutor.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index 59db0c0c0..b59bced70 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -187,12 +187,12 @@ class ProcessExecutor $self->markJobDone(); return $job['process']; - }, function () use (&$job, $self) { + }, function ($e) use (&$job, $self) { $job['status'] = ProcessExecutor::STATUS_FAILED; $self->markJobDone(); - return \React\Promise\reject($job['process']); + throw $e; }); $this->jobs[$job['id']] =& $job; @@ -272,7 +272,7 @@ class ProcessExecutor public function hasActiveJob($index = null) { // tick - foreach ($this->jobs as &$job) { + foreach ($this->jobs as $job) { if ($job['status'] === self::STATUS_STARTED) { if (!$job['process']->isRunning()) { call_user_func($job['resolve'], $job['process']); From 9f380d606caa6901d10e5f3e1b917d605379cebd Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 10:01:48 +0200 Subject: [PATCH 06/11] Add basic progress bar capability while waiting for jobs to complete --- src/Composer/IO/ConsoleIO.php | 10 ++++++ .../Installer/InstallationManager.php | 10 +++++- src/Composer/Util/HttpDownloader.php | 30 ++++++++-------- src/Composer/Util/Loop.php | 34 +++++++++++++------ src/Composer/Util/ProcessExecutor.php | 28 +++++++-------- 5 files changed, 72 insertions(+), 40 deletions(-) diff --git a/src/Composer/IO/ConsoleIO.php b/src/Composer/IO/ConsoleIO.php index 925a528be..ebe38f26a 100644 --- a/src/Composer/IO/ConsoleIO.php +++ b/src/Composer/IO/ConsoleIO.php @@ -14,6 +14,7 @@ namespace Composer\IO; use Composer\Question\StrictConfirmationQuestion; use Symfony\Component\Console\Helper\HelperSet; +use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -253,6 +254,15 @@ class ConsoleIO extends BaseIO } } + /** + * @param int $max + * @return ProgressBar + */ + public function getProgressBar($max = 0) + { + return new ProgressBar($this->getErrorOutput(), $max); + } + /** * {@inheritDoc} */ diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index 9e55c4ac7..47b3e2914 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -13,6 +13,7 @@ namespace Composer\Installer; use Composer\IO\IOInterface; +use Composer\IO\ConsoleIO; use Composer\Package\PackageInterface; use Composer\Package\AliasPackage; use Composer\Repository\RepositoryInterface; @@ -330,7 +331,14 @@ class InstallationManager // execute all prepare => installs/updates/removes => cleanup steps if (!empty($promises)) { - $this->loop->wait($promises); + $progress = null; + if ($io instanceof ConsoleIO && !$io->isDebug()) { + $progress = $io->getProgressBar(); + } + $this->loop->wait($promises, $progress); + if ($progress) { + $progress->clear(); + } } } catch (\Exception $e) { $runCleanup(); diff --git a/src/Composer/Util/HttpDownloader.php b/src/Composer/Util/HttpDownloader.php index 6fe53390e..889fae07e 100644 --- a/src/Composer/Util/HttpDownloader.php +++ b/src/Composer/Util/HttpDownloader.php @@ -267,21 +267,12 @@ class HttpDownloader public function markJobDone() { $this->runningJobs--; - - foreach ($this->jobs as $job) { - if ($job['status'] === self::STATUS_QUEUED) { - $this->startJob($job['id']); - if ($this->runningJobs >= $this->maxJobs) { - return; - } - } - } } public function wait($index = null) { while (true) { - if (!$this->hasActiveJob($index)) { + if (!$this->countActiveJobs($index)) { return; } @@ -299,26 +290,37 @@ class HttpDownloader /** * @internal + * + * @return int number of active (queued or started) jobs */ - public function hasActiveJob($index = null) + public function countActiveJobs($index = null) { + if ($this->runningJobs < $this->maxJobs) { + foreach ($this->jobs as $job) { + if ($job['status'] === self::STATUS_QUEUED && $this->runningJobs < $this->maxJobs) { + $this->startJob($job['id']); + } + } + } + if ($this->curl) { $this->curl->tick(); } if (null !== $index) { - return $this->jobs[$index]['status'] < self::STATUS_COMPLETED; + return $this->jobs[$index]['status'] < self::STATUS_COMPLETED ? 1 : 0; } + $active = 0; foreach ($this->jobs as $job) { if ($job['status'] < self::STATUS_COMPLETED) { - return true; + $active++; } elseif (!$job['sync']) { unset($this->jobs[$job['id']]); } } - return false; + return $active; } private function getResponse($index) diff --git a/src/Composer/Util/Loop.php b/src/Composer/Util/Loop.php index b7382f1ed..00159d562 100644 --- a/src/Composer/Util/Loop.php +++ b/src/Composer/Util/Loop.php @@ -14,6 +14,7 @@ namespace Composer\Util; use Composer\Util\HttpDownloader; use React\Promise\Promise; +use Symfony\Component\Console\Helper\ProgressBar; /** * @author Jordi Boggiano @@ -36,7 +37,7 @@ class Loop } } - public function wait(array $promises) + public function wait(array $promises, ProgressBar $progress = null) { /** @var \Exception|null */ $uncaught = null; @@ -50,21 +51,32 @@ class Loop $this->currentPromises = $promises; - while (true) { - $hasActiveJob = false; - + if ($progress) { + $totalJobs = 0; if ($this->httpDownloader) { - if ($this->httpDownloader->hasActiveJob()) { - $hasActiveJob = true; - } + $totalJobs += $this->httpDownloader->countActiveJobs(); } if ($this->processExecutor) { - if ($this->processExecutor->hasActiveJob()) { - $hasActiveJob = true; - } + $totalJobs += $this->processExecutor->countActiveJobs(); + } + $progress->start($totalJobs); + } + + while (true) { + $activeJobs = 0; + + if ($this->httpDownloader) { + $activeJobs += $this->httpDownloader->countActiveJobs(); + } + if ($this->processExecutor) { + $activeJobs += $this->processExecutor->countActiveJobs(); } - if (!$hasActiveJob) { + if ($progress) { + $progress->setProgress($progress->getMaxSteps() - $activeJobs); + } + + if (!$activeJobs) { break; } diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index b59bced70..96b9235c8 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -250,7 +250,7 @@ class ProcessExecutor public function wait($index = null) { while (true) { - if (!$this->hasActiveJob($index)) { + if (!$this->countActiveJobs($index)) { return; } @@ -268,8 +268,10 @@ class ProcessExecutor /** * @internal + * + * @return int number of active (queued or started) jobs */ - public function hasActiveJob($index = null) + public function countActiveJobs($index = null) { // tick foreach ($this->jobs as $job) { @@ -278,21 +280,28 @@ class ProcessExecutor call_user_func($job['resolve'], $job['process']); } } + + if ($this->runningJobs < $this->maxJobs) { + if ($job['status'] === self::STATUS_QUEUED) { + $this->startJob($job['id']); + } + } } if (null !== $index) { - return $this->jobs[$index]['status'] < self::STATUS_COMPLETED; + return $this->jobs[$index]['status'] < self::STATUS_COMPLETED ? 1 : 0; } + $active = 0; foreach ($this->jobs as $job) { if ($job['status'] < self::STATUS_COMPLETED) { - return true; + $active++; } else { unset($this->jobs[$job['id']]); } } - return false; + return $active; } /** @@ -301,15 +310,6 @@ class ProcessExecutor public function markJobDone() { $this->runningJobs--; - - foreach ($this->jobs as $job) { - if ($job['status'] === self::STATUS_QUEUED) { - $this->startJob($job['id']); - if ($this->runningJobs >= $this->maxJobs) { - return; - } - } - } } public function splitLines($output) From 87a0fc5506687186b426f8e4386fd491d52ea5cb Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 11:11:31 +0200 Subject: [PATCH 07/11] Execute operations in batches to make sure plugins install in the expected order --- src/Composer/Downloader/ArchiveDownloader.php | 3 + .../Installer/InstallationManager.php | 147 ++++++++++-------- 2 files changed, 87 insertions(+), 63 deletions(-) diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index a9091dbe6..bcee49f9a 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/src/Composer/Downloader/ArchiveDownloader.php @@ -62,6 +62,7 @@ abstract class ArchiveDownloader extends FileDownloader } while (is_dir($temporaryDir)); $this->addCleanupPath($package, $temporaryDir); + $this->addCleanupPath($package, $path); $this->filesystem->ensureDirectoryExists($temporaryDir); $fileName = $this->getFileName($package, $path); @@ -77,6 +78,7 @@ abstract class ArchiveDownloader extends FileDownloader $filesystem->removeDirectory($path); $filesystem->removeDirectory($temporaryDir); $self->removeCleanupPath($package, $temporaryDir); + $self->removeCleanupPath($package, $path); }; $promise = null; @@ -142,6 +144,7 @@ abstract class ArchiveDownloader extends FileDownloader $filesystem->removeDirectory($temporaryDir); $self->removeCleanupPath($package, $temporaryDir); + $self->removeCleanupPath($package, $path); }, function ($e) use ($cleanup) { $cleanup(); diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index 47b3e2914..54f359eaa 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -272,73 +272,23 @@ class InstallationManager $this->loop->wait($promises); } - foreach ($operations as $index => $operation) { - $opType = $operation->getOperationType(); + // execute operations in batches to make sure every plugin is installed in the + // right order and activated before the packages depending on it are installed + while ($operations) { + $batch = array(); - // ignoring alias ops as they don't need to execute anything - if (!in_array($opType, array('update', 'install', 'uninstall'))) { - // output alias ops in debug verbosity as they have no output otherwise - if ($this->io->isDebug()) { - $this->io->writeError(' - ' . $operation->show(false)); + foreach ($operations as $index => $operation) { + unset($operations[$index]); + $batch[$index] = $operation; + if (in_array($operation->getOperationType(), array('update', 'install'), true)) { + $package = $operation->getOperationType() === 'update' ? $operation->getTargetPackage() : $operation->getPackage(); + if ($package->getType() === 'composer-plugin' || $package->getType() === 'composer-installer') { + break; + } } - $this->$opType($repo, $operation); - - continue; } - if ($opType === 'update') { - $package = $operation->getTargetPackage(); - $initialPackage = $operation->getInitialPackage(); - } else { - $package = $operation->getPackage(); - $initialPackage = null; - } - $installer = $this->getInstaller($package->getType()); - - $event = 'Composer\Installer\PackageEvents::PRE_PACKAGE_'.strtoupper($opType); - if (defined($event) && $runScripts && $this->eventDispatcher) { - $this->eventDispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); - } - - $dispatcher = $this->eventDispatcher; - $installManager = $this; - $loop = $this->loop; - $io = $this->io; - - $promise = $installer->prepare($opType, $package, $initialPackage); - if (!$promise instanceof PromiseInterface) { - $promise = \React\Promise\resolve(); - } - - $promise = $promise->then(function () use ($opType, $installManager, $repo, $operation) { - return $installManager->$opType($repo, $operation); - })->then($cleanupPromises[$index]) - ->then(function () use ($opType, $runScripts, $dispatcher, $installManager, $devMode, $repo, $operations, $operation) { - $repo->write($devMode, $installManager); - - $event = 'Composer\Installer\PackageEvents::POST_PACKAGE_'.strtoupper($opType); - if (defined($event) && $runScripts && $dispatcher) { - $dispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); - } - }, function ($e) use ($opType, $package, $io) { - $io->writeError(' ' . ucfirst($opType) .' of '.$package->getPrettyName().' failed'); - - throw $e; - }); - - $promises[] = $promise; - } - - // execute all prepare => installs/updates/removes => cleanup steps - if (!empty($promises)) { - $progress = null; - if ($io instanceof ConsoleIO && !$io->isDebug()) { - $progress = $io->getProgressBar(); - } - $this->loop->wait($promises, $progress); - if ($progress) { - $progress->clear(); - } + $this->executeBatch($repo, $batch, $cleanupPromises, $devMode, $runScripts); } } catch (\Exception $e) { $runCleanup(); @@ -366,6 +316,77 @@ class InstallationManager $repo->write($devMode, $this); } + private function executeBatch(RepositoryInterface $repo, array $operations, array $cleanupPromises, $devMode, $runScripts) + { + foreach ($operations as $index => $operation) { + $opType = $operation->getOperationType(); + + // ignoring alias ops as they don't need to execute anything + if (!in_array($opType, array('update', 'install', 'uninstall'))) { + // output alias ops in debug verbosity as they have no output otherwise + if ($this->io->isDebug()) { + $this->io->writeError(' - ' . $operation->show(false)); + } + $this->$opType($repo, $operation); + + continue; + } + + if ($opType === 'update') { + $package = $operation->getTargetPackage(); + $initialPackage = $operation->getInitialPackage(); + } else { + $package = $operation->getPackage(); + $initialPackage = null; + } + $installer = $this->getInstaller($package->getType()); + + $event = 'Composer\Installer\PackageEvents::PRE_PACKAGE_'.strtoupper($opType); + if (defined($event) && $runScripts && $this->eventDispatcher) { + $this->eventDispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); + } + + $dispatcher = $this->eventDispatcher; + $installManager = $this; + $io = $this->io; + + $promise = $installer->prepare($opType, $package, $initialPackage); + if (!$promise instanceof PromiseInterface) { + $promise = \React\Promise\resolve(); + } + + $promise = $promise->then(function () use ($opType, $installManager, $repo, $operation) { + return $installManager->$opType($repo, $operation); + })->then($cleanupPromises[$index]) + ->then(function () use ($opType, $runScripts, $dispatcher, $installManager, $devMode, $repo, $operations, $operation) { + $repo->write($devMode, $installManager); + + $event = 'Composer\Installer\PackageEvents::POST_PACKAGE_'.strtoupper($opType); + if (defined($event) && $runScripts && $dispatcher) { + $dispatcher->dispatchPackageEvent(constant($event), $devMode, $repo, $operations, $operation); + } + }, function ($e) use ($opType, $package, $io) { + $io->writeError(' ' . ucfirst($opType) .' of '.$package->getPrettyName().' failed'); + + throw $e; + }); + + $promises[] = $promise; + } + + // execute all prepare => installs/updates/removes => cleanup steps + if (!empty($promises)) { + $progress = null; + if ($io instanceof ConsoleIO && !$io->isDebug() && count($promises) > 1) { + $progress = $io->getProgressBar(); + } + $this->loop->wait($promises, $progress); + if ($progress) { + $progress->clear(); + } + } + } + /** * Executes install operation. * From 9c78eda7db409125145aa2b94944c7fca55aeae1 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 13:15:50 +0200 Subject: [PATCH 08/11] Fix FileDownloader::update impl to handle promises --- src/Composer/Downloader/ArchiveDownloader.php | 4 +-- src/Composer/Downloader/FileDownloader.php | 29 ++++++++++++++----- src/Composer/Downloader/GzipDownloader.php | 9 ------ src/Composer/Downloader/PathDownloader.php | 9 ------ src/Composer/Downloader/RarDownloader.php | 9 ------ src/Composer/Downloader/XzDownloader.php | 10 ------- src/Composer/Downloader/ZipDownloader.php | 8 ----- src/Composer/Factory.php | 6 ++-- 8 files changed, 26 insertions(+), 58 deletions(-) diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index bcee49f9a..d77d55738 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/src/Composer/Downloader/ArchiveDownloader.php @@ -29,14 +29,12 @@ abstract class ArchiveDownloader extends FileDownloader { public function download(PackageInterface $package, $path, PackageInterface $prevPackage = null, $output = true) { - $res = parent::download($package, $path, $prevPackage, $output); - // if not downgrading and the dir already exists it seems we have an inconsistent state in the vendor dir and the user should fix it if (!$prevPackage && is_dir($path) && !$this->filesystem->isDirEmpty($path)) { throw new IrrecoverableDownloadException('Expected empty path to extract '.$package.' into but directory exists: '.$path); } - return $res; + return parent::download($package, $path, $prevPackage, $output); } /** diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index f5674cf90..5f88cf140 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -27,7 +27,9 @@ use Composer\EventDispatcher\EventDispatcher; use Composer\Util\Filesystem; use Composer\Util\HttpDownloader; use Composer\Util\Url as UrlUtil; +use Composer\Util\ProcessExecutor; use Composer\Downloader\TransportException; +use React\Promise\PromiseInterface; /** * Base downloader for files @@ -51,6 +53,8 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface protected $cache; /** @var EventDispatcher */ protected $eventDispatcher; + /** @var ProcessExecutor */ + protected $process; /** * @private this is only public for php 5.3 support in closures */ @@ -67,14 +71,15 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface * @param Cache $cache Cache instance * @param Filesystem $filesystem The filesystem */ - public function __construct(IOInterface $io, Config $config, HttpDownloader $httpDownloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $filesystem = null) + public function __construct(IOInterface $io, Config $config, HttpDownloader $httpDownloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $filesystem = null, ProcessExecutor $process = null) { $this->io = $io; $this->config = $config; $this->eventDispatcher = $eventDispatcher; $this->httpDownloader = $httpDownloader; - $this->filesystem = $filesystem ?: new Filesystem(); $this->cache = $cache; + $this->process = $process ?: new ProcessExecutor($io); + $this->filesystem = $filesystem ?: new Filesystem($this->process); if ($this->cache && $this->cache->gcIsNecessary()) { $this->cache->gc($config->get('cache-files-ttl'), $config->get('cache-files-maxsize')); @@ -333,10 +338,19 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $actionName = VersionParser::isUpgrade($initial->getVersion(), $target->getVersion()) ? 'Upgrading' : 'Downgrading'; $this->io->writeError(" - " . $actionName . " " . $name . " (" . $from . " => " . $to . "): ", false); - $this->remove($initial, $path, false); - $this->install($target, $path, false); + $promise = $this->remove($initial, $path, false); + if (!$promise instanceof PromiseInterface) { + $promise = \React\Promise\resolve(); + } + $self = $this; + $io = $this->io; - $this->io->writeError(''); + return $promise->then(function () use ($self, $target, $path, $io) { + $promise = $self->install($target, $path, false); + $io->writeError(''); + + return $promise; + }); } /** @@ -410,9 +424,10 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $output = ''; try { - $res = $this->download($package, $targetDir.'_compare', null, false); + $this->download($package, $targetDir.'_compare', null, false); $this->httpDownloader->wait(); - $res = $this->install($package, $targetDir.'_compare', false); + $this->install($package, $targetDir.'_compare', false); + $this->process->wait(); $comparer = new Comparer(); $comparer->setSource($targetDir.'_compare'); diff --git a/src/Composer/Downloader/GzipDownloader.php b/src/Composer/Downloader/GzipDownloader.php index 0b12b4380..3ebff597a 100644 --- a/src/Composer/Downloader/GzipDownloader.php +++ b/src/Composer/Downloader/GzipDownloader.php @@ -29,15 +29,6 @@ use Composer\Util\Filesystem; */ class GzipDownloader extends ArchiveDownloader { - /** @var ProcessExecutor */ - protected $process; - - public function __construct(IOInterface $io, Config $config, HttpDownloader $downloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $fs = null, ProcessExecutor $process = null) - { - $this->process = $process ?: new ProcessExecutor($io); - parent::__construct($io, $config, $downloader, $eventDispatcher, $cache, $fs); - } - protected function extract(PackageInterface $package, $file, $path) { $filename = pathinfo(parse_url($package->getDistUrl(), PHP_URL_PATH), PATHINFO_FILENAME); diff --git a/src/Composer/Downloader/PathDownloader.php b/src/Composer/Downloader/PathDownloader.php index 51e9f7709..2cb74d641 100644 --- a/src/Composer/Downloader/PathDownloader.php +++ b/src/Composer/Downloader/PathDownloader.php @@ -39,15 +39,6 @@ class PathDownloader extends FileDownloader implements VcsCapableDownloaderInter const STRATEGY_SYMLINK = 10; const STRATEGY_MIRROR = 20; - /** @var ProcessExecutor */ - private $process; - - public function __construct(IOInterface $io, Config $config, HttpDownloader $downloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $fs = null, ProcessExecutor $process = null) - { - $this->process = $process ?: new ProcessExecutor($io); - parent::__construct($io, $config, $downloader, $eventDispatcher, $cache, $fs); - } - /** * {@inheritdoc} */ diff --git a/src/Composer/Downloader/RarDownloader.php b/src/Composer/Downloader/RarDownloader.php index 0ca15de1f..b0484b661 100644 --- a/src/Composer/Downloader/RarDownloader.php +++ b/src/Composer/Downloader/RarDownloader.php @@ -33,15 +33,6 @@ use RarArchive; */ class RarDownloader extends ArchiveDownloader { - /** @var ProcessExecutor */ - protected $process; - - public function __construct(IOInterface $io, Config $config, HttpDownloader $downloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $fs = null, ProcessExecutor $process = null) - { - $this->process = $process ?: new ProcessExecutor($io); - parent::__construct($io, $config, $downloader, $eventDispatcher, $cache, $fs); - } - protected function extract(PackageInterface $package, $file, $path) { $processError = null; diff --git a/src/Composer/Downloader/XzDownloader.php b/src/Composer/Downloader/XzDownloader.php index 34956d997..b043af333 100644 --- a/src/Composer/Downloader/XzDownloader.php +++ b/src/Composer/Downloader/XzDownloader.php @@ -29,16 +29,6 @@ use Composer\Util\Filesystem; */ class XzDownloader extends ArchiveDownloader { - /** @var ProcessExecutor */ - protected $process; - - public function __construct(IOInterface $io, Config $config, HttpDownloader $downloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $fs = null, ProcessExecutor $process = null) - { - $this->process = $process ?: new ProcessExecutor($io); - - parent::__construct($io, $config, $downloader, $eventDispatcher, $cache, $fs); - } - protected function extract(PackageInterface $package, $file, $path) { $command = 'tar -xJf ' . ProcessExecutor::escape($file) . ' -C ' . ProcessExecutor::escape($path); diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index c5fd2b11b..d891fb33b 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/src/Composer/Downloader/ZipDownloader.php @@ -34,17 +34,9 @@ class ZipDownloader extends ArchiveDownloader private static $hasZipArchive; private static $isWindows; - /** @var ProcessExecutor */ - protected $process; /** @var ZipArchive|null */ private $zipArchiveObject; - public function __construct(IOInterface $io, Config $config, HttpDownloader $downloader, EventDispatcher $eventDispatcher = null, Cache $cache = null, Filesystem $fs = null, ProcessExecutor $process = null) - { - $this->process = $process ?: new ProcessExecutor($io); - parent::__construct($io, $config, $downloader, $eventDispatcher, $cache, $fs); - } - /** * {@inheritDoc} */ diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index 5bc41ee6b..509fa62b0 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -495,11 +495,11 @@ class Factory $dm->setDownloader('perforce', new Downloader\PerforceDownloader($io, $config, $process, $fs)); $dm->setDownloader('zip', new Downloader\ZipDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); $dm->setDownloader('rar', new Downloader\RarDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); - $dm->setDownloader('tar', new Downloader\TarDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs)); + $dm->setDownloader('tar', new Downloader\TarDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); $dm->setDownloader('gzip', new Downloader\GzipDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); $dm->setDownloader('xz', new Downloader\XzDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); - $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs)); - $dm->setDownloader('file', new Downloader\FileDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs)); + $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); + $dm->setDownloader('file', new Downloader\FileDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); $dm->setDownloader('path', new Downloader\PathDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $fs, $process)); return $dm; From 085fe4e7e5c9b1c0122322d62e94bba2c7f0fe4f Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 13:43:21 +0200 Subject: [PATCH 09/11] Add --no-progress support and a few more fixes --- src/Composer/Command/ArchiveCommand.php | 6 +++-- src/Composer/Command/CreateProjectCommand.php | 9 +++++++- src/Composer/Command/InstallCommand.php | 2 ++ src/Composer/Command/RemoveCommand.php | 2 ++ src/Composer/Command/RequireCommand.php | 2 ++ src/Composer/Command/UpdateCommand.php | 2 ++ src/Composer/Installer.php | 2 +- .../Installer/InstallationManager.php | 22 +++++++++++++++---- 8 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Composer/Command/ArchiveCommand.php b/src/Composer/Command/ArchiveCommand.php index fd6d454f5..b521e4927 100644 --- a/src/Composer/Command/ArchiveCommand.php +++ b/src/Composer/Command/ArchiveCommand.php @@ -23,6 +23,7 @@ use Composer\Plugin\CommandEvent; use Composer\Plugin\PluginEvents; use Composer\Util\Filesystem; use Composer\Util\Loop; +use Composer\Util\ProcessExecutor; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -112,9 +113,10 @@ EOT $archiveManager = $composer->getArchiveManager(); } else { $factory = new Factory; + $process = new ProcessExecutor(); $httpDownloader = $factory->createHttpDownloader($io, $config); - $downloadManager = $factory->createDownloadManager($io, $config, $httpDownloader); - $archiveManager = $factory->createArchiveManager($config, $downloadManager, new Loop($httpDownloader)); + $downloadManager = $factory->createDownloadManager($io, $config, $httpDownloader, $process); + $archiveManager = $factory->createArchiveManager($config, $downloadManager, new Loop($httpDownloader, $process)); } if ($packageName) { diff --git a/src/Composer/Command/CreateProjectCommand.php b/src/Composer/Command/CreateProjectCommand.php index de6701ce4..27d9919d7 100644 --- a/src/Composer/Command/CreateProjectCommand.php +++ b/src/Composer/Command/CreateProjectCommand.php @@ -201,6 +201,8 @@ EOT // install dependencies of the created project if ($noInstall === false) { + $composer->getInstallationManager()->setOutputProgress(!$noProgress); + $installer = Installer::create($io, $composer); $installer->setPreferSource($preferSource) ->setPreferDist($preferDist) @@ -212,6 +214,10 @@ EOT ->setClassMapAuthoritative($config->get('classmap-authoritative')) ->setApcuAutoloader($config->get('apcu-autoloader')); + if (!$composer->getLocker()->isLocked()) { + $installer->setUpdate(true); + } + if ($disablePlugins) { $installer->disablePlugins(); } @@ -405,7 +411,8 @@ EOT ->setPreferDist($preferDist); $projectInstaller = new ProjectInstaller($directory, $dm, $fs); - $im = $factory->createInstallationManager(new Loop($httpDownloader), $io); + $im = $factory->createInstallationManager(new Loop($httpDownloader, $process), $io); + $im->setOutputProgress(!$noProgress); $im->addInstaller($projectInstaller); $im->execute(new InstalledFilesystemRepository(new JsonFile('php://memory')), array(new InstallOperation($package))); $im->notifyInstalls($io); diff --git a/src/Composer/Command/InstallCommand.php b/src/Composer/Command/InstallCommand.php index 93523cba7..f8294c47b 100644 --- a/src/Composer/Command/InstallCommand.php +++ b/src/Composer/Command/InstallCommand.php @@ -106,6 +106,8 @@ EOT $ignorePlatformReqs = $input->getOption('ignore-platform-reqs') ?: ($input->getOption('ignore-platform-req') ?: false); + $composer->getInstallationManager()->setOutputProgress(!$input->getOption('no-progress')); + $install ->setDryRun($input->getOption('dry-run')) ->setVerbose($input->getOption('verbose')) diff --git a/src/Composer/Command/RemoveCommand.php b/src/Composer/Command/RemoveCommand.php index e540056f8..53c040412 100644 --- a/src/Composer/Command/RemoveCommand.php +++ b/src/Composer/Command/RemoveCommand.php @@ -220,6 +220,8 @@ EOT $commandEvent = new CommandEvent(PluginEvents::COMMAND, 'remove', $input, $output); $composer->getEventDispatcher()->dispatch($commandEvent->getName(), $commandEvent); + $composer->getInstallationManager()->setOutputProgress(!$input->getOption('no-progress')); + $install = Installer::create($io, $composer); $updateDevMode = !$input->getOption('update-no-dev'); diff --git a/src/Composer/Command/RequireCommand.php b/src/Composer/Command/RequireCommand.php index d8ba23458..9525b9e77 100644 --- a/src/Composer/Command/RequireCommand.php +++ b/src/Composer/Command/RequireCommand.php @@ -286,6 +286,8 @@ EOT $commandEvent = new CommandEvent(PluginEvents::COMMAND, 'require', $input, $output); $composer->getEventDispatcher()->dispatch($commandEvent->getName(), $commandEvent); + $composer->getInstallationManager()->setOutputProgress(!$input->getOption('no-progress')); + $install = Installer::create($io, $composer); $ignorePlatformReqs = $input->getOption('ignore-platform-reqs') ?: ($input->getOption('ignore-platform-req') ?: false); diff --git a/src/Composer/Command/UpdateCommand.php b/src/Composer/Command/UpdateCommand.php index 9576b141f..809db532c 100644 --- a/src/Composer/Command/UpdateCommand.php +++ b/src/Composer/Command/UpdateCommand.php @@ -180,6 +180,8 @@ EOT $commandEvent = new CommandEvent(PluginEvents::COMMAND, 'update', $input, $output); $composer->getEventDispatcher()->dispatch($commandEvent->getName(), $commandEvent); + $composer->getInstallationManager()->setOutputProgress(!$input->getOption('no-progress')); + $install = Installer::create($io, $composer); $config = $composer->getConfig(); diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 8c508eaa0..40a041927 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -685,7 +685,7 @@ class Installer } if ($this->executeOperations) { - $this->installationManager->execute($localRepo, $localRepoTransaction->getOperations(), $this->devMode); + $this->installationManager->execute($localRepo, $localRepoTransaction->getOperations(), $this->devMode, $this->runScripts); } else { foreach ($localRepoTransaction->getOperations() as $operation) { // output op, but alias op only in debug verbosity diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index 54f359eaa..a9bbe7c3a 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -50,6 +50,8 @@ class InstallationManager private $io; /** @var EventDispatcher */ private $eventDispatcher; + /** @var bool */ + private $outputProgress; public function __construct(Loop $loop, IOInterface $io, EventDispatcher $eventDispatcher = null) { @@ -174,7 +176,7 @@ class InstallationManager * @param RepositoryInterface $repo repository in which to add/remove/update packages * @param OperationInterface[] $operations operations to execute * @param bool $devMode whether the install is being run in dev mode - * @param bool $operation whether to dispatch script events + * @param bool $runScripts whether to dispatch script events */ public function execute(RepositoryInterface $repo, array $operations, $devMode = true, $runScripts = true) { @@ -269,7 +271,14 @@ class InstallationManager // execute all downloads first if (!empty($promises)) { - $this->loop->wait($promises); + $progress = null; + if ($this->outputProgress && $this->io instanceof ConsoleIO && !$this->io->isDebug() && count($promises) > 1) { + $progress = $this->io->getProgressBar(); + } + $this->loop->wait($promises, $progress); + if ($progress) { + $progress->clear(); + } } // execute operations in batches to make sure every plugin is installed in the @@ -377,8 +386,8 @@ class InstallationManager // execute all prepare => installs/updates/removes => cleanup steps if (!empty($promises)) { $progress = null; - if ($io instanceof ConsoleIO && !$io->isDebug() && count($promises) > 1) { - $progress = $io->getProgressBar(); + if ($this->outputProgress && $this->io instanceof ConsoleIO && !$this->io->isDebug() && count($promises) > 1) { + $progress = $this->io->getProgressBar(); } $this->loop->wait($promises, $progress); if ($progress) { @@ -485,6 +494,11 @@ class InstallationManager return $installer->getInstallPath($package); } + public function setOutputProgress($outputProgress) + { + $this->outputProgress = $outputProgress; + } + public function notifyInstalls(IOInterface $io) { foreach ($this->notifiablePackages as $repoUrl => $packages) { From ee58f25c001332bb8aaeaa442d6ca232093d911d Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Fri, 5 Jun 2020 14:05:19 +0200 Subject: [PATCH 10/11] Fix ZipDownloaderTest --- .../Test/Downloader/ZipDownloaderTest.php | 74 ++++++++++++------- 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/tests/Composer/Test/Downloader/ZipDownloaderTest.php b/tests/Composer/Test/Downloader/ZipDownloaderTest.php index d86d2cdf1..dc0f55aec 100644 --- a/tests/Composer/Test/Downloader/ZipDownloaderTest.php +++ b/tests/Composer/Test/Downloader/ZipDownloaderTest.php @@ -60,9 +60,6 @@ class ZipDownloaderTest extends TestCase } } - /** - * @group only - */ public function testErrorMessages() { if (!class_exists('ZipArchive')) { @@ -125,7 +122,8 @@ class ZipDownloaderTest extends TestCase ->will($this->returnValue(false)); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } /** @@ -150,12 +148,10 @@ class ZipDownloaderTest extends TestCase ->will($this->throwException(new \ErrorException('Not a directory'))); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } - /** - * @group only - */ public function testZipArchiveOnlyGood() { if (!class_exists('ZipArchive')) { @@ -174,7 +170,8 @@ class ZipDownloaderTest extends TestCase ->will($this->returnValue(true)); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } /** @@ -205,16 +202,7 @@ class ZipDownloaderTest extends TestCase $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); - $e = null; - $promise->then(function () { - // noop - }, function ($ex) use (&$e) { - $e = $ex; - }); - - if ($e) { - throw $e; - } + $this->wait($promise); } public function testSystemUnzipOnlyGood() @@ -240,7 +228,8 @@ class ZipDownloaderTest extends TestCase ->will($this->returnValue(\React\Promise\resolve($procMock))); $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } public function testNonWindowsFallbackGood() @@ -279,7 +268,8 @@ class ZipDownloaderTest extends TestCase $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } /** @@ -296,10 +286,21 @@ class ZipDownloaderTest extends TestCase $this->setPrivateProperty('hasSystemUnzip', true); $this->setPrivateProperty('hasZipArchive', true); + $procMock = $this->getMockBuilder('Symfony\Component\Process\Process')->disableOriginalConstructor()->getMock(); + $procMock->expects($this->any()) + ->method('getExitCode') + ->will($this->returnValue(1)); + $procMock->expects($this->any()) + ->method('isSuccessful') + ->will($this->returnValue(false)); + $procMock->expects($this->any()) + ->method('getErrorOutput') + ->will($this->returnValue('output')); + $processExecutor = $this->getMockBuilder('Composer\Util\ProcessExecutor')->getMock(); $processExecutor->expects($this->at(0)) - ->method('execute') - ->will($this->returnValue(1)); + ->method('executeAsync') + ->will($this->returnValue(\React\Promise\resolve($procMock))); $zipArchive = $this->getMockBuilder('ZipArchive')->getMock(); $zipArchive->expects($this->at(0)) @@ -311,7 +312,8 @@ class ZipDownloaderTest extends TestCase $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } public function testWindowsFallbackGood() @@ -339,7 +341,8 @@ class ZipDownloaderTest extends TestCase $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); } /** @@ -371,7 +374,26 @@ class ZipDownloaderTest extends TestCase $downloader = new MockedZipDownloader($this->io, $this->config, $this->httpDownloader, null, null, null, $processExecutor); $this->setPrivateProperty('zipArchiveObject', $zipArchive, $downloader); - $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $promise = $downloader->extract($this->package, 'testfile.zip', 'vendor/dir'); + $this->wait($promise); + } + + private function wait($promise) + { + if (null === $promise) { + return; + } + + $e = null; + $promise->then(function () { + // noop + }, function ($ex) use (&$e) { + $e = $ex; + }); + + if ($e) { + throw $e; + } } } From aea074308c642e86377a2fa0fef7d0f97e5ebb26 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 16 Jun 2020 14:07:30 +0200 Subject: [PATCH 11/11] Update batching to install plugin deps before the plugin (alone an own batch) --- src/Composer/Downloader/ArchiveDownloader.php | 2 +- .../Installer/InstallationManager.php | 31 +++++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/Composer/Downloader/ArchiveDownloader.php b/src/Composer/Downloader/ArchiveDownloader.php index d77d55738..8f7f5ef83 100644 --- a/src/Composer/Downloader/ArchiveDownloader.php +++ b/src/Composer/Downloader/ArchiveDownloader.php @@ -95,7 +95,7 @@ abstract class ArchiveDownloader extends FileDownloader $filesystem->unlink($fileName); /** - * Returns the folder content, excluding dotfiles + * Returns the folder content, excluding .DS_Store * * @param string $dir Directory * @return \SplFileInfo[] diff --git a/src/Composer/Installer/InstallationManager.php b/src/Composer/Installer/InstallationManager.php index a9bbe7c3a..7dfb2ff3e 100644 --- a/src/Composer/Installer/InstallationManager.php +++ b/src/Composer/Installer/InstallationManager.php @@ -283,20 +283,31 @@ class InstallationManager // execute operations in batches to make sure every plugin is installed in the // right order and activated before the packages depending on it are installed - while ($operations) { - $batch = array(); - - foreach ($operations as $index => $operation) { - unset($operations[$index]); - $batch[$index] = $operation; - if (in_array($operation->getOperationType(), array('update', 'install'), true)) { - $package = $operation->getOperationType() === 'update' ? $operation->getTargetPackage() : $operation->getPackage(); - if ($package->getType() === 'composer-plugin' || $package->getType() === 'composer-installer') { - break; + $batches = array(); + $batch = array(); + foreach ($operations as $index => $operation) { + if (in_array($operation->getOperationType(), array('update', 'install'), true)) { + $package = $operation->getOperationType() === 'update' ? $operation->getTargetPackage() : $operation->getPackage(); + if ($package->getType() === 'composer-plugin' || $package->getType() === 'composer-installer') { + if ($batch) { + $batches[] = $batch; } + unset($operations[$index]); + $batches[] = array($index => $operation); + $batch = array(); + + continue; } } + unset($operations[$index]); + $batch[$index] = $operation; + } + if ($batch) { + $batches[] = $batch; + } + + foreach ($batches as $batch) { $this->executeBatch($repo, $batch, $cleanupPromises, $devMode, $runScripts); } } catch (\Exception $e) {