phar API - Externals

18 years ago by Lars Strojny — view source — reply

unread

Hi Benjamin,

Am Donnerstag, den 27.03.2008, 14:34 +0100 schrieb Benjamin Schulz:

Hi,
i just read the phar examples in the manual and found things like this:
[...]
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.

ArrayAccess is not hackish per se, but it just does not feels right for
working with archives.
[...]

$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)
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();
}
 // add the file to the archive
 $p->addFile($hugeFile, 'data/hugefile.dat');

 // 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(...);
}

I second your proposal, but a bit more flexible:

// Creating files
$phar = new Phar();
$file = $phar->createFile('/filename'); // Phar acts as a factory and returns an object PharFile or something
$file->setContent('string'); // Polymorph signature, can either take a string ...
$file->setContent(fopen('file', 'r')); // ... or a resource
$file->setContent('foo', $data); // ... or a string and data
$file->setContent('bar', array('line1', 'line2')); // ... or an array of lines
$file->setContent('baz', fopen('file', 'r')); // ... or a name and a resource
if (!$phar->isReadonly()) {
$phar->save(); // Writes the newly create file to the filesystem
}

$phar = new Phar();
$phar->add('foo');
$phar->add('bar', $data);
$phar->add(new SplFileInfo('bar'));

Creating directories:
$phar = new Phar();
$dir = $phar->createDirectory('/foo'); // Return PharDirectory object
$dir->add(new DirectoryIterator("mydir")); // Adds every file in the directory mydir
$dir->add(new RecursiveDirectoryIterator('otherdir')); // Adds every item recursivly
$dir->add(new SplFile("foo")); // Adds the file foo
$dir->add('bar'); // Adds bar
$dir->add('baz', $data)
$file = $dir->createFile('bla')
$file->setContent($data);
$dir2 = $dir->createDirectory('foo');
...
$phar->save();

No, with fluent interfaces:
$phar = new Phar();
$phar->createDirectory('foo')
->add('foo')
->add(new SplFileInfo('bar'))
->add('baz', $data);
$phar->save();

if PharFile would include a reference to the original Phar object, the
last Phar::save() could even be included in the chain.

About compression:
I would prefer to have Phar::setCompression(Phar::GZIP) or something
similiar. Or even Phar::setCompressionStrategy(PharCompressionInterface
$interface) but I have the feeling that would go too far :-)

cu, Lars

18 years ago by Marcus Boerger — view source — reply

unread

Hello Lars,

Thursday, March 27, 2008, 3:07:58 PM, you wrote:

Hi Benjamin,

Am Donnerstag, den 27.03.2008, 14:34 +0100 schrieb Benjamin Schulz:

Hi,
i just read the phar examples in the manual and found things like this:
[...]
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.

ArrayAccess is not hackish per se, but it just does not feels right for
working with archives.
[...]
$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)
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();
}
 // add the file to the archive
 $p->addFile($hugeFile, 'data/hugefile.dat');

 // 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(...);
}
I second your proposal, but a bit more flexible:

// Creating files
$phar = new Phar();
$file = $phar->createFile('/filename'); // Phar acts as a factory and
returns an object PharFile or something
$file->setContent('string'); // Polymorph signature, can either take a string ...
$file->setContent(fopen('file', 'r')); // ... or a resource
$file->setContent('foo', $data); // ... or a string and data
$file->setContent('bar', array('line1', 'line2')); // ... or an array of lines
$file->setContent('baz', fopen('file', 'r')); // ... or a name and a resource
if (!$phar->isReadonly()) {
$phar->save(); // Writes the newly create file to the filesystem
}

The above is far too complex. And results in an interface that does not
make clear what it is doing. Right now we have $phar->offsetSet() with a
very clear semantics.

Maybe what we want to add is getContents() and setContents() to SplFileInfo.

$phar = new Phar();
$phar->add('foo');
$phar->add('bar', $data);
$phar->add(new SplFileInfo('bar'));

Yet again, unclear. First probable creates an empty entry, same as
offsetSet('foo', ''); only that one is clear. The second one create an
entry with content, which is exactly what $phar->offsetSet('bar',$data)
does. The third is extremely confusing. What does it do? What will be the
entry name? And will it just take the entry name and make an emoty entry or
will it copy the data from the referenced file?

Creating directories:
$phar = new Phar();
$dir = $phar->createDirectory('/foo'); // Return PharDirectory object
$dir->add(new DirectoryIterator("mydir")); // Adds every file in the directory mydir
$dir->add(new RecursiveDirectoryIterator('otherdir')); // Adds every item recursivly
$dir->add(new SplFile("foo")); // Adds the file foo
$dir->add('bar'); // Adds bar
$dir->add('baz', $data)
$file = $dir->createFile('bla')
$file->setContent($data);
$dir2 = $dir->createDirectory('foo');
...
$phar->save();

We have a very nice Phar::buildFromIterator() for this kind of creation.
We also do not really care for directories. Maybe that is something worth
to add, especially the createDirectory() method.

No, with fluent interfaces:
$phar = new Phar();
$phar->createDirectory('foo')
->add('foo')
->add(new SplFileInfo('bar'))
->add('baz', $data);
$phar->save();

First of all I do not like fluent interfaces at all here. Because it is
unclear what it does. What is the return value of each of these?
Is it the Phar object of the PharEntry or whatever object?

If it is a reference to the entry then you confuse me even more. Because
then wher do all the adds go to? If it is the phar then it makes a bit
more sense and indeed the save() can be put at the end. But in general
fluent interfaces tend to be very unclear and make it harder to maintain
code. Thay allow nice GUI code as they can drop out all the overhead, sure
but here you see me confused.

if PharFile would include a reference to the original Phar object, the
last Phar::save() could even be included in the chain.

About compression:
I would prefer to have Phar::setCompression(Phar::GZIP) or something
similiar. Or even Phar::setCompressionStrategy(PharCompressionInterface
$interface) but I have the feeling that would go too far :-)

Maybe adding Phar::compressAllFiles() would indeed be good. This one could
easily map to the two exiting functions.

Best regards,
Marcus

18 years ago by Lars Strojny — view source — reply

unread

Hi Marcus,

Am Freitag, den 28.03.2008, 11:17 +0100 schrieb Marcus Boerger:
[...]

The above is far too complex. And results in an interface that does not
make clear what it is doing. Right now we have $phar->offsetSet() with a
very clear semantics.

Of course I still prefer
$phar->createDirectory("dir")->createFile("foo")->setContent("bar") I
would like to step back and think about how to make the ArrayAccess API
more convenient. The issue I have with ArrayAccess is, that an archive
represents a multidimensional, nested hash. The way Phar works as an
array is doubtful as it represents multi dimensions in a single
dimension hash.
$phar['path/to/file'] instead of $phar['path']['to']['file'], which
would me semantically more correct.

Maybe what we want to add is getContents() and setContents() to SplFileInfo.

$phar = new Phar();
$phar->add('foo');
$phar->add('bar', $data);
$phar->add(new SplFileInfo('bar'));

Yet again, unclear. First probable creates an empty entry, same as
offsetSet('foo', ''); only that one is clear.

No, it will take the content of the file "foo" from the harddisk and
create it as foo. A shortcut to add('foo', file_get_contents('foo')).

The second one create an entry with content, which is exactly what
$phar->offsetSet('bar',$data) does.

Yes.

The third is extremely confusing. What does it do? What will be the
entry name? And will it just take the entry name and make an emoty entry or
will it copy the data from the referenced file?

It behaves like the first example, only that it uses an SplFileInfo
object.

Creating directories:
$phar = new Phar();
$dir = $phar->createDirectory('/foo'); // Return PharDirectory object
$dir->add(new DirectoryIterator("mydir")); // Adds every file in the directory mydir
$dir->add(new RecursiveDirectoryIterator('otherdir')); // Adds every item recursivly
$dir->add(new SplFile("foo")); // Adds the file foo
$dir->add('bar'); // Adds bar
$dir->add('baz', $data)
$file = $dir->createFile('bla')
$file->setContent($data);
$dir2 = $dir->createDirectory('foo');
...
$phar->save();

We have a very nice Phar::buildFromIterator() for this kind of
creation.

OK, didn't know that. I'm not sure if one could not reuse the
add()-method (or offsetSet() if there is no chance of getting rid of the
ArrayAccess API, $phar["dir"] = new DirectoryIterator(...))

We also do not really care for directories. Maybe that is something worth
to add, especially the createDirectory() method.
[...]

$phar = new Phar();
$phar->createDirectory('foo')
->add('foo')
->add(new SplFileInfo('bar'))
->add('baz', $data);
$phar->save();

First of all I do not like fluent interfaces at all here. Because it is
unclear what it does. What is the return value of each of these?
Is it the Phar object of the PharEntry or whatever object?

createDirectory() returns a phantasy object called PharDirectory,
PharDirectory::add() returns itself. I think it is really readable,
let's go one step further and create a subdirectory

$phar->createDirectory('bla')
->add('file1', 'content')
->add('file2', 'content')
->createDirectory('foo')
->add('file1', 'content')
->add('file2', 'content');
$phar->save();

Creates bla/file1, bla/file2, bla/foo/file1, bla/foo/file2.
[...]

About compression:
I would prefer to have Phar::setCompression(Phar::GZIP) or something
similiar. Or even Phar::setCompressionStrategy(PharCompressionInterface
$interface) but I have the feeling that would go too far :-)

Maybe adding Phar::compressAllFiles() would indeed be good. This one could
easily map to the two exiting functions.

What would compressAllFiles() exactly do? Would there be any ambiguity
with Phar::compress() (if not, why not call it compress)?

cu, Lars

18 years ago by Benjamin Schulz — view source — reply

unread

The issue I have with ArrayAccess is, that an archive
represents a multidimensional, nested hash. The way Phar works as an
array is doubtful as it represents multi dimensions in a single
dimension hash.

Very good point!

$phar->createDirectory('bla')
->add('file1', 'content')
->add('file2', 'content')
->createDirectory('foo')
->add('file1', 'content')
->add('file2', 'content');
$phar->save();

-1, i am not a fan of this lazy stuff :)

Regards,
Benjamin

18 years ago by Marcus Boerger — view source — reply

unread

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

18 years ago by Benjamin Schulz — view source — reply

unread

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

18 years ago by Marcus Boerger — view source — reply

unread

Hello Benjamin,

Thursday, March 27, 2008, 8:58:47 PM, you wrote:

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?

Of course not as you can use offsetSet() already today.

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.

To be precise, the getter returns a SplInfoObject per default. That is not
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)

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?

Hmm, we have is_writeable() which takes the full path (phar+index), so this
is probbaly a good addition. But we also have ArrayAccess/offsetGet return
a SplFileInfo object which supports isWritable(). So what else do you need?
Or is there something briken, which actually might be the case.

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('...');

That is better as this would go well with the internal structures. Adding
this sounds fine with me. Assuming a) createFile() returns the file info
object and you work on that or b) setContent takes the file name, too which
would be offsetSet() then (no aliases for method names as that would be
confusing when overloading comes into play).

a)

$fi = $phar->createEntry('data.dat');
$fi->file_put_contents($data); // this is a change in SPL however

b)

$phar->createFile('data.dat');
$phar->setContent('data.dat', $data);

same as:

$phar->offsetSet('data.dat', $data);

the second is shorter, faster and clearer. As in your example the phar
object would have to keep state of a kind of dangling entry object. That
might get changed when there is something between createFile() and
setContent(). This will never be supported as it is extremely dangerous and
error prone.

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.

But after all a phar object is a container for files. And actually it is an
associative container for files. And each file can be seen as the filename
which is the key plus its data as string or just as a resource. This is
what we try to do and why we chose ArrayAccess semantics. Yet adding one or
the other method probably helps.

Best regards,
Marcus

18 years ago by Greg Beaver — view source — reply

unread

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)

Some notes:

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
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).
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.
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:

file_put_contents('phar:///path/to/archive.phar/index.php', 'file
contents');
$phar = new Phar('/path/to/archive.phar');
$phar->buildFromIterator(new DirectoryIterator('/some/path'), '/some/path');
$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:

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.
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

18 years ago by Elizabeth M Smith — view source — reply

unread

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.

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.

file_put_contents('phar:///path/to/archive.phar/index.php', 'file
contents');

$phar = new Phar('/path/to/archive.phar');
$phar->buildFromIterator(new DirectoryIterator('/some/path'),
'/some/path');

$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.

There's plenty of flexibility here, and no need to be scared of the
arrayaccess option.

It's not so much a question of being scared as a question of retraining

being forced to learn a "new way of doing things" for one extension is
frankly annoying.

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 :).

That sounds like a great idea - the two extensions SHOULD have
compatible API's - however they shouldn't deviate from the basic mental
concept of how files and directories work either. In other words
->addFile(file) and ->addDirectory(do_recursive) and ->addEmptyDir()
would keep people from head scratching. And keep us from having to
create an iterator just to shove a directory into a phar.

Thanks for your hard work Greg - the functionality looks great, I'd just
like a bit of consistency with other extensions and commonly used PHP
archive manipulation classes so retraining isn't in order just to use phars.

Looking forward to more fun!
Elizabeth

18 years ago by Marcus Boerger — view source — reply

unread

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.

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().

file_put_contents('phar:///path/to/archive.phar/index.php', 'file
contents');

$phar = new Phar('/path/to/archive.phar');
$phar->buildFromIterator(new DirectoryIterator('/some/path'),
'/some/path');

$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

18 years ago by Elizabeth M Smith — view source — reply

unread

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

18 years ago by Lars Strojny — view source — reply

unread

Hi everybody,

After having a great discussion with Marcus about the Phar API, I've
tried to sum up the proposed changes (of course with a strong focus on
my opinion about what should be changed :-)). The RFC is not yet
finished and I would like you to comment it beforehand. I've tried to
include as much as possible as discussed/proposed here. Also I would
like to help implementing it (up to the level where my skills end), so
this is not (only) about letting others work.

A few concrete questions:
Marcus, et al, could you please verify that my technical assumptions are
correct?
Elizabeth, et al, the concrete semantic of Phar::isWritable() is what?
Is it just is_writable('test.phar') or is it Phar::canWrite() &&
is_writable('test.phar')?
Benjamin, could you live with the proposed change of the ArrayAccess
usage?

The RFC: http://wiki.php.net/rfc/streamline-phar-api

cu, Lars

18 years ago by Elizabeth M Smith — view source — reply

unread

Elizabeth, et al, the concrete semantic of Phar::isWritable() is what?
Is it just is_writable('test.phar') or is it Phar::canWrite() &&
is_writable('test.phar')?

I'd vote for Phar::canWrite() && is_writable('test.phar') - so you know
if you actually can change the phar.

Benjamin, could you live with the proposed change of the ArrayAccess
usage?

The RFC: http://wiki.php.net/rfc/streamline-phar-api

cu, Lars

After looking over the rfc - the phar as a multidimensional array, the
isWriteable, the createDirectory and the Phar::create would solve most
of my current API gripes for phar - the rest...I don't know what
rewriting all the splfileobject and splfileinfo methods would achieve...

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.

Thanks,
Elizabeth Smith

18 years ago by Lars Strojny — view source — reply

unread

Hi Elizabeth,

Am Freitag, den 28.03.2008, 12:12 -0500 schrieb Elizabeth M Smith:
[...]

I'd vote for Phar::canWrite() && is_writable('test.phar') - so you know
if you actually can change the phar.

OK, fine. That's how I have it in the proposal too, and Marcus already
stated his preference the same way.
[...]

After looking over the rfc - the phar as a multidimensional array, the
isWriteable, the createDirectory and the Phar::create would solve most
of my current API gripes for phar - the rest...I don't know what
rewriting all the splfileobject and splfileinfo methods would achieve...

PharFileInfo is derived from SplFileInfo. Therefore adding
setContent()/getContent() to SplFileInfo would allow doing
setContent()/getContent() with PharFileInfo. Phar is derived from
DirectoryIterator and DirectoryIterator allows to set the info class
(setInfoClass(), the info class must be derived from SplFileInfo).
SplFileObject is derived from SplFileInfo, therefore renaming the would
make the usage of SplFileObject more straighforward, if a user does the
following:

$phar = new Phar("test.phar");
$phar->setInfoClass('SplFileObject');
$file = $phar['file']; // File would be an instance of SplFileObject

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

18 years ago by Elizabeth M Smith — view source — reply

unread

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');
}

done ;)

Thanks,
Elizabeth

18 years ago by Steph Fox — view source — reply

unread

Hi Elizabeth,

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.

Hmm.. and how would you tell the difference between: empty directory
creation, adding an existing directory plus contents, and whether the action
is recursive or not? I think this needs a bit more thought.

addFile(filename) would just add a file - no thinking required, although
if there was an addDirectory you wouldn't really need it.

I think I'm probably missing something here... (haven't read the entire
thread, can you tell?) addFile() in many cases would actually involve more
typing than we do currently via array access...?

I'm just of the opinion that the less I have to write for common actions
the better.

Yes, I'm with you on that part. The main problem is that I don't know if
Greg would want me or Marcus to add functionality (even aliasing-type
shortcuts) this week, while we're in alpha. I rather suspect he'd like to
leave edge-smoothing for 2.1.0 and just get the thing out there and tested
ASAP, otherwise I'd be implementing everything Greg implemented last weekend
right now :-)

We don't actually have missing functionality at this stage... everything
mentioned here boils down to 'prettification'. So while it can definitely go
down as 'wishlist' stuff, it's not actually essential.

Steph

18 years ago by Elizabeth M Smith — view source — reply

unread

Hiya Steph,

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.

Hmm.. and how would you tell the difference between: empty directory
creation, adding an existing directory plus contents, and whether the
action is recursive or not? I think this needs a bit more thought.

empty $phar->addDirectory();
existing dir $phar->addDirectory('/path/to/directory');

Always recursive, if you don't want recursive I'd assume you're doing
something fancy and need a more powerful API

addFile(filename) would just add a file - no thinking required, although
if there was an addDirectory you wouldn't really need it.

I think I'm probably missing something here... (haven't read the entire
thread, can you tell?) addFile() in many cases would actually involve
more typing than we do currently via array access...?

Eh, after further review an addDirectory would do it for me

I'm just of the opinion that the less I have to write for common actions
the better.

Yes, I'm with you on that part. The main problem is that I don't know if
Greg would want me or Marcus to add functionality (even aliasing-type
shortcuts) this week, while we're in alpha. I rather suspect he'd like
to leave edge-smoothing for 2.1.0 and just get the thing out there and
tested ASAP, otherwise I'd be implementing everything Greg implemented
last weekend right now :-)

We don't actually have missing functionality at this stage... everything
mentioned here boils down to 'prettification'. So while it can
definitely go down as 'wishlist' stuff, it's not actually essential.

Steph

No one is complaining about functionality. However I think API issues
are more than "prettification" and "wishlist" - maybe not alpha
important but certainly beta and release - if I'm not going to be doing
fancy manipulation of phars and only want a quick way to create them and
run them (which is true in my case) if the pain of using the extension
outweighs the benefits I won't bother (yes I am lazy) Although I do
agree it doesn't have to be in the alpha, it can wait until Greg gets
back - I just think a quick and easy way to do something common (create
and populate a new phar quickly) would help push popularity, people are
attracted to "easy" and I know I'd love phar to really succeed ;)

Thanks,
Elizabeth

18 years ago by Steph Fox — view source — reply

unread

No one is complaining about functionality. However I think API issues
are more than "prettification" and "wishlist" - maybe not alpha
important but certainly beta and release - if I'm not going to be doing
fancy manipulation of phars and only want a quick way to create them and
run them (which is true in my case) if the pain of using the extension
outweighs the benefits I won't bother (yes I am lazy) Although I do
agree it doesn't have to be in the alpha, it can wait until Greg gets
back - I just think a quick and easy way to do something common (create
and populate a new phar quickly) would help push popularity, people are
attracted to "easy" and I know I'd love phar to really succeed ;)

It doesn't have to wait until Greg gets back, just until Greg answers email
:)

Steph

Thanks,
Elizabeth

18 years ago by Marcus Boerger — view source — reply

unread

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