This should never have been put in Component.

Two issues exist to try to clean it up, but even with them it's not clear whether it could stay in Component.
#2043773: Replace php function wrappers in file.inc with a Drupal\Core\File\FileSystem class
#1673246: Adopt Lootils\Archiver

CommentFileSizeAuthor
#5 drupal-archiver-5.patch10.08 KBParisLiakos
#5 interdiff-2235099.txt2.31 KBParisLiakos
#1 archiver.patch10.47 KBcatch

Comments

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new archiver.patch10.47 KB
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

It would be handy in /vendor, but...

https://github.com/pear/Archive_Tar/blob/master/Archive/Tar.php#L42

Ouch.

catch’s picture

catch
he/him
Primary language English
commented

Also we deliberately forked this when it first went in to use the drupal_* wrappers, pulling in the unforked PEAR library into vendor would be a functional regression.

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Quick fix, +Novice
ParisLiakos’s picture

ParisLiakos commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new interdiff-2235099.txt2.31 KB
new drupal-archiver-5.patch10.08 KB

yes lets move it already. like catch said removing the drupal_* wrappers there is a functional regression. and well with those there, they cant be in component, because the dependencies of those wrappers go down all the way to our stream wrapper system.

We need to open a followup to fix coding standards in ArchiverTar.php though.

I just rerolled it now, and replaced Implements... with {@inheritdoc} since we are touching them already

The last submitted patch, 1: archiver.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x, but this really feels like stretching the definition of a critical. :)

  • Commit 7a8d22e on 8.x by webchick: 
    Issue #2235099 by ParisLiakos, catch: Move Archiver to Drupal/Core.
catch’s picture

catch
he/him
Primary language English
commented

@Angie it's actually the last issue from #1929270: [meta] Drupal-agnostic components should not be calling Drupal functions and allowed me to downgrade #2043773: Replace php function wrappers in file.inc with a Drupal\Core\File\FileSystem class to major.

The Component directory is a 'feature' of re-usable libraries. If you can't re-use them, then it is completely pointless to have the two directories.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.