Drupal Core (8.8.4) Update Forces Honeypot to Recall Hook_Update_N

This seems like it would be a major bug in Drupal core if sites re-ran old update hooks. Are you sure there might not be some other possibility here, like the module got reinstalled somehow or was missing from the codebase and then got re-added?

Also, can you provide any specifics about how to reproduce the problem? I haven't seen this happen elsewhere, but if more people start running into it, maybe it's a bug in Drupal core that needs to be resolved there.

FYI, we are facing the same issue as described

Just in case, I added a patch

@DrColossos - do you have any output from the failure that could be useful in debugging this? I wonder if there was a change to Drupal core that messed with the way updates are run that I'm not accounting for in the module...

There was no additional error message. I attempted a "drush updb" and encountered the said error, that the column already exists.

Setting to needs review so the bot can have a go at it.

The patch seems simple enough, and it seems more people are running into this. It still seems like this would be a major issue in Drupal core if it's re-running really old updates though :(

This sounds like a core bug where schema information for some modules is missing on some sites. The root cause is still unknown. This surfaced in 8.8.3 and has been fixed in 8.8.7: #3120910: Sites with missing schema information can't update to 8.8.3+ - attempts to run all historical updates

I'm upgrading a site with Honeypot 8.x-1.29 from Drupal 8.7.0 to Drupal 8.9.3. When I run drush updb I get the following warning:

[warning] Schema information for module honeypot was missing from the database. You should manually review the module updates and your database to check if any updates have been skipped up to, and including, honeypot_update_8000().

After applying the patch in #4 I continue to get this warning and the database updates appear to complete successfully, however honeypot_update_8100() is actually still outstanding. Rerunning the updates is successful.

This same issue we are seeing with Drupal 9.1.11 release updgrade:

honeypot 8100 hook_update_n 8100 - Adds the 'hostname' column to the {honeypot_user} table.

That patch #4 was able to resolve this issue while we are using Honey pot 2.0.1 release.

This really does seem to be the problem that @longwave mentioned in #8.

The warning shown in #9 is the result of the core fix for that issue - that indicates that you did encounter that core bug. I'm not sure why the core fix wasn't sufficient.

Only one thing looks wrong in the honeypot code - hook_update_8100() should really be hook_update_8101(), as "00" update is reserved and shouldn't be used. Maybe that's the problem.

#4 should not be committed as written because you should never change or rename a hook_update_N() - that makes it impossible to detect which version of the hook_update_N() was run on a given site.

Instead, I guess taking #4 and putting that into a new hook_update_8101() should avoid the problem as well as being more correct.

Here's a new patch which deals with the original problem in a way that I believe is completely backwards-compatible so that all sites get updated properly whether or not the hook_update_8100() is attempted more than once.

Can someone try this out?

Added an update test.

Updated patch for latest changes to D10.
The test failure seen in #14 has not been fixed - the update test checks to see if the non-updated database contains the hostname column. It should not contain that column before the update, but it does, causing the test to fail. I do not know why this happens, and I have found no similar test in core to use as an example for why it fails. Fixing this test is why this issue is still open.

I have commented out the problem lines in the test just to verify everything else works.

Stupid Drupal. Wasted more than 15 months because of a stupid undocumented problem in update tests, where IF YOU'RE on DRUPALCI you can't access the database the same as you would in any other Functional test. It works on my local just fine of course. And there were no examples in core or contrib where people tested the addition of a database column - probably because no-one could get it to work. And I got no help in the various channels where I asked for help with this.

Committed. Good riddance.