Re: [PHP-DEV] practical phar considerations
Hi Greg,
Am Freitag, den 28.03.2008, 17:13 -0500 schrieb Greg Beaver:
[...]
> In addition, this would mean that offsetGet() would have to create a
> directory. Why?
>
> $phar['full']['path']['to']['file'] = 'hi' calls offsetGet() with
> 'full', 'path', and 'to', and offsetSet() only with 'file'.
I know that and I discussed that with Marcus beforehand. I don't think
that implicitly creating the directory is much an issue. If somebody
does not want that, he can just use isset() before.
> In other words, the proposed change is much more complex, introduces
> magic and counterintuitive behavior inside the extension. For instance,
> let's say we have this code:
>
> $a = $phar['dir'];
> $b = $phar['dri']; // typo
> ?>
>
> if phar.readonly=1, the second line would throw an exception for
> attempting to create the directory "dri"
Wouldn't the second line throw an exeception anyway because the index is
not defined? And: can't offsetGet() check weither Phar::canWrite() is
true?
> What about this?
>
> unset($phar['dri']); // dri doesn't exist
> ?>
>
> The above code would first create the dri directory, and then erase it.
No, it would not. Please take a look at
http://lars.schokokeks.org/php/phar.phps and
http://lars.schokokeks.org/php/phar.php
> In other words, the proposed syntax is not a good idea.
Two of three assumptions were not correct or at least arguable, so I
can't agree that easily.
> 2) Names must be very carefully considered in relation to existing APIs
> and the pre-existing methods.
>
> Phar::create(), for instance, should be named buildFromDirectory() to
> match buildFromIterator().
I agree, that this name is much better.
> what benefit is there in adding complexity of remembering the Phar::GZ
> constant over setCompressedGZ()? We have to think hard about changes
> like this.
setCompressedGZ() is just not the way object oriented APIs should
behave, as it taints the API with concrete implementation details
("there can be gzip compression and every compression algorithm is
exposed" opposed to "there can be compression, one variant is gzip").
Phar::compress() indicates that "the archive can be compressed" with the
concrete algorithm as a parameter. What about in 5 years when a new,
free compression algorithm rises, which fixes the problems of bzip2.
Would you like to add another setCompressedFooBar() or just add the
algorithm and the class constant and be fine?
If you insist on setCompressedFoo(), I would at least propose to not
implement that as a setter but to describe the attached behaviour, which
would be Phar::compressWithGzip() or something like that. Nevertheless,
please seriously consider to do it with a single compress()-method.
> 3) I would like to suggest these changes, and these alone:
>
> * simplify all compression methods, with more consistency of naming
> that clearly differentiates between compressing files within an archive,
> and compressing the entire archive. This abstract idea is more
> important to me than the specific naming, but it should be very easy to do.
What would be your concrete proposal regarding that?
[...]
cu, Lars
signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil
Re: [PHP-DEV] [RFC] Double quoted NOWDOC is HEREDOC
Hi Felipe, Am Freitag, den 28.03.2008, 20:28 -0300 schrieb Felipe Pena: > 2008/3/24, Marcus Boerger <[EMAIL PROTECTED]>: [...] > +1 for same reason. > > However, the patch is wrong, see below: > > $foo = 'foobar'; > > $test = <<<"a > $foo > a; > > var_dump($test); > /* > string(7) "foobar > " > */ > > $test = b<<<"a" > $foo > a; > > /* > Parse error: syntax error, unexpected $end > */ > > Here's a possible fix: > - http://felipe.ath.cx/diff/double-quote-heredoc.diff Thanks for bringing this up, didn't thought about that. I will create a number of tests to cover that properly. cu, Lars signature.asc Description: Dies ist ein digital signierter Nachrichtenteil
Re: [PHP-DEV] Re: phar API
Hello Elizabeth,
Friday, March 28, 2008, 2:21:01 PM, you wrote:
> First of all Greg, thanks for all your hard work on phar - however I do
> agree that some of the API choices are going to cause confusion with
> "casual" phar users.
>> 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
> However Phar::canWrite(); merely tells me if the readonly ini choice is
> flipped, correct? It doesn't relate to the actual file. While the
> is_writeable function will do what I want for a specific file - I agree
> that a $phar->isWriteable() method would be a nicer (and more intuitive
> for the OO nuts) option.
You are right about Phar::canWrite() however there is also
Phar::isWriteable(). All we have to do is overload it correctly so that it
returns SplFileInfo::isWriteable() && Phar::canWrite().
>> 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');
> I don't know about you, but the common way I work with archives is to
> build a directory structure and just shove it into an archive wholesale
> - although the second option allows this - it's very verbose! Any
> chance of a $phar = new Phar('/path/to/phar', '/path/to/archive');
> option for the lazy? Or even a shortcut static
> Phar::create('/path/to/phar', '/path/to/archive') - and no I don't want
> to have to do an iterator too unless I'm worried about filtering what's
> going in, extra steps are usually not a good thing. the option to have
> it is good, the choice to force it is not.
We liked the Phar::buildFromIterator() approach a lot :-)
Best regards,
Marcus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Double quoted NOWDOC is HEREDOC
Hi Lars, On Sat, Mar 22, 2008 at 9:17 PM, Lars Strojny <[EMAIL PROTECTED]> wrote: > Hi, > > as we introduce NOWDOC in 5.3 it would be logical to allow a double > quoted syntax sister of NOWDOC which acts as HEREDOC (same as for $var = > "$var" vs $var = '$var'). Currently we have the following options: +1 for the record here, I was already in favour of having that in 5.3 in the previous thread. And to show how great is our new wiki, Lars created a RFC as well: http://wiki.php.net/rfc/heredoc-with-double-quotes Thanks for your work! Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] phar API
Hello Elizabeth,
Friday, March 28, 2008, 7:50:28 PM, you wrote:
>>> The only things I see missing are an addFile and addDirectory shortcut
>>> methods for when I don't want to mess with all the metadata or creating
>>> iterators.
>>
>> addDirectory() is called createDirectory() in my RFC, just ignore the
>> return value. What would you like addFile() to do? Adding an empty file?
>>
>> cu, Lars
> Not add an empty directory - just add a directory and its contents much
> like the Phar::create would do - an assumption that you want everything
> in that directory inside with no iterators or magic required.
> addFile(filename) would just add a file - no thinking required, although
> if there was an addDirectory you wouldn't really need it.
> I'm just of the opinion that the less I have to write for common actions
> the better.
> $phar = new Phar('/path/to/phar');
> if($phar->isWriteable())
> {
> $phar->addDirectory('/path/to/some/stuff');
> }
A method named AddDirdectory() sounds very confusing to me as to me it
would just add an empty directory. So the thing you proposed should be
named addFromDirectory() which would be in line with buildFromIterator().
And in the end you could just use that with a DirectoryIterator.
$phar = new Phar('/path/to/phar');
if($phar->isWriteable())
{
$phar->buildFromDirectory(new DirectoyIterator('/path/to/some/stuff'));
}
Now I am wondering only why we didn't do buildFromIterator() as a static
factory method and have the current one as addFromIterator().
Maybe you guys also want the *FromDirectory() versions?
Best regards,
Marcus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Richard's lack of understanding of instances vs classes.
Hello Richard, Friday, March 28, 2008, 6:27:24 PM, you wrote: > On 27/03/2008, Lokrain <[EMAIL PROTECTED]> wrote: >> Hello Internals, >> >> This discussion was very interesting to me so I made some research about all >> languages OOP. >> Each time I saw definition of public, protected, private there was an >> explanation which never >> mentioned instances, but classes. I certainly thought that Richard is right >> saying: >> >> >> Surely it shouldn't work at all unless the $foo === $this? >> >> >> >> I was even amazed that I haven't thought about this ever...and the >> conclusion of my research >> is that as, like Stanislav said, this keywords(public, etc) are for classes >> not for instances... >> >> I learned something new today :) Thanks for this discussion. >> >> Best Regards, Dimitar Isusov >> > My confusion was that I assumed public/protected/private related to > instances and not classes. Whilst I accept that this is the way it is, > it doesn't FEEL right that one instance of class foo can call a > protected member of class bar because class bar extended class foo > along the way. > If they were static calls, that SORT of makes more sense to me. > I suppose this lack of understanding comes from only being self-taught. Having learned all the theory behind helps a lot. And I agree when just scimming over the OOP stuff you easily might get the impression that dynamic means instance and static means class. But it really all is about the class. Just think of it as you never provide a keyword to an instance and all you want it to control which programmer is allowed to touch/use what. Best regards, Marcus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Double quoted NOWDOC is HEREDOC
Hi Felipe, ..., Am Freitag, den 28.03.2008, 20:28 -0300 schrieb Felipe Pena: > 2008/3/24, Marcus Boerger <[EMAIL PROTECTED]>: [...] > Here's a possible fix: > - http://felipe.ath.cx/diff/double-quote-heredoc.diff Tested your patch a bit further and it seems to work fine. Created a few other test cases. Can we close the voting until 00:00h GMT, April, 2nd 2008? If there is a majority for the syntax until then, I will commit the patch and the test cases. cu, Lars signature.asc Description: Dies ist ein digital signierter Nachrichtenteil
Re: [PHP-DEV] Re: phar API
Hi Marcus, warning I smashed those two replies together into this
> You are right about Phar::canWrite() however there is also
> Phar::isWriteable(). All we have to do is overload it correctly so that it
> returns SplFileInfo::isWriteable() && Phar::canWrite().
Sounds good - one gripe fixed ;)
> A method named AddDirdectory() sounds very confusing to me as to me it
> would just add an empty directory. So the thing you proposed should be
> named addFromDirectory() which would be in line with buildFromIterator().
That makes sense
> And in the end you could just use that with a DirectoryIterator.
>
> $phar = new Phar('/path/to/phar');
> if($phar->isWriteable())
> {
> $phar->buildFromDirectory(new
DirectoyIterator('/path/to/some/stuff'));
> }
I kind of feel like I'm talking to a brick wall over this - you also
mentioned
> We liked the Phar::buildFromIterator() approach a lot :-)
That's nice for you, I don't appreciate it being my only option. I'm
not a fan for normal use cases (might be useful if I want to pass a
filteriterator in there to manipulate what I'm shoving in) However that
is just not a normal use case for me. I'd venture to say most people
won't need it and will find it annoying to not have an easier option.
>
> Now I am wondering only why we didn't do buildFromIterator() as a static
> factory method and have the current one as addFromIterator().
>
> Maybe you guys also want the *FromDirectory() versions?
The buildFrom* versions as static factories
and addFrom* versions work on the object - sounds like a winner to me
Thanks,
Elizabeth
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] CVS Account Request: alexxed
Submitting translated documentation -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] practical phar considerations
Lars Strojny wrote:
Hi Greg,
Am Freitag, den 28.03.2008, 17:13 -0500 schrieb Greg Beaver:
[...]
In addition, this would mean that offsetGet() would have to create a
directory. Why?
$phar['full']['path']['to']['file'] = 'hi' calls offsetGet() with
'full', 'path', and 'to', and offsetSet() only with 'file'.
I know that and I discussed that with Marcus beforehand. I don't think
that implicitly creating the directory is much an issue. If somebody
does not want that, he can just use isset() before.
Hi Lars,
If one uses file_put_contents('/path/to/this/file', 'hi') and
'/path/to/this' does not exist, there is an error. The same is true of
fopen, regardless of mode. mkdir() even fails unless the recursive
parameter is explicitly specified. I do not think that this behavior
should be different in ext/phar, especially because all that is needed
is to create 1 directory (full/path/to/) and add the file.
Incidentally, you cut out the portion of my objections that related to
the creation of many unnecessary entries in the phar archive. It is
important to understand that every unnecessary entry in a phar archive
causes phar to do extra processing on loadup, which ultimately increases
the latency (this will be true until we implement caching of phar
manifests, which will be relatively easy to do, I think). In addition,
the performance of:
$phar['long/path/to/subdir/file'] = 'hi';
compared to:
$phar['long']['path']['to']['subdir']['file'] = 'hi';
is significantly slower. Each [] results in a call to offsetGet and
instantiation of a new PharFileInfo object plus the final offsetSet call.
In other words, the proposed change is much more complex, introduces
magic and counterintuitive behavior inside the extension. For instance,
let's say we have this code:
if phar.readonly=1, the second line would throw an exception for
attempting to create the directory "dri"
Wouldn't the second line throw an exeception anyway because the index is
not defined? And: can't offsetGet() check weither Phar::canWrite() is
true?
Yes, but if you're attempting to access the "dir" directory, an
intuitive exception would be "dri not found" instead of "write access is
disabled, cannot create dri directory." If I got the write access
error, I would report a bug, since I was not trying to write anything.
What about this?
The above code would first create the dri directory, and then erase it.
No, it would not. Please take a look at
http://lars.schokokeks.org/php/phar.phps and
http://lars.schokokeks.org/php/phar.php
You're right, chock this up to jet lag on my part. :)
In other words, the proposed syntax is not a good idea.
Two of three assumptions were not correct or at least arguable, so I
can't agree that easily.
Only 1 was incorrect, but my objection still holds on a fundamental
level: offsetGet() should not attempt to write anything.
However, let's examine the feature from another perspective - I think it
could be useful as a read access method, for several reasons:
1) foreach($phar as $name => $file)
This construct does not iterate over every file in the phar archive, but
instead iterates over only those files in the top-most directory. If a
directory returned implemented ArrayAccess and Iterator, one could treat
it like a phar object. Currently this isn't possible. This means
ultimately one should be able to do:
foreach($phar['subdir'] as $name => $file)
or
foreach($phar['subdir/anothersubdir'] as $name => $file)
or
foreach($phar['subdir']['anothersubdir'] as $name => $file)
2) Some might find it easier to do something like:
$phar['subdir/with/really/long/path/file1.txt'] = 'a';
$subdir = $phar['subdir/with/really/long/path'];
$subdir['file2.txt'] = 'a';
$subdir['file3.txt'] = 'a';
Incidentally, if you want to know a *real* hack in the implementation of
ArrayAccess, you can make a directory now with this code:
$phar['subdir/with/really/long/path/'] = 0;
Needless to say, that probably should be removed as an option :).
Could you live with $phar['this']['thing'] as a read access API
(offsetGet only)?
2) Names must be very carefully considered in relation to existing APIs
and the pre-existing methods.
Phar::create(), for instance, should be named buildFromDirectory() to
match buildFromIterator().
I agree, that this name is much better.
what benefit is there in adding complexity of remembering the Phar::GZ
constant over setCompressedGZ()? We have to think hard about changes
like this.
setCompressedGZ() is just not the way object oriented APIs should
behave, as it taints the API with concrete implementation details
("there can be gzip compression and every compression algorithm is
exposed" opposed to "there can be compression, one variant is gzip").
Phar::compress() indicates that "the archive can be compressed" with the
concrete algorithm as a parameter. What about in 5 years when a new,
free compression algorithm rises, which fixes the pro
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote: Hi Marcus, Sorry, if I broke something in latest patch, I just didn't see any comments from others about it. And from my point of view the patch did unnecessary things. I still think the same, becaus I mainly changed code that checks for wrapper in given argument, but not in include_path. BTW: I might miss something. Greg, could you please make a patch that fix the problem that Marcus describes (don't forget a test case). Can you elaborate on the problem you want me to fix? I thought Marcus was saying that changes made to my last patch in the ultimate commit were the problem? Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] practical phar considerations
Hi Greg,
Am Samstag, den 29.03.2008, 17:58 -0500 schrieb Greg Beaver:
[...]
> If one uses file_put_contents('/path/to/this/file', 'hi') and
> '/path/to/this' does not exist, there is an error. The same is true of
> fopen, regardless of mode. mkdir() even fails unless the recursive
> parameter is explicitly specified. I do not think that this behavior
> should be different in ext/phar, especially because all that is needed
> is to create 1 directory (full/path/to/) and add the file.
I mostly agree. Except that the whole argument chain applies to the
current API. You mentioned mkdir("foo/bar/baz") triggering an error
without the recursive argument, but Phar['foo/bar/baz'] does not.
[... Performance concerns ...]
> $phar['long/path/to/subdir/file'] = 'hi';
>
> compared to:
>
> $phar['long']['path']['to']['subdir']['file'] = 'hi';
>
> is significantly slower. Each [] results in a call to offsetGet and
> instantiation of a new PharFileInfo object plus the final offsetSet call.
Could you estimate how hard that performance hit would be?
[]
> Yes, but if you're attempting to access the "dir" directory, an
> intuitive exception would be "dri not found" instead of "write access is
> disabled, cannot create dri directory." If I got the write access
> error, I would report a bug, since I was not trying to write anything.
The exception message would be far away from being optimal, yes.
[...]
> Only 1 was incorrect, but my objection still holds on a fundamental
> level: offsetGet() should not attempt to write anything.
>
> However, let's examine the feature from another perspective - I think it
> could be useful as a read access method, for several reasons:
>
> 1) foreach($phar as $name => $file)
>
> This construct does not iterate over every file in the phar archive, but
> instead iterates over only those files in the top-most directory. If a
> directory returned implemented ArrayAccess and Iterator, one could treat
> it like a phar object. Currently this isn't possible. This means
> ultimately one should be able to do:
[... Code snippets ... ]
I would like that, yes.
> 2) Some might find it easier to do something like:
>
> $phar['subdir/with/really/long/path/file1.txt'] = 'a';
> $subdir = $phar['subdir/with/really/long/path'];
> $subdir['file2.txt'] = 'a';
> $subdir['file3.txt'] = 'a';
>
> Incidentally, if you want to know a *real* hack in the implementation of
> ArrayAccess, you can make a directory now with this code:
>
> $phar['subdir/with/really/long/path/'] = 0;
>
> Needless to say, that probably should be removed as an option :).
Yes, this shouldn't be allowed while
$phar['foo/bar/baz'] = new DirectoryIterator(); could be allowed.
> Could you live with $phar['this']['thing'] as a read access API
> (offsetGet only)?
I'm not sure if I would vote for a partial solution. I think - also I
really do think the current ArrayAccess API is cluttered - it is as
least consistent, which is a plus.
[...]
> As I was trying to say in the portion you quoted below, Phar::compress()
> is already used to compress the entire archive, and should not be
> overloaded. I would be happy with these method names:
>
> Phar::compressArchive() /* Steph hated this when I originally proposed
> it, perhaps she's changed her mind? */
I like it. And much better, it does not contain the compression
format :-)
> Phar::compressAllFiles()
Wouldn't compressFiles() be fine?
> Phar::uncompressArchive()
> Phar::uncompressAllFiles()
Same here, maybe uncompressFiles()
> If I need to explain what the two methods do, then we need to choose new
> names, could you react to these names and let me know what you think
> they would do?
compressAllFiles()/uncompressAllFiles(): compress all the files
contained in the archive
compressArchive()/uncompressArchive(): Compress the archive
[...]
> Lars, I also think it might help to understand my reaction better that
> we've been publicly discussing the API of compression and other
> phar-related issues on pecl-dev for a while, with no feedback.
I would not care so much for phar, if it's inclusion in core has not
been targeted.
> To suddenly see an RFC with major changes without having even been notified
> was annoying, I hope that in the future you will at least contact the
> leads of the project you are proposing changes for so that minor
> disagreements can be worked out in private.
On http://pecl.php.net/package/phar Marcus is listed as one of the two
project leads. I've talked to him *before* writing the RFC on friday. So
this accusation fizzles out. I sort of understand your irritation, but
from my point of view improvement takes place constantly. In the time
Phar was discussed on pecl-dev I was not subscribed to it, now I am. So
the next time I will come up earlier most presumably.
[...]
> Do you want to take a crack at a patch for any of these API features
> once they are worked out (buildFromDirectory should be an easy one, if
> you wou
