From b333d7a4856853c224c4dd15efed146333c577c6 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 04:31:24 +0200 Subject: [PATCH 1/9] act on target-dir changes during update --- src/Composer/Installer/LibraryInstaller.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index 281ffa11a..a754eb590 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -157,8 +157,12 @@ class LibraryInstaller implements InstallerInterface protected function updateCode(PackageInterface $initial, PackageInterface $target) { - $downloadPath = $this->getInstallPath($initial); - $this->downloadManager->update($initial, $target, $downloadPath); + $initialDownloadPath = $this->getInstallPath($initial); + $targetDownloadPath = $this->getInstallPath($target); + if ($targetDownloadPath != $initialDownloadPath) { + $this->filesystem->copyThenRemove($initialDownloadPath, $targetDownloadPath); + } + $this->downloadManager->update($initial, $target, $targetDownloadPath); } protected function removeCode(PackageInterface $package) From dd4db91ae79ed4ce713afa7bac93bf5b69634940 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 05:02:06 +0200 Subject: [PATCH 2/9] using mkdir() in copyThenRemove() leads to errors if the target exists or not a dir, use ensureDirectoryExists() instead --- src/Composer/Util/Filesystem.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Composer/Util/Filesystem.php b/src/Composer/Util/Filesystem.php index 3dade2ce9..bdef6e402 100644 --- a/src/Composer/Util/Filesystem.php +++ b/src/Composer/Util/Filesystem.php @@ -129,15 +129,12 @@ class Filesystem { $it = new RecursiveDirectoryIterator($source, RecursiveDirectoryIterator::SKIP_DOTS); $ri = new RecursiveIteratorIterator($it, RecursiveIteratorIterator::SELF_FIRST); - - if (!file_exists($target)) { - mkdir($target, 0777, true); - } + $this->ensureDirectoryExists($target); foreach ($ri as $file) { $targetPath = $target . DIRECTORY_SEPARATOR . $ri->getSubPathName(); if ($file->isDir()) { - mkdir($targetPath); + $this->ensureDirectoryExists($targetPath); } else { copy($file->getPathname(), $targetPath); } From f82c820a32e468c20fa8061989814c51f1407762 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 05:39:35 +0200 Subject: [PATCH 3/9] do not try to test update inside fixtures --- .../Test/Plugin/PluginInstallerTest.php | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/Composer/Test/Plugin/PluginInstallerTest.php b/tests/Composer/Test/Plugin/PluginInstallerTest.php index 3b64f2699..4163c0f2e 100644 --- a/tests/Composer/Test/Plugin/PluginInstallerTest.php +++ b/tests/Composer/Test/Plugin/PluginInstallerTest.php @@ -20,6 +20,7 @@ use Composer\Package\Loader\ArrayLoader; use Composer\Package\PackageInterface; use Composer\Plugin\PluginManager; use Composer\Autoload\AutoloadGenerator; +use Composer\Util\Filesystem; class PluginInstallerTest extends \PHPUnit_Framework_TestCase { @@ -30,13 +31,17 @@ class PluginInstallerTest extends \PHPUnit_Framework_TestCase protected $repository; protected $io; protected $autoloadGenerator; + protected $directory; protected function setUp() { $loader = new JsonLoader(new ArrayLoader()); $this->packages = array(); + $this->directory = sys_get_temp_dir() . '/' . uniqid(); for ($i = 1; $i <= 4; $i++) { - $this->packages[] = $loader->load(__DIR__.'/Fixtures/plugin-v'.$i.'/composer.json'); + $filename = '/Fixtures/plugin-v'.$i.'/composer.json'; + mkdir(dirname($this->directory . $filename), 0777, TRUE); + $this->packages[] = $loader->load(__DIR__ . $filename); } $dm = $this->getMockBuilder('Composer\Downloader\DownloadManager') @@ -77,13 +82,18 @@ class PluginInstallerTest extends \PHPUnit_Framework_TestCase $config->merge(array( 'config' => array( - 'vendor-dir' => __DIR__.'/Fixtures/', - 'home' => __DIR__.'/Fixtures', - 'bin-dir' => __DIR__.'/Fixtures/bin', + 'vendor-dir' => $this->directory.'/Fixtures/', + 'home' => $this->directory.'/Fixtures', + 'bin-dir' => $this->directory.'/Fixtures/bin', ), )); } + protected function tearDown() { + $filesystem = new Filesystem(); + $filesystem->removeDirectoryPhp($this->directory); + } + public function testInstallNewPlugin() { $this->repository From c6ec739766c53245edd9f18b8d31f91abd04d8d3 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 06:02:36 +0200 Subject: [PATCH 4/9] allow injecting a mock filesystem into LibraryInstaller and fix LibraryInstallerTest --- src/Composer/Installer/LibraryInstaller.php | 5 +++-- tests/Composer/Test/Installer/LibraryInstallerTest.php | 8 +++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index a754eb590..cd3a6d947 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -41,15 +41,16 @@ class LibraryInstaller implements InstallerInterface * @param IOInterface $io * @param Composer $composer * @param string $type + * @param Filesystem $filesystem */ - public function __construct(IOInterface $io, Composer $composer, $type = 'library') + public function __construct(IOInterface $io, Composer $composer, $type = 'library', $filesystem = NULL) { $this->composer = $composer; $this->downloadManager = $composer->getDownloadManager(); $this->io = $io; $this->type = $type; - $this->filesystem = new Filesystem(); + $this->filesystem = $filesystem ?: new Filesystem(); $this->vendorDir = rtrim($composer->getConfig()->get('vendor-dir'), '/'); $this->binDir = rtrim($composer->getConfig()->get('bin-dir'), '/'); } diff --git a/tests/Composer/Test/Installer/LibraryInstallerTest.php b/tests/Composer/Test/Installer/LibraryInstallerTest.php index dd36e6184..92e0437d6 100644 --- a/tests/Composer/Test/Installer/LibraryInstallerTest.php +++ b/tests/Composer/Test/Installer/LibraryInstallerTest.php @@ -131,7 +131,8 @@ class LibraryInstallerTest extends TestCase */ public function testUpdate() { - $library = new LibraryInstaller($this->io, $this->composer); + $filesystem = $this->getMockBuilder('Composer\Util\Filesystem')->getMock(); + $library = new LibraryInstaller($this->io, $this->composer, 'library', $filesystem); $initial = $this->createPackageMock(); $target = $this->createPackageMock(); @@ -140,6 +141,11 @@ class LibraryInstallerTest extends TestCase ->method('getPrettyName') ->will($this->returnValue('package1')); + $target + ->expects($this->once()) + ->method('getPrettyName') + ->will($this->returnValue('package1')); + $this->repository ->expects($this->exactly(3)) ->method('hasPackage') From 24c9ef72d60f4080b1d16af92a60b8929e9cf1b2 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 06:04:15 +0200 Subject: [PATCH 5/9] make LibraryInstallerTest a little more strict --- tests/Composer/Test/Installer/LibraryInstallerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Composer/Test/Installer/LibraryInstallerTest.php b/tests/Composer/Test/Installer/LibraryInstallerTest.php index 92e0437d6..f87a8c9c9 100644 --- a/tests/Composer/Test/Installer/LibraryInstallerTest.php +++ b/tests/Composer/Test/Installer/LibraryInstallerTest.php @@ -137,7 +137,7 @@ class LibraryInstallerTest extends TestCase $target = $this->createPackageMock(); $initial - ->expects($this->any()) + ->expects($this->once()) ->method('getPrettyName') ->will($this->returnValue('package1')); From 6c393c1c696ddd56d5ecbeec0e6387adc7d18c31 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 06:31:06 +0200 Subject: [PATCH 6/9] use the more generic removeDirectory --- tests/Composer/Test/Plugin/PluginInstallerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Composer/Test/Plugin/PluginInstallerTest.php b/tests/Composer/Test/Plugin/PluginInstallerTest.php index 4163c0f2e..79e141530 100644 --- a/tests/Composer/Test/Plugin/PluginInstallerTest.php +++ b/tests/Composer/Test/Plugin/PluginInstallerTest.php @@ -91,7 +91,7 @@ class PluginInstallerTest extends \PHPUnit_Framework_TestCase protected function tearDown() { $filesystem = new Filesystem(); - $filesystem->removeDirectoryPhp($this->directory); + $filesystem->removeDirectory($this->directory); } public function testInstallNewPlugin() From f4e9c74fee1155001683441cc4d960deb9b99fac Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Fri, 20 Sep 2013 09:58:46 +0200 Subject: [PATCH 7/9] style fixes --- src/Composer/Installer/LibraryInstaller.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index cd3a6d947..64b300c35 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -43,7 +43,7 @@ class LibraryInstaller implements InstallerInterface * @param string $type * @param Filesystem $filesystem */ - public function __construct(IOInterface $io, Composer $composer, $type = 'library', $filesystem = NULL) + public function __construct(IOInterface $io, Composer $composer, $type = 'library', $filesystem = null) { $this->composer = $composer; $this->downloadManager = $composer->getDownloadManager(); @@ -160,7 +160,7 @@ class LibraryInstaller implements InstallerInterface { $initialDownloadPath = $this->getInstallPath($initial); $targetDownloadPath = $this->getInstallPath($target); - if ($targetDownloadPath != $initialDownloadPath) { + if ($targetDownloadPath !== $initialDownloadPath) { $this->filesystem->copyThenRemove($initialDownloadPath, $targetDownloadPath); } $this->downloadManager->update($initial, $target, $targetDownloadPath); From 1a69d0a2a757e7d8a07f883e0487ee3d5f8575ec Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Sun, 22 Sep 2013 19:41:54 +0200 Subject: [PATCH 8/9] style fixes --- src/Composer/Installer/LibraryInstaller.php | 2 +- tests/Composer/Test/Plugin/PluginInstallerTest.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Composer/Installer/LibraryInstaller.php b/src/Composer/Installer/LibraryInstaller.php index 64b300c35..cb3808295 100644 --- a/src/Composer/Installer/LibraryInstaller.php +++ b/src/Composer/Installer/LibraryInstaller.php @@ -43,7 +43,7 @@ class LibraryInstaller implements InstallerInterface * @param string $type * @param Filesystem $filesystem */ - public function __construct(IOInterface $io, Composer $composer, $type = 'library', $filesystem = null) + public function __construct(IOInterface $io, Composer $composer, $type = 'library', Filesystem $filesystem = null) { $this->composer = $composer; $this->downloadManager = $composer->getDownloadManager(); diff --git a/tests/Composer/Test/Plugin/PluginInstallerTest.php b/tests/Composer/Test/Plugin/PluginInstallerTest.php index 79e141530..2502dded9 100644 --- a/tests/Composer/Test/Plugin/PluginInstallerTest.php +++ b/tests/Composer/Test/Plugin/PluginInstallerTest.php @@ -89,7 +89,8 @@ class PluginInstallerTest extends \PHPUnit_Framework_TestCase )); } - protected function tearDown() { + protected function tearDown() + { $filesystem = new Filesystem(); $filesystem->removeDirectory($this->directory); } From e32e4ad490b6326e5b71f6982c0dd6879f4be129 Mon Sep 17 00:00:00 2001 From: Karoly Negyesi Date: Sun, 22 Sep 2013 19:42:05 +0200 Subject: [PATCH 9/9] change the test to test for a target dir change --- .../Test/Installer/LibraryInstallerTest.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/Composer/Test/Installer/LibraryInstallerTest.php b/tests/Composer/Test/Installer/LibraryInstallerTest.php index f87a8c9c9..fcebc8328 100644 --- a/tests/Composer/Test/Installer/LibraryInstallerTest.php +++ b/tests/Composer/Test/Installer/LibraryInstallerTest.php @@ -131,8 +131,13 @@ class LibraryInstallerTest extends TestCase */ public function testUpdate() { - $filesystem = $this->getMockBuilder('Composer\Util\Filesystem')->getMock(); - $library = new LibraryInstaller($this->io, $this->composer, 'library', $filesystem); + $filesystem = $this->getMockBuilder('Composer\Util\Filesystem') + ->getMock(); + $filesystem + ->expects($this->once()) + ->method('copyThenRemove') + ->with($this->vendorDir.'/package1', $this->vendorDir.'/package1/newtarget'); + $initial = $this->createPackageMock(); $target = $this->createPackageMock(); @@ -146,6 +151,11 @@ class LibraryInstallerTest extends TestCase ->method('getPrettyName') ->will($this->returnValue('package1')); + $target + ->expects($this->once()) + ->method('getTargetDir') + ->will($this->returnValue('newtarget')); + $this->repository ->expects($this->exactly(3)) ->method('hasPackage') @@ -154,7 +164,7 @@ class LibraryInstallerTest extends TestCase $this->dm ->expects($this->once()) ->method('update') - ->with($initial, $target, $this->vendorDir.'/package1'); + ->with($initial, $target, $this->vendorDir.'/package1/newtarget'); $this->repository ->expects($this->once()) @@ -166,6 +176,7 @@ class LibraryInstallerTest extends TestCase ->method('addPackage') ->with($target); + $library = new LibraryInstaller($this->io, $this->composer, 'library', $filesystem); $library->update($this->repository, $initial, $target); $this->assertFileExists($this->vendorDir, 'Vendor dir should be created'); $this->assertFileExists($this->binDir, 'Bin dir should be created');