From 87b5af60a878de54e26bdf4304768a8ae4e55cb1 Mon Sep 17 00:00:00 2001 From: MakiCode Date: Fri, 2 Oct 2015 09:09:09 -0500 Subject: [PATCH 1/8] Almost finished adding --file option, need to add unit test --- src/Composer/Command/ArchiveCommand.php | 8 +++++--- src/Composer/Package/Archiver/ArchiveManager.php | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Composer/Command/ArchiveCommand.php b/src/Composer/Command/ArchiveCommand.php index b8c439678..b70f06c27 100644 --- a/src/Composer/Command/ArchiveCommand.php +++ b/src/Composer/Command/ArchiveCommand.php @@ -42,6 +42,7 @@ class ArchiveCommand extends Command new InputArgument('version', InputArgument::OPTIONAL, 'A version constraint to find the package to archive'), new InputOption('format', 'f', InputOption::VALUE_OPTIONAL, 'Format of the resulting archive: tar or zip'), new InputOption('dir', false, InputOption::VALUE_OPTIONAL, 'Write the archive to this directory'), + new InputOption('file', false, InputOption::VALUE_OPTIONAL, 'Write the archive with the given file name.'), )) ->setHelp(<<archive command creates an archive of the specified format @@ -78,7 +79,8 @@ EOT $input->getArgument('package'), $input->getArgument('version'), $input->getOption('format'), - $input->getOption('dir') + $input->getOption('dir'), + $input->getOption("file") ); if (0 === $returnCode && $composer) { @@ -88,7 +90,7 @@ EOT return $returnCode; } - protected function archive(IOInterface $io, Config $config, $packageName = null, $version = null, $format = 'tar', $dest = '.') + protected function archive(IOInterface $io, Config $config, $packageName = null, $version = null, $format = 'tar', $dest = '.', $fileName = null) { $factory = new Factory; $downloadManager = $factory->createDownloadManager($io, $config); @@ -105,7 +107,7 @@ EOT } $io->writeError('Creating the archive into "'.$dest.'".'); - $packagePath = $archiveManager->archive($package, $format, $dest); + $packagePath = $archiveManager->archive($package, $format, $dest, $fileName); $fs = new Filesystem; $shortPath = $fs->findShortestPath(getcwd(), $packagePath, true); diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index d600a7a75..527757a4e 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -96,12 +96,14 @@ class ArchiveManager * * @param PackageInterface $package The package to archive * @param string $format The format of the archive (zip, tar, ...) - * @param string $targetDir The diretory where to build the archive + * @param string $targetDir The directory where to build the archive + * @param string|null $fileName The file name to use for the archive, or null to use the generated + * package name * @throws \InvalidArgumentException * @throws \RuntimeException * @return string The path of the created archive */ - public function archive(PackageInterface $package, $format, $targetDir) + public function archive(PackageInterface $package, $format, $targetDir, $fileName = null) { if (empty($format)) { throw new \InvalidArgumentException('Format must be specified'); From 906c1c2e66ba01dff187643e49ccffbc70bb46ea Mon Sep 17 00:00:00 2001 From: MakiCode Date: Sun, 4 Oct 2015 19:53:07 -0500 Subject: [PATCH 2/8] Added check for filename in archive manager and added test --- .../Package/Archiver/ArchiveManager.php | 9 +++- .../Package/Archiver/ArchiveManagerTest.php | 41 ++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 527757a4e..656d2f7a2 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -108,6 +108,9 @@ class ArchiveManager if (empty($format)) { throw new \InvalidArgumentException('Format must be specified'); } + if(!is_null($fileName) && !is_string($fileName)) { + throw new \InvalidArgumentException('fileName must be a string'); + } // Search for the most appropriate archiver $usableArchiver = null; @@ -124,7 +127,11 @@ class ArchiveManager } $filesystem = new Filesystem(); - $packageName = $this->getPackageFilename($package); + if(null === $fileName) { + $packageName = $fileName; + } else { + $packageName = $this->getPackageFilename($package); + } // Archive filename $filesystem->ensureDirectoryExists($targetDir); diff --git a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php index f4d343e63..d5f7300a9 100644 --- a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php +++ b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php @@ -13,11 +13,16 @@ namespace Composer\Test\Package\Archiver; use Composer\Factory; +use Composer\Package\Archiver\ArchiveManager; use Composer\Package\PackageInterface; class ArchiveManagerTest extends ArchiverTest { + /** + * @var ArchiveManager + */ protected $manager; + protected $targetDir; public function setUp() @@ -55,9 +60,41 @@ class ArchiveManagerTest extends ArchiverTest unlink($target); } - protected function getTargetName(PackageInterface $package, $format) + public function testArchiveCustomFileName() { - $packageName = $this->manager->getPackageFilename($package); + $this->setupGitRepo(); + + $package = $this->setupPackage(); + + $fileName = "testArchiveName"; + + $this->manager->archive($package, 'tar', $this->targetDir, $fileName); + + $target = $this->getTargetName($package, 'tar', $fileName); + + $this->assertFileExists($target); + + $tmppath = sys_get_temp_dir().'/composer_archiver/'.$this->manager->getPackageFilename($package); + $this->assertFileNotExists($tmppath); + + unlink($target); + } + + public function testNonStringFileName() { + $this->setExpectedException("InvalidArgumentException"); + + $package = $this->setupPackage(); + + $this->manager->archive($package, 'tar', $this->targetDir, array()); + } + + protected function getTargetName(PackageInterface $package, $format, $fileName = null) + { + if(!is_null($fileName)) { + $packageName = $fileName; + } else { + $packageName = $this->manager->getPackageFilename($package); + } $target = $this->targetDir.'/'.$packageName.'.'.$format; return $target; From 0d00338bdb50ef5ba64bb0ed6c0b78b459133d3a Mon Sep 17 00:00:00 2001 From: MakiCode Date: Sun, 4 Oct 2015 20:03:06 -0500 Subject: [PATCH 3/8] Added better messages and fixed bugs --- src/Composer/Command/ArchiveCommand.php | 3 ++- src/Composer/Package/Archiver/ArchiveManager.php | 8 ++++---- .../Test/Package/Archiver/ArchiveManagerTest.php | 9 +++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Composer/Command/ArchiveCommand.php b/src/Composer/Command/ArchiveCommand.php index b70f06c27..6a4914a7e 100644 --- a/src/Composer/Command/ArchiveCommand.php +++ b/src/Composer/Command/ArchiveCommand.php @@ -42,7 +42,8 @@ class ArchiveCommand extends Command new InputArgument('version', InputArgument::OPTIONAL, 'A version constraint to find the package to archive'), new InputOption('format', 'f', InputOption::VALUE_OPTIONAL, 'Format of the resulting archive: tar or zip'), new InputOption('dir', false, InputOption::VALUE_OPTIONAL, 'Write the archive to this directory'), - new InputOption('file', false, InputOption::VALUE_OPTIONAL, 'Write the archive with the given file name.'), + new InputOption('file', false, InputOption::VALUE_OPTIONAL, 'Write the archive with the given file name.' + .' Note that the format will be appended.'), )) ->setHelp(<<archive command creates an archive of the specified format diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 656d2f7a2..4a9ce4a94 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -97,8 +97,8 @@ class ArchiveManager * @param PackageInterface $package The package to archive * @param string $format The format of the archive (zip, tar, ...) * @param string $targetDir The directory where to build the archive - * @param string|null $fileName The file name to use for the archive, or null to use the generated - * package name + * @param string|null $fileName The relative file name to use for the archive, or null to generate + * the package name. Note that the format will be appended to this name * @throws \InvalidArgumentException * @throws \RuntimeException * @return string The path of the created archive @@ -128,9 +128,9 @@ class ArchiveManager $filesystem = new Filesystem(); if(null === $fileName) { - $packageName = $fileName; - } else { $packageName = $this->getPackageFilename($package); + } else { + $packageName = $fileName; } // Archive filename diff --git a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php index d5f7300a9..082d6e0ca 100644 --- a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php +++ b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php @@ -70,7 +70,7 @@ class ArchiveManagerTest extends ArchiverTest $this->manager->archive($package, 'tar', $this->targetDir, $fileName); - $target = $this->getTargetName($package, 'tar', $fileName); + $target = $this->targetDir . "/" . $fileName . ".tar"; $this->assertFileExists($target); @@ -90,11 +90,12 @@ class ArchiveManagerTest extends ArchiverTest protected function getTargetName(PackageInterface $package, $format, $fileName = null) { - if(!is_null($fileName)) { - $packageName = $fileName; - } else { + if(null === $fileName) { $packageName = $this->manager->getPackageFilename($package); + } else { + $packageName = $fileName; } + $target = $this->targetDir.'/'.$packageName.'.'.$format; return $target; From 63ede6c9dd08bd69b797c1f4bdb19fcd6e4105d8 Mon Sep 17 00:00:00 2001 From: MakiCode Date: Wed, 7 Oct 2015 12:42:19 -0500 Subject: [PATCH 4/8] Fixed issues with PR --- src/Composer/Command/ArchiveCommand.php | 8 ++++---- src/Composer/Package/Archiver/ArchiveManager.php | 4 ++-- .../Test/Package/Archiver/ArchiveManagerTest.php | 9 +++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Composer/Command/ArchiveCommand.php b/src/Composer/Command/ArchiveCommand.php index 6a4914a7e..616976feb 100644 --- a/src/Composer/Command/ArchiveCommand.php +++ b/src/Composer/Command/ArchiveCommand.php @@ -40,9 +40,9 @@ class ArchiveCommand extends Command ->setDefinition(array( new InputArgument('package', InputArgument::OPTIONAL, 'The package to archive instead of the current project'), new InputArgument('version', InputArgument::OPTIONAL, 'A version constraint to find the package to archive'), - new InputOption('format', 'f', InputOption::VALUE_OPTIONAL, 'Format of the resulting archive: tar or zip'), - new InputOption('dir', false, InputOption::VALUE_OPTIONAL, 'Write the archive to this directory'), - new InputOption('file', false, InputOption::VALUE_OPTIONAL, 'Write the archive with the given file name.' + new InputOption('format', 'f', InputOption::VALUE_REQUIRED, 'Format of the resulting archive: tar or zip'), + new InputOption('dir', false, InputOption::VALUE_REQUIRED, 'Write the archive to this directory'), + new InputOption('file', false, InputOption::VALUE_REQUIRED, 'Write the archive with the given file name.' .' Note that the format will be appended.'), )) ->setHelp(<<getArgument('version'), $input->getOption('format'), $input->getOption('dir'), - $input->getOption("file") + $input->getOption('file') ); if (0 === $returnCode && $composer) { diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 4a9ce4a94..51669e278 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -108,7 +108,7 @@ class ArchiveManager if (empty($format)) { throw new \InvalidArgumentException('Format must be specified'); } - if(!is_null($fileName) && !is_string($fileName)) { + if (null !== $fileName && !is_string($fileName)) { throw new \InvalidArgumentException('fileName must be a string'); } @@ -127,7 +127,7 @@ class ArchiveManager } $filesystem = new Filesystem(); - if(null === $fileName) { + if (null !== $fileName) { $packageName = $this->getPackageFilename($package); } else { $packageName = $fileName; diff --git a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php index 082d6e0ca..cc0490293 100644 --- a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php +++ b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php @@ -66,11 +66,11 @@ class ArchiveManagerTest extends ArchiverTest $package = $this->setupPackage(); - $fileName = "testArchiveName"; + $fileName = 'testArchiveName'; $this->manager->archive($package, 'tar', $this->targetDir, $fileName); - $target = $this->targetDir . "/" . $fileName . ".tar"; + $target = $this->targetDir . '/' . $fileName . '.tar'; $this->assertFileExists($target); @@ -80,8 +80,9 @@ class ArchiveManagerTest extends ArchiverTest unlink($target); } - public function testNonStringFileName() { - $this->setExpectedException("InvalidArgumentException"); + public function testNonStringFileName() + { + $this->setExpectedException('InvalidArgumentException'); $package = $this->setupPackage(); From 7005d8984798245cc14625a64b6efd4a93ecd63f Mon Sep 17 00:00:00 2001 From: MakiCode Date: Wed, 7 Oct 2015 12:44:23 -0500 Subject: [PATCH 5/8] Fixed bug --- src/Composer/Package/Archiver/ArchiveManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 51669e278..787a9f742 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -127,7 +127,7 @@ class ArchiveManager } $filesystem = new Filesystem(); - if (null !== $fileName) { + if (null === $fileName) { $packageName = $this->getPackageFilename($package); } else { $packageName = $fileName; From 22e93f110b44d109384377a1d94c5cce8cfe7bb6 Mon Sep 17 00:00:00 2001 From: MakiCode Date: Wed, 7 Oct 2015 15:52:58 -0500 Subject: [PATCH 6/8] Fixed indentation --- src/Composer/Package/Archiver/ArchiveManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 787a9f742..dbf0b5977 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -98,7 +98,7 @@ class ArchiveManager * @param string $format The format of the archive (zip, tar, ...) * @param string $targetDir The directory where to build the archive * @param string|null $fileName The relative file name to use for the archive, or null to generate - * the package name. Note that the format will be appended to this name + * the package name. Note that the format will be appended to this name * @throws \InvalidArgumentException * @throws \RuntimeException * @return string The path of the created archive From fefc106ef68c401087e29dd950c7d95dd475fd79 Mon Sep 17 00:00:00 2001 From: MakiCode Date: Mon, 12 Oct 2015 10:18:57 -0500 Subject: [PATCH 7/8] Removed type check on ArchiveManager --- src/Composer/Package/Archiver/ArchiveManager.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index dbf0b5977..621fd5286 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -108,10 +108,7 @@ class ArchiveManager if (empty($format)) { throw new \InvalidArgumentException('Format must be specified'); } - if (null !== $fileName && !is_string($fileName)) { - throw new \InvalidArgumentException('fileName must be a string'); - } - + // Search for the most appropriate archiver $usableArchiver = null; foreach ($this->archivers as $archiver) { From 1c0b9cd65f1ea23ea788724e1018fab07dc0bf6c Mon Sep 17 00:00:00 2001 From: MakiCode Date: Mon, 12 Oct 2015 10:20:45 -0500 Subject: [PATCH 8/8] Removed test for non-string file name check --- src/Composer/Package/Archiver/ArchiveManager.php | 2 +- .../Test/Package/Archiver/ArchiveManagerTest.php | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Composer/Package/Archiver/ArchiveManager.php b/src/Composer/Package/Archiver/ArchiveManager.php index 621fd5286..3ff3f2e98 100644 --- a/src/Composer/Package/Archiver/ArchiveManager.php +++ b/src/Composer/Package/Archiver/ArchiveManager.php @@ -108,7 +108,7 @@ class ArchiveManager if (empty($format)) { throw new \InvalidArgumentException('Format must be specified'); } - + // Search for the most appropriate archiver $usableArchiver = null; foreach ($this->archivers as $archiver) { diff --git a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php index cc0490293..402178de1 100644 --- a/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php +++ b/tests/Composer/Test/Package/Archiver/ArchiveManagerTest.php @@ -79,16 +79,7 @@ class ArchiveManagerTest extends ArchiverTest unlink($target); } - - public function testNonStringFileName() - { - $this->setExpectedException('InvalidArgumentException'); - - $package = $this->setupPackage(); - - $this->manager->archive($package, 'tar', $this->targetDir, array()); - } - + protected function getTargetName(PackageInterface $package, $format, $fileName = null) { if(null === $fileName) {