Add an ArchiverZip class to support .zip files in update manager

Well, correct me if I'm wrong, but there seems indeed to be no support for actually handling zip-files yet.

Evidence:
- modules/system/system.archiver.inc only implements the tar-archiver which lives in modules/system/system.tar.inc. This class doesn't handle zip-files, only gzipped tarballs.
- also, in modules/system/system.api.php (line 2634...) there is no mention of a zip archiver class:

/**
 * Declare archivers to the system.
 *
 * An archiver is a class that is able to package and unpackage one or more files
 * into a single possibly compressed file.  Common examples of such files are
 * zip files and tar.gz files.  All archiver classes must implement
 * ArchiverInterface.
 *
 * Each entry should be keyed on a unique value, and specify three
 * additional keys:
 * - class: The name of the PHP class for this archiver.
 * - extensions: An array of file extensions that this archiver supports.
 * - weight: This optional key specifies the weight of this archiver.
 *   When mapping file extensions to archivers, the first archiver by
 *   weight found that supports the requested extension will be used.
 */
function hook_archiver_info() {
  return array(
    'tar' => array(
      'class' => 'ArchiverTar',
      'extensions' => array('tar', 'tar.gz', 'tar.bz2'),
    ),
  );
}

Maybe adding a zip archiver class was forgotten after this #604618: Create a common interface for Archive operations so we can handle .zip, .tar.gz. was closed?

Please support .tgz archives too.

Thanks,
XMarket

That's by design. For now, core only supports .tar, .tar.gz, and .tgz. There's plumbing to let contrib support .zip if someone wanted (perhaps via Update advanced):

#604618: Create a common interface for Archive operations so we can handle .zip, .tar.gz.
#605918: Port update manager to use the new Archiver class, not directly Archive_Tar

If we wanted to natively support .zip in core, someone needs to add an ArchiverZip class that implements the ArchiverInterface, e.g. in modules/system/system.archiver.inc.

If y'all want that in D7, great, just say so. Otherwise, feel free to move this to D8.

Note, even if we had an ArchiverZip, this wouldn't work so long as file_save_upload() is broken via #693084: Regression: file_munge_filename() extension handling broken by move to File Field.

Untested, but something like this should do.

@xmarket: see #700592: ArchiverTar class supports .tgz but we don't recognize it

Okay, tested this one and it actually works to install new modules. ;) Our ArchiverInterface is a bit stupid when it comes to the listContents() function. There's a call site in update.manager.inc that expects the array in a certain format (the one returned by tar), but the interface docs don't clearly specify what the array returned by this method is supposed to look like. See #700686: ArchiverInterface::listContents() doesn't match documentation and doesn't make sense... If that patch lands, this will need a slightly different (and simpler) patch.

So, if #700686: ArchiverInterface::listContents() doesn't match documentation and doesn't make sense lands, all we need in ArchiverZip is this:

  public function listContents() {
    $files = array();
    for ($i=0; $i < $this->zip->numFiles; $i++) {
      $files[] = $this->zip->getNameIndex($i);
    }
    return $files;
  }

I'm not going to attach that as a patch yet, since that'll confuse the bot, but it'll be easy to re-roll this as needed.

Guess I'm working on this, so I should assign myself. ;)

Yay, webchick committed #700686: ArchiverInterface::listContents() doesn't match documentation and doesn't make sense, so here's a re-roll of #6 using the function from #7.

argh, stupid patch. basically had the diff applied twice. Try this.

Based on review from chx in IRC:

A) added @link http://php.net/zip

B) added a @todo about the Exception in the constructor when open() fails.

Also, while I was working on these, I noticed upon closer inspection that ZipArchive::open() returns error codes on failure, not just FALSE, so we need to test for !== TRUE, not just !.

I guess that works.

+++ modules/system/system.archiver.inc	1 Feb 2010 08:00:42 -0000
@@ -70,3 +70,66 @@ class ArchiverTar implements ArchiverInt
+      // @todo: This should be an interface-specific Exception some day.

'Exception' should be 'exception'?

I made the small change pointed out in #12 and committed this patch to CVS HEAD. Thanks!