Problem/Motivation

Composer doesn't allow to install this module along backup_migrate v5

Steps to reproduce

composer require drupal/backup_migrate:^5@rc drupal/backup_migrate_flysystem

Proposed resolution

Change dependencies in info.yml file

Remaining tasks

Check that the functionality works by running the tests

I'm not aware of the breaking changes that the 5.0-rc release introduces so I am not sure it will work out of the box.

CommentFileSizeAuthor
#15 3172376-15-drupal_9_compatibility.patch7.42 KBfjgarlin
#9 3172376-backup-migrate-v5-3172376-9.patch5.81 KBrodrigoaguilera
#8 3172376-backup-migrate-v5-3172376-7.patch4.86 KBrodrigoaguilera
#6 3172376-backup-migrate-v5-3172376-6.patch3.89 KBrodrigoaguilera
#5 3172376-backup-migrate-v5-3172376-5.patch2.73 KBrodrigoaguilera
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rodrigoaguilera created an issue. See original summary.

  • JonMcL committed 3b02226 on 8.x-1.x 
    Issue #3172376 by rodrigoaguilera: Add compatibility with backup_migrate...
JonMcL’s picture

JonMcL CreditAttribution: JonMcL commented
Assigned: Unassigned » JonMcL
Status: Active » Fixed

I went with "^4 || ^5"

This should allow you to install version 5 of backup_migrate without forcing it onto others. Version 5 is still not the recommended release for backup_migrate and I haven't really done enough testing.

To install backup_migrate version 5.0.0-rc1 into your site, use:
composer require 'drupal/backup_migrate:^5.0'

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented

Sweet. Thanks!

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented
Version: 8.x-1.3 » 8.x-1.x-dev
Status: Fixed » Needs review
FileSize
3172376-backup-migrate-v5-3172376-5.patch2.73 KB

So the latest release allowed me to test this module alongside backup_migrate 5 and I discovered that there is changes in the namespaces so the module produces fatal errors.

I attach the patch to solve those but from a maintainers point of view I don't know how feasible is to maintain compatibility without using ugly ifs for the use statements.

Probably the best is to revert this and release a 2.0.x branch with the changes for v5 compatibility.

Thanks for the cool module and your efforts!

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented
FileSize
3172376-backup-migrate-v5-3172376-6.patch3.89 KB

I left some parts without editing

JonMcL’s picture

JonMcL CreditAttribution: JonMcL commented
Status: Needs review » Needs work

I knew I needed to do some real testing :)

There are also 2.0.x branches now for the Flysystem modules. I was going to create a new branch to properly support them as well.

I will try and get something committed in the next few days. In the meantime, I will revert the backup_migrate 5.0.x compatibility in composer.json.

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented
FileSize
3172376-backup-migrate-v5-3172376-7.patch4.86 KB

Aaand the latest patch that actually saves the file :)
The only problem I noticed is it doesn't save the metadata (.info file) because the whole saveFile is overridden.

In the meantime I realized I don't even need this module because flysystem already defines stream wrappers that I can use with the "Server File Directory" destination of backup_migrate and it works perfectly. Maybe that is worth mentioning in the description of the module.

Thanks for all the support, I hope the patches are useful

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented
FileSize
3172376-backup-migrate-v5-3172376-9.patch5.81 KB

Hmmm, using just the wrapper lead me to a lot of seek errors so I went back to this module.

There is more calls to underscore methods that I ignored. With this changes is able to read files from a directory.

JonMcL’s picture

JonMcL CreditAttribution: JonMcL commented

Thank you for all the patches so far. I do plan to work on this in the coming days, but might not have a release ready for at least a week.

Yes, I found that the standard stream wrappers created by Flysystem didn't work out and that's why this module was created.

So far I've found some changes with the Backup and Migrate classes and plugins definitions. Some of it might require changes to this module's plugin implementations.

Have you considered just using Backup and Migrate v4? Or is there a v5 requirement that you have?

rodrigoaguilera’s picture

rodrigoaguilera
Primary language Spanish
Location Madrid
CreditAttribution: rodrigoaguilera at Amazee Labs for Amazee Labs commented

Mainly for D9 compatibility

JonMcL’s picture

JonMcL CreditAttribution: JonMcL commented

Of course. That makes sense. I forgot that v4 isn't compatible.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and commented

Hi, I'm attempting to update to Drupal 9 and I see that the progress on this got kind of stuck and a change was even reverted, forcing to backup_migrate version 4.1.

Is there any progress on a 2.x branch and fully D9 compatible version?

I'll try to put something together with the above patches and the suggestions about versions, etc. but wanted to know if there was any other update that I'm not aware of.

Thanks.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and commented
Related issues: +#3139638: Automated Drupal Rector fixes
fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and commented
Title: Add compatibility with backup_migrate 5 » Add compatibility with backup_migrate 5 and Drupal 9
Status: Needs work » Needs review
FileSize
3172376-15-drupal_9_compatibility.patch7.42 KB

Mostly the above patch by @rodrigoaguilera, together with the Rector fixes, but I also changed some annotations and the requirements on the info and json files.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and commented
JonMcL’s picture

JonMcL CreditAttribution: JonMcL commented
Version: 8.x-1.x-dev » 5.0.0-alpha1
Status: Needs review » Fixed

Ahh screw it .. "We'll do it live!" :)

Everything appears to be working EXCEPT you can't rely on the restore buttons that appear on the "Saved Backups" page. At least with S3 buckets, it's not working. You must download the backup file first and then you can use the standard "Restore" page and upload the file.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and commented

Great, this will make the whole D9 upgrade much easier. Thanks.

Status: Fixed » Closed (fixed)

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