backdrop-contrib / backup_migrate

Port of the Drupal backup_migrate module to Backdrop
GNU General Public License v2.0
7 stars 14 forks source link

Updates expect `.module` file to be available (Error when running updates for 1.x-1.0.26) #160

Open manu-mei-singh opened 8 months ago

manu-mei-singh commented 8 months ago

Hey ya'll,

I am updating Backup Migrate on a number of Backdrop instances. When I try and the database updates on for backup_migrate-1.x-1.0.26 on Backdrop 1.27.1

I get the following error when I run bee -y updb on the command line :

Error: Call to undefined function backup_migrate_include() in backup_migrate_update_1009() (line 632 of /var/www/path/to/root/directory/modules/backup_migrate/backup_migrate.install).

If I run the web update I get the following error:

An AJAX HTTP error occurred. HTTP Result Code: 500 Debugging information follows. Path: https:///directory.fake/core/update.php?op=selection&token=va-qAhj8U0PAJASE3GxdtneHT_n5y0n6Ox_ULVF_zWM&id=59&op=do_nojs&op=do StatusText: Service unavailable (with message) ResponseText: Error: Call to undefined function backup_migrate_include() in backup_migrate_update_1009() (line 632 of var/www/path/to/root/directory/modules/backup_m

Note that I've changed the file paths to protect clients.

Any help would be greatly appreciated!

thanks! Manu

argiepiano commented 8 months ago

Thanks for reporting, @manu-mei-singh. Apparently there are certain instances when the update hook is run before Backdrop includes the main backup_migrate.module file. This is strange.

As a stopgap measure, you may want to change line 632 of backup_migrate.install manually from: backup_migrate_include('profiles');

TO: include_once (BACKDROP_ROOT . '/' . backdrop_get_path('module', 'backup_migrate') . '/includes/profiles.inc');

Then run update.php again.

Please report back here.

argiepiano commented 8 months ago

Actually, that may not work, since line 7 of profiles.inc invokes that same function. This is a very strange situation - I'm wondering if another contrib module is actually making the update hooks run before the module files are loaded.

Anyway, you may want, instead, to try this (replace the function as follows):

/**
 * Add the exclude_filepaths configuration for the new "DB and active config" source to all profiles.
 */
function backup_migrate_update_1009() {
  include_once (BACKDROP_ROOT . '/' . backdrop_get_path('module', 'backup_migrate') . '/backup_migrate.module');
  backup_migrate_include('profiles');
  foreach (backup_migrate_get_profiles() as $profile) {
    if (!isset($profile->filters['sources']['db_config']['exclude_filepaths'])) {
      $profile->filters['sources']['db_config']['exclude_filepaths'] = '';
      $profile->save();
    }
  }
}

Then try running update.php again.

argiepiano commented 8 months ago

EDIT: I pasted the wrong function. Now fixed. This function is in backup_migrate.install

manu-mei-singh commented 8 months ago

Hi @argiepiano , yup that worked. It did create a bunch of warnings though. Anyway thanks for your help! Because there are few more Backdrop updates I need to do -- I created a git patch. It's also attached if anyone else needs it. backup_migrate_1x1026.patch

argiepiano commented 8 months ago

Thansk, @manu-mei-singh. Would you post here some of the warnings?

Was this a D7 upgrade site, or "native" Backdrop? Also, it possible, it'd be helpful to know what contrib modules you have installed in the site. Given that BM is so widely used, I'd like to be sure other people don't run into this, so if there is a bug, I'd like to find it.

manu-mei-singh commented 8 months ago

Hi @argiepiano ,

This was update was performed on a staging "native" backdrop site. I ran the update via the web (core/update.php).

Here are the warning messages from the logs:

Warning: Trying to access array offset on value of type bool in backup_migrate_item->decode_db_row() (line 549 of /backdrop_root/modules/backup_migrate/includes/crud.inc).

Warning: foreach() argument must be of type array|object, null given in backup_migrate_item->decode_db_row() (line 549 of /backdrop_root/modules/backup_migrate/includes/crud.inc).

And it looks like I got few of these warnings.

To confirm that the update did work, I ran bee -y updb, which gave me a confirmation that the database has been updated.

The other contrib modules are IMCE and CiviCRM.

hope that helps and thanks for the all your work on this!

jenlampton commented 8 months ago

Apparently there are certain instances when the update hook is run before Backdrop includes the main backup_migrate.module file. This is strange.

@argiepiano This is expected, no module files should ever be loaded at update time. You can't depend on module code working at the time modules are updated. Consider an upgrade from Drupal 7 - if the schema were in one state for D7 but the Backdrop .module code expected a different state for B (or for config to exist!) then loading the.module file could cause a fatal error. Updates should only call APIs directly, and interact with database/config directly.

As such, i would not recommend doing a module_load_include() to the .module file, but instead move any code that the update will need to run into a .inc file, and include that from both the install and .module file.

It's strange that you found an instance where those files were loaded at update time - opcode cache maybe?.

Is it possible this explains some of the other issues with updates, like https://github.com/backdrop-contrib/backup_migrate/issues/158?

argiepiano commented 8 months ago

@jenlampton, yes, I was leaning toward that explanation. I'll figure out a solution.