[pp-many] Deprecate hook_requirements

Pretty sure we're now down to 2. 🤓

We should also update hook_requirements() itself (the module.api.php documentation function) to refer to the replacements and make it clear that this is deprecated.

Not sure about the version. It's technically not deprecated yet, but all replacements *are* in 11.2, so I think it can be argued to backdate the version it's deprecated in to 11.2 and possibly also remove this in D12?

The issue is detecting that the replacements are there.

The deprecation would continue being thrown when supporting 11.1

I discussed this briefly with @catch on slack and he suggested deprecating in 12 for removal in 13.

I see, but that seems like a blocker for actually converting to the new hooks, because we don't want to to run both and the result in a mess when merging them?

But I'm not sure this is really an issue?

We have #LegacyHook and a special version of that for hook_module_implements_alter().

We might miss a few edge cases that already converted hook_requirements() to OOP, but I think we never officially supported that, even if it would work for runtime/update phase. And I can't find anything with #[Hook('requirements' on http://codcontrib.hank.vps-private.net/search?text=%23%5BHook%28%27requi....

So all it would take is another special case in \Drupal\Core\Hook\HookCollectorPass::addProceduralImplementation() and trigger a deprecation?

I see, but that seems like a blocker for actually converting to the new hooks, because we don't want to to run both and then end up with a mess when merging them?

In contrib? No that's why Catch suggested for 12

We might miss a few edge cases that already converted hook_requirements() to OOP

I think there might be a disconnect, hook_requirements explicitly cannot be oop.
That is why it was replaced with update and runtime requirements and the new class.

If we deprecate the original hook in 11 then contrib has to think about how to support 10.

If we wait they can just drop less than 11.2 when they convert.

What I mean is that I think we should make sure that we don't call both hook_requirements() and hook_runtime_requirements() of the same module, because they both do the same thing.

We do have #LegacyHook, with that, D10 would be a non-issue. you just keep the old hook and call through to the new hook depending on $phase. But just like hook_module_implements_alter(), the problem is 11.1, which skips LegacyHook but doesn't understand the new requirements hooks.

The problem with waiting is that the new hooks and their change records are out. There are already early adopters and there will be more over the next months. field_encrypt for example now implements hook_requirements() *and* the new hook_runtime_requirements(). Right now, we call both and merge them together. We use a flat array_merge(), so it's not a huge deal, the legacy hook just wins and we ignore the same keys from the runtime requirements.

So we could leave it at that, but I'm not 100% sure that this won't cause any unexpected issues.

What we should have done is add it's own legacy attribute like LegacyModuleImplementsAlter, so that we can identify that and skip it in 11.2+. Alternatively, we could deal with when we call the hook, for example change \Drupal\system\SystemManager::listRequirements to call the runtime hook first with invokeAllWith(), remember the modules, and then skip them in a second invokeAllWith() for the legacy requirements hook.

The problem is that not all instances of hook_requirements go through module handler invoke so the attribute doesn't work unless we introduce reflection there.

Maybe that is what you are suggesting and maybe that is worth doing.

Started a merge requests to show what I mean. It's pretty verbose, but it handles deprecation and supporting BC versions with the assumption that a module with a runtime_requirements hook only implements hook_requirements() for BC.

Expecting \Drupal\Tests\system\Kernel\Module\RequirementsTest to fail with a deprecation, so we'll have to figure out testing around that. Might want to convert that module to new hooks because it's used in a few tests and have two new modules. One that only implements hook_requirements() and hook_requirements_alter() and triggers deprecations and that has both and doesn't.

And we'll need to do something similar for update and install requirements.

Would this need a CR or is it covered under another?