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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Active » Needs review
FileSize
archiver.patch10.47 KB
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 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
CreditAttribution: catch 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
CreditAttribution: catch commented
Issue tags: +Quick fix, +Novice
ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff-2235099.txt2.31 KB
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 🇨🇦
CreditAttribution: webchick 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
CreditAttribution: catch 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.