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); + $filesystem = $this->filesystem; + $self = $this; + + $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 { - $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; - } + $promise = $this->extract($package, $fileName, $temporaryDir); + } catch (\Exception $e) { + $cleanup(); + throw $e; + } + + if (!$promise instanceof PromiseInterface) { + $promise = \React\Promise\resolve(); + } - $this->filesystem->unlink($fileName); + 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); } - $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); + 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; 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); } }