backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Update Archive_Tar to release 1.5.0. #6446

Closed avpaderno closed 6 months ago

avpaderno commented 7 months ago

This is the equivalent of https://www.drupal.org/project/drupal/issues/3429165 for Drupal 7.

Essentially, there is a new Archive_Tar release (1.5.0) which slightly change the permissions used for the created temporary files. Backdrop uses release 1.4.9 created on December 4, 2019.

avpaderno commented 7 months ago

The changes between 1.4.9 and 1.5.0 are not just the permissions used for files, though. A private method has been renamed too, but since that is a private method, the change is backward-compatible.

indigoxela commented 7 months ago

@kiamlaluno many thanks for working on this. :pray:

A quick (off topic) question: as this is 3rd party code, we could add a // phpcs:ignoreFile to stop phpcs nagging on it. Or we could add an exclusion via commandline option. Which one's preferred?

avpaderno commented 7 months ago

In this case, phpcs:ignoreFile seems a better solution, IMO. At least, if the core/modules/system/system.tar.inc file were removed, it would not be necessary to edit a configuration file.

I can update the PR.

avpaderno commented 7 months ago

As expected, all checks have passed.

I also added a comment showing as Backdrop addition the line with the PHP_CodeSniffer directive.

// Backdrop addition Ignore PHP_CodeSniffer warnings/errors for this file.
// phpcs:ignoreFile
indigoxela commented 7 months ago

The only usage I'm aware of in core is full config export and import, so I tested with that:

Works for me, no problems (as expected). :+1:

argiepiano commented 7 months ago

If possible this should be tested with backup_migrate, since that module makes use of Archive_Tar. I could try this... but not in the near future.

indigoxela commented 7 months ago

... this should be tested with backup_migrate

Absolutely, many thanks for the hint.

EDIT: Wait, the "files" backup's actually using tar. Back to testing... :wink:

So that also still works properly. :+1:

argiepiano commented 7 months ago

Thanks so much, @indigoxela! If it's not too much to ask, could you try a backup of the DB and config files? (instead of just a DB). Thanks again

indigoxela commented 7 months ago

... could you try a backup of the DB and config files?

Sure, I'm still there, no effort. :wink:

First I tried an active config backup and restore (with checking again on commandline, that the .tar.gz is correct). Changed a Layout before restoring and after restore the initial layout setup was back.

Works.

Then a combined db dump and config backup and restore (again verified file on commandline).

Works.

The dblog's always wiped after db restore (as expected), but I had error display turned on - nothing suspicious showed up. (Tested on PHP 8.2 as usual.)

argiepiano commented 7 months ago

Wonderful. Thanks!

herbdool commented 7 months ago

RTBC. This is also RTBC in D7.

quicksketch commented 6 months ago

Thanks @kiamlaluno, @indigoxela, @argiepiano, and @herbdool! I merged https://github.com/backdrop/backdrop/pull/4689 into 1.x and 1.27.x.