Feature dist #804

Closed
wants to merge 45 commits into
from
Commits
+515 −18
Split
@@ -21,25 +21,31 @@
*/
class ArrayDumper
{
+ /**
+ * @var array
+ */
+ protected static $keys = array(
+ 'binaries' => 'bin',
+ 'scripts',
+ 'type',
+ 'extra',
+ 'installationSource' => 'installation-source',
+ 'license',
+ 'authors',
+ 'description',
+ 'homepage',
+ 'keywords',
+ 'autoload',
+ 'repositories',
+ 'includePaths' => 'include-path',
+ 'support',
+ );
+
+ /**
+ * @return array
+ */
public function dump(PackageInterface $package)
{
- $keys = array(
- 'binaries' => 'bin',
- 'scripts',
- 'type',
- 'extra',
- 'installationSource' => 'installation-source',
- 'license',
- 'authors',
- 'description',
- 'homepage',
- 'keywords',
- 'autoload',
- 'repositories',
- 'includePaths' => 'include-path',
- 'support',
- );
-
$data = array();
$data['name'] = $package->getPrettyName();
$data['version'] = $package->getPrettyVersion();
@@ -78,7 +84,7 @@ public function dump(PackageInterface $package)
$data['suggest'] = $packages;
}
- foreach ($keys as $method => $key) {
+ foreach (self::$keys as $method => $key) {
@Baachi

Baachi Aug 20, 2012

static::$keys would be better

@till

till Aug 20, 2012

Contributor

Why?

@stof

stof Aug 20, 2012

Contributor

because you made it protected, meaning it is a potential extension point and using self does not allow extending. But as I don't see the need for an extension point on this array, and it was not there before, you could simply switch to private and keep self

@till

till Aug 20, 2012

Contributor

Maybe we just keep it as is because this code-base is still pre 1.0.0 and we can lock down the API after.

if (is_numeric($method)) {
$method = $key;
}
@@ -0,0 +1,166 @@
+<?php
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ * Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Package\Dumper;
+
+use Composer\Package\PackageInterface;
+use Composer\Util\ProcessExecutor;
+use Composer\Downloader\GitDownloader;
+use Composer\Downloader\HgDownloader;
+use Composer\Downloader\SvnDownloader;
+use Composer\IO\NullIO;
+use Composer\Factory;
+
+/**
+ * @author Till Klampaeckel <till@php.net>
+ */
+abstract class BaseDumper implements DumperInterface
+{
+ /**
+ * Format: zip or tar.
+ * @var string
+ */
+ protected $format = '';
+
+ /**
+ * Path to where to dump the export to.
+ * @var mixed|null
+ */
+ protected $path;
+
+ /**
+ * @var ProcessExecutor
+ */
+ protected $process;
+
+ /**
+ * Working directory.
+ * @var string
+ */
+ protected $temp;
+
+ /**
+ * @param mixed $path
+ * @param ProcessExecutor|null $process
+ *
+ * @throws \InvalidArgumentException
+ */
+ public function __construct($path = null, ProcessExecutor $process = null)
+ {
+ if (!empty($path)) {
@Baachi

Baachi Aug 20, 2012

That would be better :)

if  (null !== $path && !is_writeable($path)) {
    throw new \InvalidArgumentException("Not authorized to write to '{$path}'");
}
+ if (!is_writable($path)) {
+ throw new \InvalidArgumentException("Not authorized to write to '{$path}'");
+ }
+ $this->path = $path;
+ }
@Baachi

Baachi Aug 21, 2012

Missing empty line here

@till

till Aug 22, 2012

Contributor

Explain, please?

@MattKetmo

MattKetmo Aug 22, 2012

Contributor

There is a CS rule for empty line before return ("Add a blank line before return statements, unless the return is alone inside a statement-group (like an if statement);" -- http://symfony.com/doc/current/contributing/code/standards.html).

This is not the case of this line, but the CS is not respected on other lines of the file...

@Baachi

Baachi Aug 23, 2012

My bad, sorry guys :)

+ $this->process = $process ?: new ProcessExecutor();
+ $this->temp = sys_get_temp_dir();
+ }
+
+ /**
+ * @return \Composer\Downloader\DownloadManager
+ */
+ public function getDownloadManager()
+ {
+ $factory = new Factory;
@stof

stof Jun 25, 2012

Contributor

you should add the parenthesis even if there is no arguments

@Seldaek

Seldaek Jun 25, 2012

Owner

That really doesn't matter..

+ $dm = $factory->createDownloadManager(new NullIO());
+ return $dm;
+ }
+
+ /**
+ * @param PackageInterface $package
+ * @param string $extension
+ *
+ * @return string
+ * @throws \InvalidArgumentException When unknown 'format' is encountered.
+ */
+ public function getFilename(PackageInterface $package, $extension)
+ {
+ $name = $package->getPrettyVersion();
+ $fileName = sprintf('%s.%s', $name, $extension);
+ return $fileName;
+ }
+
+ /**
+ * @param PackageInterface $package
+ *
+ * @return string
+ * @throws \RuntimeException
+ */
+ protected function getAndEnsureWorkDirectory(PackageInterface $package)
+ {
+ $workDir = sprintf('%s/%s/%s', $this->temp, $this->format, $package->getName());
+ if (!file_exists($workDir)) {
+ mkdir($workDir, 0777, true);
+ }
+ if (!file_exists($workDir)) {
+ throw new \RuntimeException("Could not find '{$workDir}' directory.");
+ }
+ return $workDir;
+ }
+
+ /**
+ * Package the given directory into an archive.
+ *
+ * The format is most likely \Phar::TAR or \Phar::ZIP.
+ *
+ * @param string $filename
+ * @param string $workDir
+ * @param int $format
+ *
+ * @throws \RuntimeException
+ */
+ protected function package($filename, $workDir, $format)
+ {
+ try {
+ $phar = new \PharData($filename, null, null, $format);
+ $phar->buildFromDirectory($workDir);
+ } catch (\UnexpectedValueException $e) {
+ $message = "Original PHAR exception: " . (string) $e;
+ $message .= PHP_EOL . PHP_EOL;
+ $message .= sprintf("Could not create archive '%s' from '%s'.", $filename, $workDir);
+ throw new \RuntimeException($message);
+ }
+ }
+
+ /**
+ * @param string $fileName
+ * @param string $sourceRef
+ * @param string $workDir
+ */
+ protected function packageGit($fileName, $sourceRef, $workDir)
+ {
+ $command = sprintf(
+ 'git archive --format %s --output %s %s',
+ $this->format,
+ escapeshellarg(sprintf('%s/%s', $this->path, $fileName)),
+ $sourceRef
+ );
+ $this->process->execute($command, $output, $workDir);
+ }
+
+ /**
+ * @param string $fileName
+ * @param string $sourceRef
+ * @param string $workDir
+ */
+ protected function packageHg($fileName, $sourceRef, $workDir)
+ {
+ $format = ($this->format == 'tarball')?'tar':$this->format;
+ $command = sprintf(
+ 'hg archive --rev %s --type %s %s',
+ $sourceRef,
+ $format,
+ escapeshellarg(sprintf('%s/%s', $this->path, $fileName))
+ );
+ $this->process->execute($command, $output, $workDir);
+ }
+}
@@ -0,0 +1,29 @@
+<?php
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ * Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+namespace Composer\Package\Dumper;
+
+use Composer\Package\PackageInterface;
+
+/**
+ * @author Till Klampaeckel <till@php.net>
+ */
+interface DumperInterface
+{
+ /**
+ * Return value depends on implementation - e.g. generating a tar or zip the
+ * method currently returns void, the ArrayDumper returns an array.
+ *
+ * @param PackageInterface $package
+ *
+ * @return void
+ */
+ public function dump(PackageInterface $package);
+}
@@ -0,0 +1,57 @@
+<?php
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ * Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Package\Dumper;
+
+use Composer\Package\Dumper\BaseDumper;
+use Composer\Package\Dumper\DumperInterface;
+use Composer\Package\PackageInterface;
+use Composer\Util\ProcessExecutor;
+
+/**
+ * @author Ulf Härnhammar <ulfharn@gmail.com>
+ */
+class TarDumper extends BaseDumper
+{
+ protected $format = 'tar';
@pborreli

pborreli Aug 20, 2012

Contributor

why don't use it and factorize dump method (removing duplicate code) between both dumper classes ?

@till

till Aug 20, 2012

Contributor

@pborreli If you have time, send me a pr to till/composer and I'll include your fixes.

@MattKetmo

MattKetmo Aug 20, 2012

Contributor

I will make a PR in the next few days. I've some ideas to develop ;)

+
+ /**
+ * @param PackageInterface $package
+ * @throws \InvalidArgumentException
+ */
+ public function dump(PackageInterface $package)
+ {
+ $workDir = $this->getAndEnsureWorkDirectory($package);
+
+ $fileName = $this->getFilename($package, 'tar');
+ $sourceType = $package->getSourceType();
+ $sourceRef = $package->getSourceReference();
+
+ $dm = $this->getDownloadManager();
+ $dm->download($package, $workDir, true);
+
+ switch ($sourceType) {
+ case 'git':
+ $this->packageGit($fileName, $sourceRef, $workDir);
+ break;
+ case 'hg':
+ $this->packageHg($fileName, $sourceRef, $workDir);
+ break;
+ case 'svn':
+ $dir = $workDir . (substr($sourceRef, 0, 1) !== '/')?'/':'' . $sourceRef;
+ $this->package($fileName, $dir, \Phar::TAR);
+ break;
+ default:
@Baachi

Baachi Aug 21, 2012

Use the tar shell command instead of throw an exception?
What do you think?

@stof

stof Aug 21, 2012

Contributor

@Baachi there is simply no other source type supported in composer currently anyway (and the installation is forced from source)

+ throw new \InvalidArgumentException(
+ "Unable to handle repositories of type '{$sourceType}'.");
+ }
+ }
+}
@@ -0,0 +1,56 @@
+<?php
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ * Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Package\Dumper;
+
+use Composer\Package\Dumper\BaseDumper;
+use Composer\Package\Dumper\DumperInterface;
+use Composer\Package\PackageInterface;
+use Composer\Util\ProcessExecutor;
+
+/**
+ * @author Till Klampaeckel <till@php.net>
+ */
+class ZipDumper extends BaseDumper
+{
+ protected $format = 'zip';
+
+ /**
+ * @param PackageInterface $package
+ * @throws \InvalidArgumentException
+ */
+ public function dump(PackageInterface $package)
+ {
+ $workDir = $this->getAndEnsureWorkDirectory($package);
+
+ $fileName = $this->getFilename($package, 'zip');
+ $sourceType = $package->getSourceType();
+ $sourceRef = $package->getSourceReference();
+
+ $dm = $this->getDownloadManager();
+ $dm->download($package, $workDir, true);
+
+ switch ($sourceType) {
+ case 'git':
+ $this->packageGit($fileName, $sourceRef, $workDir);
+ break;
+ case 'hg':
+ $this->packageHg($fileName, $sourceRef, $workDir);
+ break;
+ case 'svn':
+ $dir = $workDir . (substr($sourceRef, 0, 1) !== '/')?'/':'' . $sourceRef;
@pborreli

pborreli Aug 20, 2012

Contributor

i would write it

$dir = $workDir . ($sourceRef[0] === '/') ?: '/' . $sourceRef;

or even

$dir = $workDir . '/' . ltrim($sourceRef, '/');
+ $this->package($fileName, $dir, \Phar::ZIP);
+ break;
+ default:
+ throw new \InvalidArgumentException("Unable to handle repositories of type '{$sourceType}'.");
+ }
+ }
+}
Oops, something went wrong.