Drupal 8/9 now include Archive_Tar via composer.
Drupal 7 still has its own copy of most of the code from the third party library in system.tar.inc in the system module.
There's very little if any test coverage of Archive_Tar within core.
I'm not sure about D8/9 but for D7 at least it'd be useful to have some basic integration tests which ensure that when updates to the library are made, we don't introduce regressions in functionality.
FWIW when manually testing the latest updates, I did something like this:
/path/to/drupal-7.x$ wget -P /tmp https://ftp.drupal.org/files/projects/seckit-7.x-1.11.tar.gz
/path/to/drupal-7.x$ drush php
Psy Shell v0.9.9 (PHP 7.2.24-0ubuntu0.18.04.1 — cli) by Justin Hileman
>>> $tar = new Archive_Tar('/tmp/seckit-7.x-1.11.tar.gz');
=> Archive_Tar {#5914
+_tarname: "/tmp/seckit-7.x-1.11.tar.gz",
+_compress: true,
+_compress_type: "gz",
+_separator: " ",
+_file: 0,
+_temp_tarname: "",
+_ignore_regexp: "",
+error_object: null,
+"_fmt": "Z100filename/Z8mode/Z8uid/Z8gid/Z12size/Z12mtime/Z8checksum/Z1typeflag/Z100link/Z6magic/Z2version/Z32uname/Z32gname/Z8devmajor/Z8devminor/Z131prefix",
}
>>> $tar->listContent();
=> [
[
"checksum" => 4636,
"filename" => "seckit/",
"mode" => 493,
"uid" => 0,
"gid" => 0,
"size" => 0,
"mtime" => 1561463898,
"typeflag" => "5",
"link" => "",
],
[
"checksum" => 5694,
"filename" => "seckit/CHANGELOG.txt",
...etc...
...in order to ensure that the basic functionality of the class worked.
Comments
Comment #4
mcdruid commentedSome notes on how D7 uses Archive_Tar:
It's registered as the handler for relevant file extensions via:
hook_archiver_info()The archiver for a given file is retrieved by:
archiver_get_archiver()Two functions in core that actually use it (AFAICS):
update_manager_archive_extract()file_entity_upload_archive_form_submit()Comment #5
mcdruid commentedFirst pass at some very basic tests.
This is a bit crude, but it's a start.
Comment #6
mcdruid commentedErm, forgot the tarball which is part of the tests.
This is going to make the patches ugly.
No interdiff for this one, as that would just be the tarball.
Comment #7
mcdruid commentedAdding tests from a recent security issue. These should be merged with #6.
The patches are again a bit ugly because of the tarballs.
Comment #8
mcdruid commentedThis merges #6 and #7
Not including an interdiff because of the tarballs included in the patches.
Comment #9
mcdruid commentedSmall tweaks to the tests, including a comment which explains the slightly odd behaviour of \ArchiverTar::add a little.
I think these are ready for final review and commit, however I won't RTBC my own patch.
Comment #10
fabianx commentedRTBC + 1
Comment #12
mcdruid commentedComment #13
mcdruid commented