From bc7c93ae85ae524cee94b6dbb627eb156141515f Mon Sep 17 00:00:00 2001 From: Sandy Pleyte Date: Mon, 24 Feb 2014 13:34:50 +0100 Subject: [PATCH 01/12] Fix for #1966, use the preferred-install from the rootPackage config to install the dependencies. --- src/Composer/Command/CreateProjectCommand.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Composer/Command/CreateProjectCommand.php b/src/Composer/Command/CreateProjectCommand.php index 5aab270ce..1b28f0e28 100644 --- a/src/Composer/Command/CreateProjectCommand.php +++ b/src/Composer/Command/CreateProjectCommand.php @@ -161,6 +161,24 @@ EOT $composer->getEventDispatcher()->dispatchCommandEvent(ScriptEvents::POST_ROOT_PACKAGE_INSTALL, $installDevPackages); } + // Update preferSource / preferDist with preferred-install from the root package if both vars still + // have their default initial value (false) + $config = $composer->getConfig(); + if ($config->has('preferred-install') && $preferDist === false && $preferSource === false) { + switch ($config->get('preferred-install')) { + case 'source': + $preferSource = true; + break; + case 'dist': + $preferDist = true; + break; + case 'auto': + default: + // noop + break; + } + } + // install dependencies of the created project if ($noInstall === false) { $installer = Installer::create($io, $composer); From 9af5eaa57461a7a6b886edaa847af36202e626f5 Mon Sep 17 00:00:00 2001 From: Sandy Pleyte Date: Mon, 24 Feb 2014 15:27:41 +0100 Subject: [PATCH 02/12] Refactored the code with the switch statement. --- src/Composer/Command/CreateProjectCommand.php | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/src/Composer/Command/CreateProjectCommand.php b/src/Composer/Command/CreateProjectCommand.php index 1b28f0e28..bfdddf31e 100644 --- a/src/Composer/Command/CreateProjectCommand.php +++ b/src/Composer/Command/CreateProjectCommand.php @@ -102,18 +102,8 @@ EOT $preferSource = false; $preferDist = false; - switch ($config->get('preferred-install')) { - case 'source': - $preferSource = true; - break; - case 'dist': - $preferDist = true; - break; - case 'auto': - default: - // noop - break; - } + $this->updatePreferredOptions($config, $preferSource, $preferDist); + if ($input->getOption('prefer-source') || $input->getOption('prefer-dist')) { $preferSource = $input->getOption('prefer-source'); $preferDist = $input->getOption('prefer-dist'); @@ -143,7 +133,7 @@ EOT ); } - public function installProject(IOInterface $io, $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false, $noInstall = false) + public function installProject(IOInterface $io, Config $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false, $noInstall = false) { $oldCwd = getcwd(); @@ -163,20 +153,9 @@ EOT // Update preferSource / preferDist with preferred-install from the root package if both vars still // have their default initial value (false) - $config = $composer->getConfig(); - if ($config->has('preferred-install') && $preferDist === false && $preferSource === false) { - switch ($config->get('preferred-install')) { - case 'source': - $preferSource = true; - break; - case 'dist': - $preferDist = true; - break; - case 'auto': - default: - // noop - break; - } + $rootPackageConfig = $composer->getConfig(); + if ($rootPackageConfig->has('preferred-install') && $preferDist === false && $preferSource === false) { + $this->updatePreferredOptions($rootPackageConfig, $preferSource, $preferDist); } // install dependencies of the created project @@ -256,7 +235,7 @@ EOT return 0; } - protected function installRootPackage(IOInterface $io, $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false) + protected function installRootPackage(IOInterface $io, Config $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false) { if (null === $repositoryUrl) { $sourceRepo = new CompositeRepository(Factory::createDefaultRepositories($io, $config)); @@ -361,4 +340,28 @@ EOT { return new InstallationManager(); } + + + /** + * Updated preferSource or preferDist based on the preferredInstall config option + * @param Config $config + * @param boolean $preferSource + * @param boolean $preferDist + */ + protected function updatePreferredOptions(Config $config, &$preferSource, &$preferDist) + { + switch ($config->get('preferred-install')) { + case 'source': + $preferSource = true; + break; + case 'dist': + $preferDist = true; + break; + case 'auto': + default: + // noop + break; + } + + } } From ab8f67e8cf75dee3c784660d7467d797492bb021 Mon Sep 17 00:00:00 2001 From: Sandy Pleyte Date: Mon, 24 Feb 2014 16:20:10 +0100 Subject: [PATCH 03/12] Always use rootPackage config --- src/Composer/Command/CreateProjectCommand.php | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Composer/Command/CreateProjectCommand.php b/src/Composer/Command/CreateProjectCommand.php index bfdddf31e..8022fb5c3 100644 --- a/src/Composer/Command/CreateProjectCommand.php +++ b/src/Composer/Command/CreateProjectCommand.php @@ -102,12 +102,7 @@ EOT $preferSource = false; $preferDist = false; - $this->updatePreferredOptions($config, $preferSource, $preferDist); - - if ($input->getOption('prefer-source') || $input->getOption('prefer-dist')) { - $preferSource = $input->getOption('prefer-source'); - $preferDist = $input->getOption('prefer-dist'); - } + $this->updatePreferredOptions($config, $input, $preferSource, $preferDist); if ($input->getOption('no-custom-installers')) { $output->writeln('You are using the deprecated option "no-custom-installers". Use "no-plugins" instead.'); @@ -129,11 +124,12 @@ EOT $input->getOption('no-scripts'), $input->getOption('keep-vcs'), $input->getOption('no-progress'), - $input->getOption('no-install') + $input->getOption('no-install'), + $input ); } - public function installProject(IOInterface $io, Config $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false, $noInstall = false) + public function installProject(IOInterface $io, Config $config, $packageName, $directory = null, $packageVersion = null, $stability = 'stable', $preferSource = false, $preferDist = false, $installDevPackages = false, $repositoryUrl = null, $disablePlugins = false, $noScripts = false, $keepVcs = false, $noProgress = false, $noInstall = false, InputInterface $input) { $oldCwd = getcwd(); @@ -154,9 +150,7 @@ EOT // Update preferSource / preferDist with preferred-install from the root package if both vars still // have their default initial value (false) $rootPackageConfig = $composer->getConfig(); - if ($rootPackageConfig->has('preferred-install') && $preferDist === false && $preferSource === false) { - $this->updatePreferredOptions($rootPackageConfig, $preferSource, $preferDist); - } + $this->updatePreferredOptions($rootPackageConfig, $input, $preferSource, $preferDist); // install dependencies of the created project if ($noInstall === false) { @@ -345,16 +339,19 @@ EOT /** * Updated preferSource or preferDist based on the preferredInstall config option * @param Config $config + * @param InputInterface $input * @param boolean $preferSource * @param boolean $preferDist */ - protected function updatePreferredOptions(Config $config, &$preferSource, &$preferDist) + protected function updatePreferredOptions(Config $config, InputInterface $input, &$preferSource, &$preferDist) { switch ($config->get('preferred-install')) { case 'source': $preferSource = true; + $preferDist = false; break; case 'dist': + $preferSource = false; $preferDist = true; break; case 'auto': @@ -363,5 +360,9 @@ EOT break; } + if ($input->getOption('prefer-source') || $input->getOption('prefer-dist')) { + $preferSource = $input->getOption('prefer-source'); + $preferDist = $input->getOption('prefer-dist'); + } } } From ee62ec60f0aa6f42aaac3587f46e2baf5046a036 Mon Sep 17 00:00:00 2001 From: Sandy Pleyte Date: Mon, 24 Feb 2014 16:22:44 +0100 Subject: [PATCH 04/12] Remove old comment --- src/Composer/Command/CreateProjectCommand.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Composer/Command/CreateProjectCommand.php b/src/Composer/Command/CreateProjectCommand.php index 8022fb5c3..87369dcaf 100644 --- a/src/Composer/Command/CreateProjectCommand.php +++ b/src/Composer/Command/CreateProjectCommand.php @@ -147,8 +147,6 @@ EOT $composer->getEventDispatcher()->dispatchCommandEvent(ScriptEvents::POST_ROOT_PACKAGE_INSTALL, $installDevPackages); } - // Update preferSource / preferDist with preferred-install from the root package if both vars still - // have their default initial value (false) $rootPackageConfig = $composer->getConfig(); $this->updatePreferredOptions($rootPackageConfig, $input, $preferSource, $preferDist); From 5ed18d9aa2d08931cb9b4a61cb8beab388011e8d Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 18:40:33 +0100 Subject: [PATCH 05/12] Fail over from source to dist and vice versa when downloads fail Any RuntimeException descendent will be caught and cause another download attempt using either source or dist depending on what was attempted first. --- src/Composer/Downloader/DownloadManager.php | 47 ++++++-- src/Composer/Factory.php | 2 +- tests/Composer/Test/ComposerTest.php | 3 +- .../Test/Downloader/DownloadManagerTest.php | 104 ++++++++++++++---- tests/Composer/Test/InstallerTest.php | 2 +- 5 files changed, 123 insertions(+), 35 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index 51649cc3a..39a16611e 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -14,6 +14,7 @@ namespace Composer\Downloader; use Composer\Package\PackageInterface; use Composer\Downloader\DownloaderInterface; +use Composer\IO\IOInterface; use Composer\Util\Filesystem; /** @@ -23,6 +24,7 @@ use Composer\Util\Filesystem; */ class DownloadManager { + private $io; private $preferDist = false; private $preferSource = false; private $filesystem; @@ -31,11 +33,13 @@ class DownloadManager /** * Initializes download manager. * + * @param IOInterface $io The Input Output Interface * @param bool $preferSource prefer downloading from source * @param Filesystem|null $filesystem custom Filesystem object */ - public function __construct($preferSource = false, Filesystem $filesystem = null) + public function __construct(IOInterface $io, $preferSource = false, Filesystem $filesystem = null) { + $this->io = $io; $this->preferSource = $preferSource; $this->filesystem = $filesystem ?: new Filesystem(); } @@ -168,19 +172,44 @@ class DownloadManager $sourceType = $package->getSourceType(); $distType = $package->getDistType(); - if ((!$package->isDev() || $this->preferDist || !$sourceType) && !($preferSource && $sourceType) && $distType) { - $package->setInstallationSource('dist'); - } elseif ($sourceType) { - $package->setInstallationSource('source'); - } else { + $wantDist = !$package->isDev() || $this->preferDist || !$sourceType; + $wantSource = $preferSource && $sourceType; + + $types = array(); + if ($sourceType) { + $types[] = 'source'; + } + if ($distType) { + $types[] = 'dist'; + } + + if (empty($types)) { throw new \InvalidArgumentException('Package '.$package.' must have a source or dist specified'); } + if ($wantDist && !$wantSource) { + $types = array_reverse($types); + } + $this->filesystem->ensureDirectoryExists($targetDir); - $downloader = $this->getDownloaderForInstalledPackage($package); - if ($downloader) { - $downloader->download($package, $targetDir); + foreach ($types as $source) { + $package->setInstallationSource($source); + try { + $downloader = $this->getDownloaderForInstalledPackage($package); + if ($downloader) { + $downloader->download($package, $targetDir); + } + break; + } catch (\RuntimeException $e) { + $this->io->write( + 'Caught an exception while trying to download '. + $package->getPrettyString(). + ': '. + $e->getMessage().'' + ); + continue; + } } } diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index f829cb459..27f83dd87 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -344,7 +344,7 @@ class Factory $cache = new Cache($io, $config->get('cache-files-dir'), 'a-z0-9_./'); } - $dm = new Downloader\DownloadManager(); + $dm = new Downloader\DownloadManager($io); switch ($config->get('preferred-install')) { case 'dist': $dm->setPreferDist(true); diff --git a/tests/Composer/Test/ComposerTest.php b/tests/Composer/Test/ComposerTest.php index f667e88e5..df00c36f7 100644 --- a/tests/Composer/Test/ComposerTest.php +++ b/tests/Composer/Test/ComposerTest.php @@ -47,7 +47,8 @@ class ComposerTest extends TestCase public function testSetGetDownloadManager() { $composer = new Composer(); - $manager = $this->getMock('Composer\Downloader\DownloadManager'); + $io = $this->getMock('Composer\IO\IOInterface'); + $manager = $this->getMock('Composer\Downloader\DownloadManager', array(), array($io)); $composer->setDownloadManager($manager); $this->assertSame($manager, $composer->getDownloadManager()); diff --git a/tests/Composer/Test/Downloader/DownloadManagerTest.php b/tests/Composer/Test/Downloader/DownloadManagerTest.php index 48242a818..1728c583e 100644 --- a/tests/Composer/Test/Downloader/DownloadManagerTest.php +++ b/tests/Composer/Test/Downloader/DownloadManagerTest.php @@ -17,16 +17,18 @@ use Composer\Downloader\DownloadManager; class DownloadManagerTest extends \PHPUnit_Framework_TestCase { protected $filesystem; + protected $io; public function setUp() { $this->filesystem = $this->getMock('Composer\Util\Filesystem'); + $this->io = $this->getMock('Composer\IO\IOInterface'); } public function testSetGetDownloader() { $downloader = $this->createDownloaderMock(); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $manager->setDownloader('test', $downloader); $this->assertSame($downloader, $manager->getDownloader('test')); @@ -43,7 +45,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getInstallationSource') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->setExpectedException('InvalidArgumentException'); @@ -69,7 +71,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('dist')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -101,7 +103,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('source')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -135,7 +137,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('source')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -167,7 +169,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('dist')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -190,7 +192,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getType') ->will($this->returnValue('metapackage')); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->assertNull($manager->getDownloaderForInstalledPackage($package)); } @@ -219,7 +221,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -231,6 +233,62 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager->download($package, 'target_dir'); } + public function testFullPackageDownloadFailover() + { + $package = $this->createPackageMock(); + $package + ->expects($this->once()) + ->method('getSourceType') + ->will($this->returnValue('git')); + $package + ->expects($this->once()) + ->method('getDistType') + ->will($this->returnValue('pear')); + $package + ->expects($this->any()) + ->method('getPrettyString') + ->will($this->returnValue('prettyPackage')); + + $package + ->expects($this->at(3)) + ->method('setInstallationSource') + ->with('dist'); + $package + ->expects($this->at(5)) + ->method('setInstallationSource') + ->with('source'); + + $downloaderFail = $this->createDownloaderMock(); + $downloaderFail + ->expects($this->once()) + ->method('download') + ->with($package, 'target_dir') + ->will($this->throwException(new \RuntimeException("Foo"))); + + $downloaderSuccess = $this->createDownloaderMock(); + $downloaderSuccess + ->expects($this->once()) + ->method('download') + ->with($package, 'target_dir'); + + $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') + ->setConstructorArgs(array($this->io, false, $this->filesystem)) + ->setMethods(array('getDownloaderForInstalledPackage')) + ->getMock(); + $manager + ->expects($this->at(0)) + ->method('getDownloaderForInstalledPackage') + ->with($package) + ->will($this->returnValue($downloaderFail)); + $manager + ->expects($this->at(1)) + ->method('getDownloaderForInstalledPackage') + ->with($package) + ->will($this->returnValue($downloaderSuccess)); + + $manager->download($package, 'target_dir'); + } + public function testBadPackageDownload() { $package = $this->createPackageMock(); @@ -243,7 +301,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistType') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->setExpectedException('InvalidArgumentException'); $manager->download($package, 'target_dir'); @@ -273,7 +331,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -309,7 +367,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -339,7 +397,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with('source'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -375,7 +433,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -412,7 +470,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -449,7 +507,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -474,7 +532,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistType') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $manager->setPreferSource(true); $this->setExpectedException('InvalidArgumentException'); @@ -510,7 +568,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, $target, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -547,7 +605,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -588,7 +646,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, $target, 'vendor/pkg'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -625,7 +683,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, 'vendor/pkg'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -647,7 +705,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $target = $this->createPackageMock(); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -670,7 +728,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -687,7 +745,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $package = $this->createPackageMock(); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 4e992c577..cc17973bc 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -51,7 +51,7 @@ class InstallerTest extends TestCase { $io = $this->getMock('Composer\IO\IOInterface'); - $downloadManager = $this->getMock('Composer\Downloader\DownloadManager'); + $downloadManager = $this->getMock('Composer\Downloader\DownloadManager', array(), array($io)); $config = $this->getMock('Composer\Config'); $repositoryManager = new RepositoryManager($io, $config); From 35fbe3fd42b7b87e3a8b225d0631bdf64358cbf6 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 18:53:34 +0100 Subject: [PATCH 06/12] Download failover means we can now always try github zip urls for dist --- src/Composer/Repository/Vcs/GitHubDriver.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Composer/Repository/Vcs/GitHubDriver.php b/src/Composer/Repository/Vcs/GitHubDriver.php index 3cf2befb5..1bcbf8a3e 100644 --- a/src/Composer/Repository/Vcs/GitHubDriver.php +++ b/src/Composer/Repository/Vcs/GitHubDriver.php @@ -117,10 +117,6 @@ class GitHubDriver extends VcsDriver */ public function getDist($identifier) { - if ($this->gitDriver) { - return $this->gitDriver->getDist($identifier); - } - $url = $this->getApiUrl() . '/repos/'.$this->owner.'/'.$this->repository.'/zipball/'.$identifier; return array('type' => 'zip', 'url' => $url, 'reference' => $identifier, 'shasum' => ''); From a80fde97d525ac0747f0352a507b86a0c06e4a4f Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 19:22:32 +0100 Subject: [PATCH 07/12] Make the github driver behave like git if "no-api" is specified. --- doc/05-repositories.md | 5 ++++ src/Composer/Repository/Vcs/GitHubDriver.php | 26 ++++++++++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/doc/05-repositories.md b/doc/05-repositories.md index fdb7ce1a2..a4e92d7ed 100644 --- a/doc/05-repositories.md +++ b/doc/05-repositories.md @@ -292,6 +292,11 @@ The VCS driver to be used is detected automatically based on the URL. However, should you need to specify one for whatever reason, you can use `git`, `svn` or `hg` as the repository type instead of `vcs`. +If you set the `no-api` key to `true` on a github repository it will clone the +repository as it would with any other git repository instead of using the +GitHub API. But unlike using the `git` driver directly, composer will still +attempt to use github's zip files. + #### Subversion Options Since Subversion has no native concept of branches and tags, Composer assumes diff --git a/src/Composer/Repository/Vcs/GitHubDriver.php b/src/Composer/Repository/Vcs/GitHubDriver.php index 3cf2befb5..67ce43551 100644 --- a/src/Composer/Repository/Vcs/GitHubDriver.php +++ b/src/Composer/Repository/Vcs/GitHubDriver.php @@ -52,6 +52,11 @@ class GitHubDriver extends VcsDriver $this->originUrl = !empty($match[1]) ? $match[1] : $match[2]; $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.$this->originUrl.'/'.$this->owner.'/'.$this->repository); + if (isset($this->repoConfig['no-api']) && $this->repoConfig['no-api']) { + $this->setupGitDriver(); + return; + } + $this->fetchRootIdentifier(); } @@ -405,14 +410,7 @@ class GitHubDriver extends VcsDriver // GitHub returns 404 for private repositories) and we // cannot ask for authentication credentials (because we // are not interactive) then we fallback to GitDriver. - $this->gitDriver = new GitDriver( - array('url' => $this->generateSshUrl()), - $this->io, - $this->config, - $this->process, - $this->remoteFilesystem - ); - $this->gitDriver->initialize(); + $this->setupGitDriver(); return; } catch (\RuntimeException $e) { @@ -422,4 +420,16 @@ class GitHubDriver extends VcsDriver throw $e; } } + + protected function setupGitDriver() + { + $this->gitDriver = new GitDriver( + array('url' => $this->generateSshUrl()), + $this->io, + $this->config, + $this->process, + $this->remoteFilesystem + ); + $this->gitDriver->initialize(); + } } From 1ccf4b0fc337aaecd5b34c6400c16161bfd5b849 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 19:51:03 +0100 Subject: [PATCH 08/12] Correct the tests for dist urls for github --- .../Composer/Test/Repository/Vcs/GitHubDriverTest.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php index a3cb9dd23..906c20071 100644 --- a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php @@ -283,19 +283,16 @@ class GitHubDriverTest extends \PHPUnit_Framework_TestCase $this->assertEquals('test_master', $gitHubDriver->getRootIdentifier()); - // Dist is not available for GitDriver - $dist = $gitHubDriver->getDist($identifier); - $this->assertNull($dist); + $dist = $gitHubDriver->getDist($sha); + $this->assertEquals('zip', $dist['type']); + $this->assertEquals('https://api.github.com/repos/composer/packagist/zipball/SOMESHA', $dist['url']); + $this->assertEquals($sha, $dist['reference']); $source = $gitHubDriver->getSource($identifier); $this->assertEquals('git', $source['type']); $this->assertEquals($repoSshUrl, $source['url']); $this->assertEquals($identifier, $source['reference']); - // Dist is not available for GitDriver - $dist = $gitHubDriver->getDist($sha); - $this->assertNull($dist); - $source = $gitHubDriver->getSource($sha); $this->assertEquals('git', $source['type']); $this->assertEquals($repoSshUrl, $source['url']); From 31fd6c233c29cc520ead2b01a7ba4b92a601ef56 Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 19:52:20 +0100 Subject: [PATCH 09/12] Rethrow download exceptions when no options left & clean up code --- src/Composer/Downloader/DownloadManager.php | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index 39a16611e..ab35d488f 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -172,28 +172,25 @@ class DownloadManager $sourceType = $package->getSourceType(); $distType = $package->getDistType(); - $wantDist = !$package->isDev() || $this->preferDist || !$sourceType; - $wantSource = $preferSource && $sourceType; - - $types = array(); + $sources = array(); if ($sourceType) { - $types[] = 'source'; + $sources[] = 'source'; } if ($distType) { - $types[] = 'dist'; + $sources[] = 'dist'; } - if (empty($types)) { + if (empty($sources)) { throw new \InvalidArgumentException('Package '.$package.' must have a source or dist specified'); } - if ($wantDist && !$wantSource) { - $types = array_reverse($types); + if ((!$package->isDev() || $this->preferDist) && !$preferSource) { + $sources = array_reverse($sources); } $this->filesystem->ensureDirectoryExists($targetDir); - foreach ($types as $source) { + foreach ($sources as $i => $source) { $package->setInstallationSource($source); try { $downloader = $this->getDownloaderForInstalledPackage($package); @@ -202,13 +199,16 @@ class DownloadManager } break; } catch (\RuntimeException $e) { + if ($i == count($sources) - 1) { + throw $e; + } + $this->io->write( 'Caught an exception while trying to download '. $package->getPrettyString(). ': '. $e->getMessage().'' ); - continue; } } } From 665a2bd0c0d2a0a6e2f57bc9b2d518b592cf2cc5 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Tue, 25 Feb 2014 13:34:39 +0100 Subject: [PATCH 10/12] Tweak error message and make TransportException extend from RuntimeException, refs #2753 --- src/Composer/Downloader/DownloadManager.php | 9 ++++++--- src/Composer/Downloader/TransportException.php | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index ab35d488f..f2296d7d7 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -191,6 +191,9 @@ class DownloadManager $this->filesystem->ensureDirectoryExists($targetDir); foreach ($sources as $i => $source) { + if (isset($e)) { + $this->io->write('Now trying to download from ' . $source . ''); + } $package->setInstallationSource($source); try { $downloader = $this->getDownloaderForInstalledPackage($package); @@ -204,9 +207,9 @@ class DownloadManager } $this->io->write( - 'Caught an exception while trying to download '. - $package->getPrettyString(). - ': '. + 'Failed to download '. + $package->getPrettyName(). + ' from ' . $source . ': '. $e->getMessage().'' ); } diff --git a/src/Composer/Downloader/TransportException.php b/src/Composer/Downloader/TransportException.php index d157dde3c..b28f8d470 100644 --- a/src/Composer/Downloader/TransportException.php +++ b/src/Composer/Downloader/TransportException.php @@ -15,7 +15,7 @@ namespace Composer\Downloader; /** * @author Jordi Boggiano */ -class TransportException extends \Exception +class TransportException extends \RuntimeException { protected $headers; From edfaf727e57b1616579c99a26b4f2edaa3ff0d5a Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 25 Feb 2014 15:55:44 +0100 Subject: [PATCH 11/12] When using the github driver with no-api don't reset to an ssh url --- src/Composer/Repository/Vcs/GitHubDriver.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Composer/Repository/Vcs/GitHubDriver.php b/src/Composer/Repository/Vcs/GitHubDriver.php index 60f343ab2..c99c8cf18 100644 --- a/src/Composer/Repository/Vcs/GitHubDriver.php +++ b/src/Composer/Repository/Vcs/GitHubDriver.php @@ -53,7 +53,7 @@ class GitHubDriver extends VcsDriver $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.$this->originUrl.'/'.$this->owner.'/'.$this->repository); if (isset($this->repoConfig['no-api']) && $this->repoConfig['no-api']) { - $this->setupGitDriver(); + $this->setupGitDriver($this->getUrl()); return; } @@ -406,7 +406,7 @@ class GitHubDriver extends VcsDriver // GitHub returns 404 for private repositories) and we // cannot ask for authentication credentials (because we // are not interactive) then we fallback to GitDriver. - $this->setupGitDriver(); + $this->setupGitDriver($this->generateSshUrl()); return; } catch (\RuntimeException $e) { @@ -417,10 +417,10 @@ class GitHubDriver extends VcsDriver } } - protected function setupGitDriver() + protected function setupGitDriver($url) { $this->gitDriver = new GitDriver( - array('url' => $this->generateSshUrl()), + array('url' => $url), $this->io, $this->config, $this->process, From b808ff5e28944ab2e25e050f8df848c5842bc14c Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Tue, 25 Feb 2014 15:57:35 +0100 Subject: [PATCH 12/12] Don't hardcode the URL to an https one either --- src/Composer/Repository/Vcs/GitHubDriver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Repository/Vcs/GitHubDriver.php b/src/Composer/Repository/Vcs/GitHubDriver.php index c99c8cf18..38c7869a5 100644 --- a/src/Composer/Repository/Vcs/GitHubDriver.php +++ b/src/Composer/Repository/Vcs/GitHubDriver.php @@ -53,7 +53,7 @@ class GitHubDriver extends VcsDriver $this->cache = new Cache($this->io, $this->config->get('cache-repo-dir').'/'.$this->originUrl.'/'.$this->owner.'/'.$this->repository); if (isset($this->repoConfig['no-api']) && $this->repoConfig['no-api']) { - $this->setupGitDriver($this->getUrl()); + $this->setupGitDriver($this->url); return; }