Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36626
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 38.99.98.18 as permitted sender)
Message-ID: <47EC1271.9020405@php.net>
Date: Thu, 27 Mar 2008 16:32:33 -0500
User-Agent: Thunderbird 2.0.0.12 (Windows/20080213)
MIME-Version: 1.0
To: Benjamin Schulz <php@benjaminschulz.com>
CC: internals Mailing List <internals@lists.php.net>, lars@strojny.net
References: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com>
In-Reply-To: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: phar API
From: cellog@php.net (Greg Beaver)

Benjamin Schulz wrote:
> Hi,
> i just read the phar examples in the manual and found things like this:
> 
> $p = new Phar('coollibrary.phar');
> if (Phar::canWrite()) {
>     $fp = fopen('hugefile.dat', 'rb');
>     $p['data/hugefile.dat'] = $fp;
>     if (Phar::canCompress()) {
>         $p['data/hugefile.dat']->setCompressedGZ();
>     }
>     $p['images/wow.jpg'] = file_get_contents('images/wow.jpg');
>     $p['images/wow.jpg']->setMetaData(array('mime-type' => 'image/jpeg'));
>     $p['index.php'] = file_get_contents('index.php');
> }
> 
> 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.
> The second thing that looks weird to me is that the setter (offsetSet) 
> sets the file content but the getter (offsetGet) retrieves a file object.
> What about changing this into something more OO like this (just a 
> proposal):
> 
> $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)

<snip example>

Some notes:

1) if you want to know writability without Phar::canWrite(), you can 
also use is_writeable('phar:///path/to/archive.phar/index.php') just 
like you can with any other file
2) ArrayAccess is a very natural way of viewing an archive - archives 
are a collection of files that can be accessed randomly, just like an 
associative array.  As such, it can be viewed abstractly as an array of 
objects with attributes and methods (files with properties like 
compression, checksum, size, metadata, contents and operations to modify 
the file or query the contents).
3) Phar extends DirectoryIterator.  As such, one needs to think of Phar 
as an enhanced DirectoryIterator.  This comes with the benefits of 
DirectoryIterator (easy iteration, ability to iterate over a CSV file 
quite easily, ability to iterate over lines of the file easily) and its 
disadvantages (not designed for easy modification of the files or access 
to their contents).  Fortunately, Marcus is a lead maintainer of both 
Phar and SPL, and so he has already fixed some of the missing pieces in SPL.
4) Phar does not force one-true-way of building the archive.  The stream 
wrapper is fully enabled with write capabilities as well.

Let's take a look at the ways you can populate the contents of a phar 
archive:

1) file_put_contents('phar:///path/to/archive.phar/index.php', 'file 
contents');
2) $phar = new Phar('/path/to/archive.phar'); 
$phar->buildFromIterator(new DirectoryIterator('/some/path'), '/some/path');
3) $phar = new Phar('/path/to/archive.phar'); $phar['index.php'] = 
file_get_contents('/some/path/index.php');

There's plenty of flexibility here, and no need to be scared of the 
arrayaccess option.

I do agree that although I am perfectly comfortable with the current 
API, it may seem inconsistent to others who think the way you do to 
directly set contents that way when offsetGet() does not return the 
contents.  However, the solution is quite simple, which is to make even 
clearer what operation we are doing:

$contents = $phar['index.php']->contents;
$phar['index.php']->contents = 'something';

This way, it is even clearer that the archive is an "array" of 
PharFileInfo objects.  This would be very easy to implement and would 
not require removing the existing functionality.

I see no problem in adding an alias to offsetSet() called addFile() and 
another for addFromString() and a method for addEmptyDir().  In case it 
isn't obvious, I will not do this change unless it adheres to the 
existing ext/zip API to allow easy mental migration for developers 
between the two extensions, so don't even think about suggesting other 
possibilities, thank you :).

Also, as an important note, the only up-to-date manual is at 
http://docs.php.net/manual/en/book.phar.php and furthermore the manual 
is not yet completely updated to the 2.0 API, which is one of the 
reasons I marked phar alpha on release.  None of the code you quoted, 
however, is out of date.  Code that is out of date in the manual is in 
compressAllFiles*() and some of the coding examples use the old syntax. 
  The changes missing from the manual are as follows:

1) compressAllFilesGZ() and compressAllFilesBZIP2() do nothing for 
tar-based archives, as there is no way to individually compress files in 
a tar archive.  To compress an entire archive, use the new compress() 
method.  Formerly, compressAllFilesGZ() changed a .tar to a .tar.gz.
2) convertToTar/convertToZip/convertToPhar() all return a Phar or a 
PharData object, allowing fluent interface niceness such as :

$phar = $phar->convertToTar()->compress(Phar::GZ);

Thanks for checking out phar.

Greg