Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36621
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: Thu, 27 Mar 2008 19:38:55 +0100
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <542136731.20080327193855@marcus-boerger.de>
To: Benjamin Schulz <php@benjaminschulz.com>
CC: internals Mailing List <internals@lists.php.net>
In-Reply-To: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com>
References: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] phar API
From: helly@php.net (Marcus Boerger)

Hello Benjamin,

Thursday, March 27, 2008, 2:34:06 PM, you 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".

I wouldn't call it hackish. I'd eventually call it new to people that
haven't used the new PHP 5.0 features yet.

> 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.

So you are basically saying we should remove ArrayAccess, right?

> 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.

Right now the setter takes a resource or a string. Maybe we can allow an
SplFileInfo instance too. But note that this is even more wierd as we then
would need to convert this instance into another instance and still only
transfer the contents.

> 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)

We can already create readonly archives. And obviously you cannot create
signed archives where only selected entries are readonly.

> 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();
>      }

Hi? This makes no sense whatsover. Because:
Ther is no connection between your stuff.
- $hugeFile would be an anonymous entry in your file
- and then wher is $p['data/hugefile.dat'] coming from?
- and did you not just write ArrayAccess is wrong?

>      // add the file to the archive
>      $p->addFile($hugeFile, 'data/hugefile.dat');

Well I prefer the other order. Oh and then that is just what
$p->offsetSet('data/hugefile.dat', $fp); does already.

>      // 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(...);

Well right now, you'd do $indexPhp = $p['index.php']; $indexPhp->...

Ok, all in all I would be ok with adding more functionality. But it needs
to work.


Best regards,
 Marcus