Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36639
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.94.56 as permitted sender)
Date: Fri, 28 Mar 2008 11:17:09 +0100
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <26719276.20080328111709@marcus-boerger.de>
To: Lars Strojny <lars@strojny.net>
CC: Benjamin Schulz <php@benjaminschulz.com>, 
	internals Mailing List <internals@lists.php.net>
In-Reply-To: <1206626878.7434.56.camel@localhost>
References: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com> <1206626878.7434.56.camel@localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] phar API
From: helly@php.net (Marcus Boerger)

Hello Lars,

Thursday, March 27, 2008, 3:07:58 PM, you wrote:

> Hi Benjamin,

> Am Donnerstag, den 27.03.2008, 14:34 +0100 schrieb Benjamin Schulz:
>> Hi,
>> i just read the phar examples in the manual and found things like this:
> [...]
>> First thing: yes i fully understand what the code is doing but i still
>> think that it doesn't need to be so "hackish". One thing is that i
>> think there is no point in using ArrayAccess here,  in my oppinion
>> ArrayAccess is great to hack stuff but it doesn't belong in such
>> (maybe soon?) core functionality.

> ArrayAccess is not hackish per se, but it just does not feels right for
> working with archives.
> [...]
>> $p = new Phar('coollibrary.phar');
>> 
>> /* What about creating a non-static pendant to canWrite()?
>>     Maybe there is an archive file that has only read permissions on  
>> the filesystem or
>>     phar will be able to generate readonly-archives later? (Might be  
>> interesting for  companies that want to provide readonly and signed archives for  
>> the customers)
>> if ($p->canWrite()) {
>>      // Create a file instance
>>      $hugeFile = $p->createFile();
>> 
>>      $fp = fopen('hugefile.dat', 'rb');
>> 
>>      // Set the content
>>      $hugeFile->setContent($fp); (or maybe even  
>> setContentResourceHandle(...))
>>      if (Phar::canCompress()) {
>>          /* how is the compression implemented? through streamfilters?
>>             than how about a ->setCompression('bzip')
>>          that internally resolves to the bzip.(de)compress:// stuff?
>>          $p['data/hugefile.dat']->setCompressedGZ();
>>      }
>> 
>>      // add the file to the archive
>>      $p->addFile($hugeFile, 'data/hugefile.dat');

>>      // another option would be to pass the file path to the  
>> createFile() method and
>>      // implicitely create it in the archive
>>      $indexPhp = $p->createFile('index.php');
>>      $indexPhp->setContent(...);
>> }
>> 
> I second your proposal, but a bit more flexible:

> // Creating files
> $phar = new Phar();
> $file = $phar->createFile('/filename'); ﻿// Phar acts as a factory and
> returns an object PharFile or something
> $file->setContent('string'); // Polymorph signature, can either take a string ...
> $file->setContent(fopen('file', 'r')); // ... or a resource
> $file->setContent('foo', $data); // ... or a string and data
> $file->setContent('bar', array('line1', 'line2')); // ... or an array of lines
> $file->setContent('baz', fopen('file', 'r')); // ... or a name and a resource
> if (!$phar->isReadonly()) {
>     $phar->save(); // Writes the newly create file to the filesystem
> }

The above is far too complex. And results in an interface that does not
make clear what it is doing. Right now we have $phar->offsetSet() with a
very clear semantics.

Maybe what we want to add is getContents() and setContents() to SplFileInfo.

> $phar = new Phar();
> $phar->add('foo');
> $phar->add('bar', $data);
> $phar->add(new SplFileInfo('bar'));

Yet again, unclear. First probable creates an empty entry, same as
offsetSet('foo', ''); only that one is clear. The second one create an
entry with content, which is exactly what $phar->offsetSet('bar',$data)
does. The third is extremely confusing. What does it do? What will be the
entry name? And will it just take the entry name and make an emoty entry or
will it copy the data from the referenced file?

> Creating directories:
> $phar = new Phar();
> $dir = $phar->createDirectory('/foo'); // Return PharDirectory object
> $dir->add(new DirectoryIterator("mydir")); // Adds every file in the directory mydir
> $dir->add(new RecursiveDirectoryIterator('otherdir')); // Adds every item recursivly
> $dir->add(new SplFile("foo")); // Adds the file foo
> $dir->add('bar'); // Adds bar
> $dir->add('baz', $data)
> $file = $dir->createFile('bla')
> $file->setContent($data);
> $dir2 = $dir->createDirectory('foo');
> ...
> $phar->save();

We have a very nice Phar::buildFromIterator() for this kind of creation.
We also do not really care for directories. Maybe that is something worth
to add, especially the createDirectory() method.

> No, with fluent interfaces:
> $phar = new Phar();
> $phar->createDirectory('foo')
>    ->add('foo')
>    ->add(new SplFileInfo('bar'))
>    ->add('baz', $data);
> $phar->save();

First of all I do not like fluent interfaces at all here. Because it is
unclear what it does. What is the return value of each of these?
Is it the Phar object of the PharEntry or whatever object?

If it is a reference to the entry then you confuse me even more. Because
then wher do all the adds go to? If it is the phar then it makes a bit
more sense and indeed the save() can be put at the end. But in general
fluent interfaces tend to be very unclear and make it harder to maintain
code. Thay allow nice GUI code as they can drop out all the overhead, sure
but here you see me confused.

> if PharFile would include a reference to the original Phar object, the
> last Phar::save() could even be included in the chain.

> About compression:
> I would prefer to have Phar::setCompression(Phar::GZIP) or something
> similiar. Or even Phar::setCompressionStrategy(PharCompressionInterface
> $interface) but I have the feeling that would go too far :-)

Maybe adding Phar::compressAllFiles() would indeed be good. This one could
easily map to the two exiting functions.

Best regards,
 Marcus