Problem/Motivation

We have an installation profile whose machine name is social and a module whose machine name is social_post. Because their machine names, some hook implementations, for example social_post_update_8100() or social_post_update_dependencies() are considered hook_post_update_NAME() implementations from the installation profile instead of hook_update_N() or hook_update_dependencies() implementations done from the module.

The same can happen with the Standard installation profile, for example.

Steps to reproduce

  1. Install Drupal using the Standard installation profile
  2. Install a module whose machine name is standard_post
  3. Go to update.php/selection and check the reported pending updates

I have also attached a test that confirms this for the testing install profile.

Proposed resolution

Accept as hook_post_update_NAME() implementations only the functions that are inside a MODULE.post_update.php/em> file, which is what the hook_post_update_NAME() documentation says those hooks should be placed.

Remaining tasks

  • Attach test module for those who want to test it using the above HTT
  • Add patch for testing purposes

  • Add patch with proposed fix

  • See if we need to clean up the post_update storage

\Drupal::keyValue('post_update')->get('existing_updates') stores these hook_update_N() versions wrongly.

User interface changes

None.

API changes

None.

Data model changes

N/A.

Release notes snippet

N/A.

CommentFileSizeAuthor
#15 after.png4.5 KBrishi.kulshreshtha
#15 before.png8.69 KBrishi.kulshreshtha
#4 3236316-post-update-naming-convention-issue.patch7.19 KBronaldtebrake
Screenshot 2021-09-17 at 14.43.34.png148.78 KBronaldtebrake
standard_post.zip1.39 KBronaldtebrake

Issue fork drupal-3236316

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ronaldtebrake created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Naming convention issue with post update hooks. » Naming convention issue with post update hooks
ronaldtebrake’s picture

ronaldtebrake commented
Version: 10.0.x-dev » 9.3.x-dev
ronaldtebrake’s picture

ronaldtebrake commented
Issue summary: View changes
StatusFileSize
new 3236316-post-update-naming-convention-issue.patch7.19 KB

There seems to be something wrong with the created branch of the issue fork, so for now just adding it as a patch.

I've added:

1. A new module called testing_post with some basic update hooks & post update hooks, but by using this module name it means that Drupal will treat hook_update_N and hook_update_dependencies as post update hooks from the testing install profile instead.

2. UpdatePostUpdateInstallProfileTest.php
To test if Drupal correctly sees these as update hooks and if the update hooks are being ran and not shown as pending post_update hooks after being fired.

ronaldtebrake opened merge request !1215

ronaldtebrake’s picture

ronaldtebrake commented
Issue summary: View changes
Status: Active » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

@ronaldtebrake should I review the patch or the MR - I assumed the MR, so hiding the patch for now.

Left a comment on the MR, whilst I like the idea, I think there are modules (e.g. webform) for which this would be a hard break.

So we may need to be a bit more graceful, ie. trigger a deprecation error or something - not sure.

I think another approach is #3167625: Deprecate/replace hook_update_N() in favor of an object-oriented approach similar to Laravel migrations

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

tests prove there's an issue

in addition we noticed another issue (this time D8 core though, but I imagine same issue in D9 , probably D7 also)

if the modulename is xyz_update

then in the xyz_update.install file, the hook updates don't get triggered

so the function looks like 

/**
 * Sorta kinda tries to implement hook_update().
 */
xyz_update_update_8002() {
  // Code;
}

this update hook isn't detected, doesn't run

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Assigned: ronaldtebrake » Unassigned
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Naming convention issue with post update hooks » hook_update_N() and hook_update_dependecies() implementations done from a module could be taken as hook_post_update_NAME() implementations done from a(nother) module
Issue summary: View changes
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: hook_update_N() and hook_update_dependecies() implementations done from a module could be taken as hook_post_update_NAME() implementations done from a(nother) module » hook_update_N() and hook_update_dependecies() implementations done from a module could be taken as hook_post_update_NAME() implementations done from another module

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

rishi.kulshreshtha’s picture

rishi.kulshreshtha
he/him/his
Primary language English
Location ME, 🇺🇸
commented
Status: Needs review » Needs work
StatusFileSize
new before.png8.69 KB
new after.png4.5 KB

The patch got applied cleanly, but Drupal stopped detecting the unfinished my_theme_post_update_custom_stuff(). I even made the NAME completely numerical, for instance, my_theme_post_update_10001(), as a workaround, but it didn't work either.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nicxvan’s picture

nicxvan commented
Component: base system » extension system
Related issues: +#3129912: post-update hooks get picked up if defined in the wrong module file

I think this is a duplicate https://www.drupal.org/project/drupal/issues/3129912

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.