Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36682
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain strojny.net designates 85.10.204.248 as permitted sender)
To: php-dev <internals@lists.php.net>
Cc: Gregory Beaver <greg@chiaraquartet.net>
In-Reply-To: <47EEC989.5020603@chiaraquartet.net>
References: <47ED6D97.3030109@chiaraquartet.net>
	 <1206788577.3621.15.camel@localhost>  <47EEC989.5020603@chiaraquartet.net>
Date: Sun, 30 Mar 2008 03:19:44 +0200
Message-ID: <1206839984.3621.71.camel@localhost>
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=_milch.schokokeks.org-27416-1206839986-0001-2"
Subject: Re: [PHP-DEV] practical phar considerations
From: lars@strojny.net (Lars Strojny)

--=_milch.schokokeks.org-27416-1206839986-0001-2
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

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=20
> '/path/to/this' does not exist, there is an error.  The same is true of=20
> fopen, regardless of mode.  mkdir() even fails unless the recursive=20
> parameter is explicitly specified.  I do not think that this behavior=20
> should be different in ext/phar, especially because all that is needed=20
> 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'] =3D 'hi';
>=20
> compared to:
>=20
> $phar['long']['path']['to']['subdir']['file'] =3D 'hi';
>=20
> is significantly slower.  Each [] results in a call to offsetGet and=20
> 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=20
> intuitive exception would be "dri not found" instead of "write access is=20
> disabled, cannot create dri directory."  If I got the write access=20
> 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=20
> level: offsetGet() should not attempt to write anything.
>=20
> However, let's examine the feature from another perspective - I think it=20
> could be useful as a read access method, for several reasons:
>=20
> 1) foreach($phar as $name =3D> $file)
>=20
> This construct does not iterate over every file in the phar archive, but=20
> instead iterates over only those files in the top-most directory.  If a=20
> directory returned implemented ArrayAccess and Iterator, one could treat=20
> it like a phar object.  Currently this isn't possible.  This means=20
> ultimately one should be able to do:
[... Code snippets ... ]

I would like that, yes.

> 2) Some might find it easier to do something like:
>=20
> $phar['subdir/with/really/long/path/file1.txt'] =3D 'a';
> $subdir =3D $phar['subdir/with/really/long/path'];
> $subdir['file2.txt'] =3D 'a';
> $subdir['file3.txt'] =3D 'a';
>=20
> Incidentally, if you want to know a *real* hack in the implementation of=20
> ArrayAccess, you can make a directory now with this code:
>=20
> $phar['subdir/with/really/long/path/'] =3D 0;
>=20
> Needless to say, that probably should be removed as an option :).

Yes, this shouldn't be allowed while
$phar['foo/bar/baz'] =3D new DirectoryIterator(); could be allowed.

> Could you live with $phar['this']['thing'] as a read access API=20
> (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()=20
> is already used to compress the entire archive, and should not be=20
> overloaded.  I would be happy with these method names:
>=20
> Phar::compressArchive() /* Steph hated this when I originally proposed=20
> 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=20
> names, could you react to these names and let me know what you think=20
> 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=20
> we've been publicly discussing the API of compression and other=20
> phar-related issues on pecl-dev for a while, with no feedback. =20

I would not care so much for phar, if it's inclusion in core has not
been targeted.=20

> To suddenly see an RFC with major changes without having even been notifi=
ed=20
> was annoying, I hope that in the future you will at least contact the=20
> leads of the project you are proposing changes for so that minor=20
> 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=20
> once they are worked out (buildFromDirectory should be an easy one, if=20
> you would like to start there)?

Yes, I'm willing to help with it to a degree, as I said in a previous
mail.

cu, Lars

--=_milch.schokokeks.org-27416-1206839986-0001-2
Content-Type: application/pgp-signature; name="signature.asc"
Content-Transfer-Encoding: 7bit
Content-Description: Dies ist ein digital signierter Nachrichtenteil

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (GNU/Linux)

iQIcBAABAgAGBQJH7uqwAAoJECQPF+sCY6wHwe8P/0GzD3vFYPotyqU6hT/o9mUS
ODLC1i3Hm6kkMzhD4trIh9/j49IklJV0fcA0VjFTZcENovkkhSBolyy36fb25c9F
9DDZRfT8egfJTLY/LB8jmBdEjWzIRmcAkO82zDZdgTTqBmL+uQD3rY8AXfHq7Nqy
j9Ild5Y/2wuM8X44B4rhpMyfrKG0BzkVhllo4jQKEcg+cYZZSPd3nmYy4P4mttNm
D0j/9hEo6jXfGJfSoWz7x8USueH6IFPvJ+jv1HMCgiWseZLNo2jBx3rSlZC9cWL2
fpu9uk//to7mxCzPrEGL7+ngS9NnSudctrAL+66X4CFJrysnPowb69AUvu/h6vSu
V4xN1XRV/+jEb2SufcMD50LprR2Te5B1sXPAj/ElLhHnmGpscuerjLmUqgKVR6/R
M+OT5ienf/VzMUQQccQU0xPP2O3GvvsM4BN5BYpsVSIICGuXU+uPLZ4jPobQAT4a
Bygu1JPbxADv1/DCBWFNMKCTPEUu/ZSo3euxEFkcyTOq73CmXsV6Y6Cj2dh53CWa
ic7Bn1/jgkiyMeTnTvjYRBFwL2bY7eUm3ycbGAxD+uRJPCr/cGG4MEeOabsE1FoA
zS1iNHXdULtNUzBoPHZjqTspOqfWBu9N3yRVWFk1zqR/pJCsVEPKKf1eeSw+dmXq
6IHiyJzKc3b3AiMtJxGD
=71Xn
-----END PGP SIGNATURE-----

--=_milch.schokokeks.org-27416-1206839986-0001-2--