Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36624
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain benjaminschulz.com from 80.81.249.139 cause and error)
To: Marcus Boerger <helly@php.net>
In-Reply-To: <542136731.20080327193855@marcus-boerger.de>
References: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com> <542136731.20080327193855@marcus-boerger.de>
Message-ID: <FAB780C8-4A0E-4DBC-BF99-1560B139389F@benjaminschulz.com>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0 (Apple Message framework v919.2)
Date: Thu, 27 Mar 2008 20:58:47 +0100
Cc: internals Mailing List <internals@lists.php.net>
Subject: Re: [PHP-DEV] phar API
From: php@benjaminschulz.com (Benjamin Schulz)

Hi Marcus,

>> 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.

I used PHP 5 when it had namespaces the first time ;) So i am not new  
to PHP5
features, that's not the point.

>
>> 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?

In my personal oppinion: yes. There is no need for array syntax if you  
provide
a OO api, why not setContent() or something else? There is no real  
difference
in the implementation, 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.

The point is that the getter does something completely different from  
the
setter. The getter returns a file object but the setter sets the content
of the file, this is just not consistent.

>
>
>> 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 how do i check if the archive is readonly? canWrite() just tells me
if the php.ini setting is enabled, right?

> And obviously you cannot create
> signed archives where only selected entries are readonly.

I don't see where this has to do with the point of my mail but i think
it could easily achieved with
$p->getFile('data/hugefile.dat')->canWrite()

>
>
>> 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

correct, it would be just some "random" file object related to the phar
archive until i use addFile() like below, if you need this relation one
could provide something like
$p->createFile('data/hugefile.dat');
$p->setContent('...');

> - and then wher is $p['data/hugefile.dat'] coming from?
> - and did you not just write ArrayAccess is wrong?

sorry, my fault - it should be $hugeFile

>>     // 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.

Do you think that is well named? It's what the ArrayAccess interface
prescribes but wouldn't a $p->setFile(), ->addFile() describe better
what's happening? I mean we're talking about OO here.

>>
>>     // 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->...

Yep, array syntax? but it's an object right? That is what i mean with  
"hackish" - no
need for array syntax if i work with objects. I still believe that  
ArrayAccess has
it's advantages but IMHO not in this case.

regards,
Benjamin