Problem/Motivation

Let's remove hook_hook_info and support for it in 12 sinc.e it was deprecated for removal

Steps to reproduce

N/A

Proposed resolution

Remove the autoloading of .inc and persistent groupIncludes .inc for hook_hook_info resolution.
Remove tests related to it
Remove remaining hook_hook_info implementations
See https://www.drupal.org/node/3489765 for more information, this is not a normal deprecation removal

Remaining tasks

Review

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3571067

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

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan commented
Assigned: Unassigned » nicxvan

nicxvan opened merge request !14601

nicxvan’s picture

nicxvan commented
Title: Remove hook_hook_info and support in HookCollectorPass » [pp-2] Remove hook_hook_info and support in HookCollectorPass
Issue summary: View changes
Status: Active » Postponed
Related issues: +#3571063: Remove Deprecated methods in ModuleHandler and ModuleHandlerInterface slated for removal in 12, +#3571069: Remove hook_module_implements_alter and support in HookCollectorPass
nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Title: [pp-2] Remove hook_hook_info and support in HookCollectorPass » [pp-1] Remove hook_hook_info and support in HookCollectorPass
nicxvan’s picture

nicxvan commented
Title: [pp-1] Remove hook_hook_info and support in HookCollectorPass » Remove hook_hook_info and support in HookCollectorPass

Let's postpone the other one instead since this is closer.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
Status: Postponed » Needs review
smustgrave’s picture

smustgrave commented

Should hook_hook_info() be removed from Hook.php ?

nicxvan’s picture

nicxvan commented

Good catch, updated!

I searched again, I think that was the only remaining reference.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

That's also all I found too. LGTM!

nicxvan’s picture

nicxvan commented
Title: Remove hook_hook_info and support in HookCollectorPass » [12.x] Remove hook_hook_info and support in HookCollectorPass
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work

We can do a little bit more cleanup I think.

nicxvan’s picture

nicxvan commented
Status: Needs work » Needs review

You're right, I must have been thinking of something else, I removed it.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Thanks, even more code removed now, yay.

  • catch committed cfe4a9ad on main 
    task: #3571067 [12.x] Remove hook_hook_info and support in...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !14601

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I think this broke old requirements hooks in .install.

I was trying to enable a module with drush and had search_api enabled.

Drush doesn't yet support the new requirements hook but that doesn't really matter, core still would call the old one too. In \Drush\Commands\pm\PmCommands::validateEnableModules(), it calls invoke on that for the new module that I'm trying to install. it also loads the .install file, but only for that module obviously.

However, that also initializes the hook list, and while it still scans .install files, it no longer loads them automatically. Which is kind of fine but at the same time, it expects them to be loaded as it unconditionally resolves that callback, resulting in "Class "search_api_requirements" does not exist."

I think #3549397: [regression] Uninstalling and installing modules during config:import can lead to fatal errors might address that in a way, but relying on catching exception for this case doesn't seem so great. I wonder if we could get away with skipping requirements hooks from hook discovery completely and force a fallback to legacy invoke?

catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

Not reverting this yet but re-opening so it doesn't get lost.

nicxvan’s picture

nicxvan commented

Got it, I know what to do.

nicxvan changed the visibility of the branch drupal-3571067-requirements to hidden.

nicxvan’s picture

nicxvan commented
Related issues: +#3574003: Properly include install files for install hook_requirements

Created a follow up here: #3574003: Properly include install files for install hook_requirements

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Fixed

I'm OK with closing this again now that we have the follow-up.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

nicxvan’s picture

nicxvan commented
Assigned: nicxvan » Unassigned

Status: Fixed » Closed (fixed)

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