Add Archive Tar to Drupal

+1 Very useful library for many purposes.

Question: If we need to change it to decouple it from PEAR anyway, can or should we make it conform to our coding standards as well? I'd volunteer to do that, if we want to do it.

+1 subscribe ;)

This would really help to make the Plugin Manager OS and platform independant.

We should really fix the coding standards of this (that's a good idea, sun). Also, I've re-written a lot of this code to make it much much simpler - not the untarring part, but the tarring part. If we want to include that part, let's use my part because it would get rid of a lot of code.

The code lives in http://drupal.org/project/features - you pass it an array of filename=>file and it works. Much of this code could also be simplified because it stays the same ($binary_data_last, most of $binary_data_first, $checksum, the beginning of $tar). We should also do something better about gzipping- theres no real way to know if it was gzipped without checking function_exists('gzencode')

function features_tar_create($files) {
  $tar = '';
  foreach ($files as $name => $contents) {
    $binary_data_first = pack("a100a8a8a8a12A12",
      $name,
      '100644 ', // File permissions
      '   765 ', // UID,
      '   765 ', // GID,
      sprintf("%11s ", decoct(strlen($contents))), // Filesize,
      sprintf("%11s", decoct(time())) // Creation time
    );
    $binary_data_last = pack("a1a100a6a2a32a32a8a8a155a12", '', '', '', '', '', '', '', '', '', '');

    $checksum = 0;
    for ($i = 0; $i < 148; $i++) {
      $checksum += ord(substr($binary_data_first, $i, 1));
    }
    for ($i = 148; $i < 156; $i++) {
      $checksum += ord(' ');
    }
    for ($i = 156, $j = 0; $i < 512; $i++, $j++) {
      $checksum += ord(substr($binary_data_last, $j, 1));
    }

    $tar .= $binary_data_first;
    $tar .= pack("a8", sprintf("%6s ", decoct($checksum)));
    $tar .= $binary_data_last;

    $buffer = str_split($contents, 512);
    foreach ($buffer as $item) {
      $tar .= pack("a512", $item);
    }
  }
  if (function_exists('gzencode')) {
    $tar = gzencode($tar);
  }
  return $tar;
}

If that part of the code can be simplified that much, untarring can be probably simplified too. I'm going to try it - gimme a few hours.

Oh, i haven't tested mine on windows.

This would make life a lot better over at #395472: Plugin Manager in Core: Part 1 (backend)...

I'm not convinced it makes sense to "fix" the code by porting to our coding standards. That just makes it basically impossible to fold in upstream changes in the future in any sane, automated way. Either we're including PEAR's tar library with an eye towards being able to merge in changes later, or we're completely forking it and we're totally on our own to support it indefinitely. There are pros/cons to each approach, but we should be clear what our intentions are before we go further...

That said, if Dmitri wants to provide the Drupalified, simplified fork for review, we'll have something concrete to consider.

Chx has convinced me. Four +1's and another patch is blocked by this. RTBC. We can replace it later like we replaced simpletest.

Yeah, personally I agree with dww and chx - leave it as is, until I finish fixing it (its going very well).

We've adopted other libraries before -- e.g. SimpleTest. I'd be in favor of taking the parts we need (i.e. remove tarring, keep untarring), and adjusting the coding style to ours. I'll have to study the Pear library some more but as a first comment, I think it is worth exploring more.

The alternative is to rely on any of the PHP libraries: http://be2.php.net/manual/en/refs.compression.php.

At least one of my modules could use the tarring feature. So, in the meantime, I agree with others in this issue to do it similarly to SimpleTest:

1) Use and commit the (almost unmodified) library as a starting point now.

2) Implement our own tests.

3) Write our own implementation, specific to our needs, adhering to our coding standards.

That allows us to move forward in other areas.

http://drupal.org/node/487162 after this is committed

@dww is right on the money.

Converting to our "standards" away from the PEAR standard (which is used in many organizations and frameworks outside of PEAR as well), doesn't really buy us much IMO.

And then every time we want to incorporate changes from upstream we end up with a 1000line patch.

For me, one of the main tenets of open source software is to collaborate. By taking it in and messing with it, we then have to maintain it, which means we are duplicating effort instead of collaborating with the developer. We make the code in a sense less free by doing this, because now we are dividing the community using it.

In Drupal generally, I notice a trend of wanting to rebuild everything to make it more "drupally". But look at successful projects like GNU/Linux and Ubuntu. If you look at any level of Linux or Ubuntu you'll find packages interoping with eachother using a wide array of languages, coding styles, architectures, etc. This means that it is able to grow. If Cannonical decided that we only accept packages which use GTK and Gnome, it would either have to message and then maintain a ton of code, or limit its growth.

In this case, if you really feel the need to provide an interface layer on top of it, just extend it. It's written in a clean OOP style, so this can be done easily.

my 4c on a topic I feel kinda strongly about.

Best,
J

[12:10pm] chx: JacobSingh_: my problem is that your comment in the Archive_Tar is also about *rewriting* Archive_Tar not about adding it. Do you support adding Archive_Tar being added to core?
[12:10pm] JacobSingh_: oh, absolutely
[12:10pm] chx: throw out unneeded words
[12:11pm] chx: JacobSingh_: and where does this "oh absolutely" in your followup?

JacobSingh: Oh, Absolutely

I'd really like some other people to chime in on this, to be honest. #487162: Better archive looks really compelling to me as well. Let's get some more opinions, and then I'll pull the trigger on either this patch or #487162. Thanks!

I think we should get this in first. #487162: Better archive is virtually untested except on my laptop, so it's far from being ready - for example, I highly doubt it works on Windows. This patch is blocking #395472: Plugin Manager in Core: Part 1 (backend), and so it's important to get this in ASAP, while the other can wait.

POTX could also benefit from beeing able to tar files out of the box... we could implement the mode=multiple feature if we are able to tar the bunch of splitted POT files.

dmitrig01's code in that issue is obviously more drupalish and, in long-term, most probably also a better fit for us.

However, while this is bloated, is has been tested millions of times with different php versions, configurations, OS's and so on.

To quote from the linked issue:

This is only tested on my macbook MAMP, so it probably doesn't work cross-platform. Still needs commenting, tests, and testing.

And this will take a bit time. If we have tar support, we can continue with the plugin manager patches and if we feel that #487162: Better archive is ready, replace that patch with it. that shouldn't be hard.

Just my $0.02 ;)

I spent some time reviewing http://pear.php.net/package/Archive_Tar and some of the rest of the PEAR site. Some interesting things to note:

* The list of PEAR projects that depend on this library include PEAR itself. Both PEAR and Archive_Tar's downloads are around 200K (I'm guessing they have similar problems as Drupal.org in figuring out their actual usage stats since that seems quite low).
* It has commits as recently as 2 months ago, latest release in March 2009.
* Their oldest bug report is exactly 666 days old today. ;) While this is humorous, it also means the project has been around for a few years and so has some legs.

I fought tooth and nail against forking SimpleTest (just spent 20 minutes looking for the issue but didn't find it :() for the same reasons dww and Jacob Singh outlined. In the end, I lost that battle; the reason being that there were bugs in the underlying framework we needed to be able to fix and they were masked with how complex the SimpleTest library was, so we had no choice but to re-write and "Drupalfy" it.

However, as a result, the Drupal 7 release cycle has seen literally hundreds of Drupal developer hours (and really good Drupal developers' development hours) on our forked testing framework, which is rapidly falling behind SimpleTest "proper," despite dedicating these resources.

Seeing more Drupal developers burning hours on code that is running on hundreds of thousands of implementations already and is required to run PEAR in the first place (meaning it's wide-spread and widely tested in tons of distributions) fills me with a particular sense of dread.

IMO, put it in in its entirety with zero modifications, and let the PEAR folks worry about plumbing while we make the house gorgeous and awesome. :)

Alright, thank for some of the extra input. I've committed it to CVS HEAD. Let's keep rollin' ...

Why is this in system module and not in an file in /includes/?

It would be helpful if information about this was added to http://drupal.org/update/modules/6/7, so core and contrib maintainers know that we now have a tar lib and how to use it.

Also, it seems like the code won't work without Zlib. Is this true?

I added a section about this library to the module upgrade guide: http://drupal.org/update/modules/6/7#archive-tar ... I didn't see any usage examples in the Drupal source (yet), so I just put in links to a good external article with usage examples.

I don't know about Zlib, so I didn't put anything in about that. If this is true, someone should probably make a note of it on the upgrade page.

Marking "fixed" pending someone else thinking the doc is wrong or insufficient in some way.

I believe it's part of UNSTABLE-8 and not of UNSTABLE-9.

OK, it's in UNSTABLE-8 now.