From 895d901bf9c27534d34ab438b0f2f2a1cf75a42e Mon Sep 17 00:00:00 2001 From: mikey179 Date: Tue, 21 Feb 2012 00:02:34 +0100 Subject: [PATCH 1/3] better error handling when git command runs into a failure, fixes #340 --- src/Composer/Downloader/GitDownloader.php | 16 +++++++++++++--- .../Test/Downloader/GitDownloaderTest.php | 12 ++++++++---- 2 files changed, 21 insertions(+), 7 deletions(-) mode change 100644 => 100755 src/Composer/Downloader/GitDownloader.php mode change 100644 => 100755 tests/Composer/Test/Downloader/GitDownloaderTest.php diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php old mode 100644 new mode 100755 index 170749674..d5c413dd3 --- a/src/Composer/Downloader/GitDownloader.php +++ b/src/Composer/Downloader/GitDownloader.php @@ -29,7 +29,10 @@ class GitDownloader extends VcsDownloader $ref = escapeshellarg($package->getSourceReference()); $path = escapeshellarg($path); $this->io->write(" Cloning ".$package->getSourceReference()); - $this->process->execute(sprintf('git clone %s %s && cd %2$s && git checkout %3$s && git reset --hard %3$s', $url, $path, $ref), $ignoredOutput); + $command = sprintf('git clone %s %s && cd %2$s && git checkout %3$s && git reset --hard %3$s', $url, $path, $ref); + if (0 !== $this->process->execute($command, $ignoredOutput)) { + throw new \RuntimeException('Failed to execute ' . $command); + } } /** @@ -40,7 +43,10 @@ class GitDownloader extends VcsDownloader $ref = escapeshellarg($target->getSourceReference()); $path = escapeshellarg($path); $this->io->write(" Checking out ".$target->getSourceReference()); - $this->process->execute(sprintf('cd %s && git fetch && git checkout %2$s && git reset --hard %2$s', $path, $ref), $ignoredOutput); + $command = sprintf('cd %s && git fetch && git checkout %2$s && git reset --hard %2$s', $path, $ref); + if (0 !== $this->process->execute($command, $ignoredOutput)) { + throw new \RuntimeException('Failed to execute ' . $command); + } } /** @@ -48,7 +54,11 @@ class GitDownloader extends VcsDownloader */ protected function enforceCleanDirectory($path) { - $this->process->execute(sprintf('cd %s && git status --porcelain', escapeshellarg($path)), $output); + $command = sprintf('cd %s && git status --porcelain', escapeshellarg($path)); + if (0 !== $this->process->execute($command, $output)) { + throw new \RuntimeException('Failed to execute ' . $command); + } + if (trim($output)) { throw new \RuntimeException('Source directory has uncommitted changes'); } diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php old mode 100644 new mode 100755 index ff1c3ac07..525d2d0e9 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -43,7 +43,8 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->once()) ->method('execute') - ->with($this->equalTo($expectedGitCommand)); + ->with($this->equalTo($expectedGitCommand)) + ->will($this->returnValue(0)); $downloader = new GitDownloader($this->getMock('Composer\IO\IOInterface'), $processExecutor); $downloader->download($packageMock, 'composerPath'); @@ -79,10 +80,12 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->at(0)) ->method('execute') - ->with($this->equalTo($expectedGitResetCommand)); + ->with($this->equalTo($expectedGitResetCommand)) + ->will($this->returnValue(0)); $processExecutor->expects($this->at(1)) ->method('execute') - ->with($this->equalTo($expectedGitUpdateCommand)); + ->with($this->equalTo($expectedGitUpdateCommand)) + ->will($this->returnValue(0)); $downloader = new GitDownloader($this->getMock('Composer\IO\IOInterface'), $processExecutor); $downloader->update($packageMock, $packageMock, 'composerPath'); @@ -96,7 +99,8 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $processExecutor->expects($this->any()) ->method('execute') - ->with($this->equalTo($expectedGitResetCommand)); + ->with($this->equalTo($expectedGitResetCommand)) + ->will($this->returnValue(0)); $filesystem = $this->getMock('Composer\Util\Filesystem'); $filesystem->expects($this->any()) ->method('removeDirectory') From 0e5a4e07baa7630d4e02b01c681c9df42db69500 Mon Sep 17 00:00:00 2001 From: mikey179 Date: Tue, 21 Feb 2012 00:12:39 +0100 Subject: [PATCH 2/3] add tests to make sure a runtime exception is thrown if return code from git command line call is not 0 --- .../Test/Downloader/GitDownloaderTest.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php index 525d2d0e9..885a80d35 100755 --- a/tests/Composer/Test/Downloader/GitDownloaderTest.php +++ b/tests/Composer/Test/Downloader/GitDownloaderTest.php @@ -50,6 +50,29 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase $downloader->download($packageMock, 'composerPath'); } + /** + * @expectedException \RuntimeException + */ + public function testDownloadThrowsRuntimeExceptionIfGitCommandFails() + { + $expectedGitCommand = $this->getCmd('git clone \'https://github.com/l3l0/composer\' \'composerPath\' && cd \'composerPath\' && git checkout \'ref\' && git reset --hard \'ref\''); + $packageMock = $this->getMock('Composer\Package\PackageInterface'); + $packageMock->expects($this->any()) + ->method('getSourceReference') + ->will($this->returnValue('ref')); + $packageMock->expects($this->once()) + ->method('getSourceUrl') + ->will($this->returnValue('https://github.com/l3l0/composer')); + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->once()) + ->method('execute') + ->with($this->equalTo($expectedGitCommand)) + ->will($this->returnValue(1)); + + $downloader = new GitDownloader($this->getMock('Composer\IO\IOInterface'), $processExecutor); + $downloader->download($packageMock, 'composerPath'); + } + /** * @expectedException \InvalidArgumentException */ @@ -91,6 +114,35 @@ class GitDownloaderTest extends \PHPUnit_Framework_TestCase $downloader->update($packageMock, $packageMock, 'composerPath'); } + /** + * @expectedException \RuntimeException + */ + public function testUpdateThrowsRuntimeExceptionIfGitCommandFails() + { + $expectedGitUpdateCommand = $this->getCmd('cd \'composerPath\' && git fetch && git checkout \'ref\' && git reset --hard \'ref\''); + $expectedGitResetCommand = $this->getCmd('cd \'composerPath\' && git status --porcelain'); + + $packageMock = $this->getMock('Composer\Package\PackageInterface'); + $packageMock->expects($this->any()) + ->method('getSourceReference') + ->will($this->returnValue('ref')); + $packageMock->expects($this->any()) + ->method('getSourceUrl') + ->will($this->returnValue('https://github.com/l3l0/composer')); + $processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); + $processExecutor->expects($this->at(0)) + ->method('execute') + ->with($this->equalTo($expectedGitResetCommand)) + ->will($this->returnValue(0)); + $processExecutor->expects($this->at(1)) + ->method('execute') + ->with($this->equalTo($expectedGitUpdateCommand)) + ->will($this->returnValue(1)); + + $downloader = new GitDownloader($this->getMock('Composer\IO\IOInterface'), $processExecutor); + $downloader->update($packageMock, $packageMock, 'composerPath'); + } + public function testRemove() { $expectedGitResetCommand = $this->getCmd('cd \'composerPath\' && git status --porcelain'); From 025f6066ff8faa3e950ef948617e81a4834713f8 Mon Sep 17 00:00:00 2001 From: Frank Kleine Date: Tue, 21 Feb 2012 00:40:43 +0100 Subject: [PATCH 3/3] fix file permissions --- src/Composer/Downloader/GitDownloader.php | 0 tests/Composer/Test/Downloader/GitDownloaderTest.php | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 src/Composer/Downloader/GitDownloader.php mode change 100755 => 100644 tests/Composer/Test/Downloader/GitDownloaderTest.php diff --git a/src/Composer/Downloader/GitDownloader.php b/src/Composer/Downloader/GitDownloader.php old mode 100755 new mode 100644 diff --git a/tests/Composer/Test/Downloader/GitDownloaderTest.php b/tests/Composer/Test/Downloader/GitDownloaderTest.php old mode 100755 new mode 100644