Backdrop sometimes reports "No pending updates" when updates are needed (but not possible)

[jenlampton]

Description of the bug

Backdrop reports "No pending updates" when updates are needed (but not possible) instead of "Some of the pending updates cannot be applied because their dependencies were not met".

Description of the experience

Every time I "clear all caches" (via admin menu, or the button on the Performance page) the site thinks it needs to run database updates, and adds the big red (1) into the admin bar.

On the next reload of the status report page, either by clicking on the Big Red Dot, or by refreshing the Status report page (if I am already on it) the warning about needing to run database updates disappears.

Attempting to run database updates also results in the updates page printing "No pending updates".

Why?

• In my database the schema_version for redirect module was set to -1.
• Because of the schema_version, Backdrop knows updates need to be run.
• Redirect module has a redirect_update_last_removed of 7101.
• Because of this, updates for redirect cannot be run, since the schema_version is not "greater than" 7101.

function redirect_update_last_removed() {
  return 7101;
}

What's the problem?

The fact that backdrop told me "No pending updates" when there were actually pending updates is the problem.

Is this really a problem?

Yes and no?

• No, because I was the one who got my database into this state in the first place (redirect should never have been at -1 to start with)
• But Yes, because it would be really nice if I knew what was happening when I got into this pickle :)

Were do we fix it?

I think there's some code in update.php that doesn't catch when the schema_version is -1.

// Find and label any incompatible updates.
  foreach (update_resolve_dependencies($starting_updates) as $function => $data) {
    if (!$data['allowed']) {
      $incompatible_updates_exist = TRUE;
      $incompatible_count++;
      $module_update_key = $data['module'] . '_updates';
      if (isset($form['start'][$module_update_key]['#items'][$data['number']])) {
        $text = $data['missing_dependencies'] ? 'This update will been skipped due to the following missing dependencies: <em>' . implode(', ', $data['missing_dependencies']) . '</em>' : "This update will be skipped due to an error in the module's code.";
        $form['start'][$module_update_key]['#items'][$data['number']] .= '<div class="warning">' . $text . '</div>';
      }
      // Move the module containing this update to the top of the list.
      $form['start'] = array($module_update_key => $form['start'][$module_update_key]) + $form['start'];
    }
  }

  // Warn the user if any updates were incompatible.
  if ($incompatible_updates_exist) {
    backdrop_set_message('Some of the pending updates cannot be applied because their dependencies were not met.', 'warning');
  }

Steps To Reproduce

To reproduce the behavior:

1. Manually change the schema_version for redirect module to -1
2. Clear all caches (see red dot appear)
3. Visit status report page (see red dot disappear)
4. Clear all caches on the status report page (see red dot appear again)
5. Refresh the page (see red dot disappear again)
6. Clear all caches on the status report page one more time (see red dot appear again)
7. Attempt to run database updates (see "No pending updates")

Actual behavior

saw "No pending updates"

Expected behavior

expected "Some of the pending updates cannot be applied because their dependencies were not met"

Additional information

• This site was originally upgraded from Drupal
• I recently updated from 1.24.2 to 1.27.1, and only noticed the problem after that

[laryn]

Related: https://github.com/backdrop/backdrop-issues/issues/2858

[stpaultim]

I reported a similar problem in the D2B Migrate module issue queue.

https://github.com/backdrop-contrib/d2b_migrate/issues/56

I would appreciate guidance on whether or not my issue might be related to this one and/or if I should create a new issue in the core issue queue. I think that issue may be more related to the upgrade process than it is to D2B Migrate?

Atten: @jenlampton, @docwilmot, @irinaz or @laryn

Just found this issue that also looks similar:

https://github.com/backdrop/backdrop-issues/issues/4714

[irinaz]

@stpaultim , I had seen this behaviour when D7 db is not loaded, and therefore, there are not pending updates.

[klonos]

I somehow missed this issue here. It was brought to my attention via the respective Zulip thread. I was looking into these 3 issues in D7 that I find related/relevant:

• https://www.drupal.org/project/drupal/issues/938560
• https://www.drupal.org/project/drupal/issues/1239370
• https://www.drupal.org/project/drupal/issues/2155779

[klonos]

...as I mentioned in the Zulip thread:

I'm thinking that getting into a situation where schema_version is -1 for enabled modules sounds like a serious bug :thinking: ...would be interested to reproduce it, and see how it could be best fixed.

Looking at the various code that sets schema_version to SCHEMA_UNINSTALLED (-1), I see the following in backdrop_uninstall_modules():

  foreach ($module_list as $module) {
    // Uninstall the module.
    module_load_install($module);
    module_invoke($module, 'uninstall');
    config_uninstall_config($module);
    backdrop_uninstall_schema($module);

    watchdog('system', '%module module uninstalled.', array('%module' => $module), WATCHDOG_INFO);
    backdrop_set_installed_schema_version($module, SCHEMA_UNINSTALLED);
  }

I believe that we should not be setting the schema to SCHEMA_UNINSTALLED here unconditionally. Looking at the return value of backdrop_uninstall_schema():

 * @return
 *   An array of arrays with the following key/value pairs:
 *    - success: a boolean indicating whether the query succeeded.
 *    - query: the SQL query(s) executed, passed through check_plain().

So it seems to me that in the foreach loop in backdrop_uninstall_modules() we should be checking whether the call to backdrop_uninstall_schema() actually returned success as TRUE, and only then we should be proceeding with the backdrop_set_installed_schema_version($module, SCHEMA_UNINSTALLED) - otherwise throw a useful message and log something relevant/helpful in watchdog.

That is the only place as far as I can tell where SCHEMA_UNINSTALLED is being used in a backdrop_set_installed_schema_version() - the rest of the calls in code are in various backdrop_get_installed_schema_version()s. So this seems to be the only possible culprit for setting the schema_version to -1 while keeping the module enabled (if something during the uninstallation process has gone wrong).

The above is with regards of figuring out how we could get in that situation. When it comes to the problem reported here, update_get_update_list() has this logic:

  foreach ($modules as $module => $schema_version) {
    // Skip uninstalled and incompatible modules.
    if ($schema_version == SCHEMA_UNINSTALLED || update_check_incompatibility($module)) {
      continue;
    }
    // Otherwise, get the list of updates defined by this module.
    ...

It seems to me that since we know that we can get into a situation where schema_version is 1 and at the same time status can be 1 (although that is a bug), then we should be doing some additional defensive coding there, and be skipping only if $schema_version == SCHEMA_UNINSTALLED AND ~~module_exists($module)~~ !in_array($module, system_list('module_enabled')).

[avpaderno]

Actually, the documentation for backdrop_uninstall_schema() does not match its code: Without a return statement, it cannot return any value.

That return value description is the same used for a Drupal 6 function (update_sql()) which has been removed from Drupal 7.

(The same wrong description is used for drupal_uninstall_schema(). That would explain why backdrop_uninstall_schema() has that description too.) DatabaseSchema_mysql::dropTable(), the method that effectively deletes the tables defined from a module's schema, returns FALSE when the table does not exist, and TRUE after it executed DROP TABLE.
That value is not returned to the function calling backdrop_uninstall_schema(), though.

[klonos]

Good points @kiamlaluno 👍🏼 ...there's quite a few things to untangle here apparently 🤔