Problem/Motivation
Hook requirements uses phase right now to manage whether it is runtime, install, or update.
Let's move the update portion.
Create hook_update_requirements() and hook_update_requirements_alter() .
It is currently invoked in update.inc
function update_check_requirements() {
// Because this is one of the earliest points in the update process,
// detect and fix missing schema versions for modules here to ensure
// it runs on all update code paths.
_update_fix_missing_schema();
// Check requirements of all loaded modules.
$requirements = \Drupal::moduleHandler()->invokeAll('requirements', ['update']);
\Drupal::moduleHandler()->alter('requirements', $requirements);
$requirements += update_system_schema_requirements();
return $requirements;
}
Steps to reproduce
N/A
Proposed resolution
Create hook_update_requirements()
Create hook_update_requirements_alter()
Invoke hook_update_requirements() after hook_requirements()
Merge results
Run hook_requirements_alter()
Run hook_update_requirements_alter()
Remaining tasks
N/A
User interface changes
N/A
Introduced terminology
API changes
hook_update_requirements()
hook_update_requirements_alter()
Can be implemented to set or alter requirements during update.
Data model changes
N/A
Release notes snippet
Issue fork drupal-3490842
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:
Comments
Comment #2
nicxvan commentedComment #3
nicxvan commentedComment #4
nicxvan commentedComment #5
nicxvan commentedComment #6
nicxvan commentedComment #8
nicxvan commentedComment #9
nicxvan commentedComment #10
nicxvan commentedComment #11
nicxvan commentedComment #12
nicxvan commentedAdding tests
Comment #13
nicxvan commentedTest added
Comment #14
nicxvan commentedComment #15
nicxvan commentedComment #16
nicxvan commentedComment #17
alexpottThe issue summary here is missing the why we should do this. What is wrong with the old system of using $phase. If the answer is the hook system updates to be OOP - then I'm not sure that we're thinking about this the right way around.
Comment #18
nicxvan commentedThe short answer is the different phases have different requirements for what can be injected. Also almost all modules only interact with one phase so this would be a clean move.
I'll reply more fully on the meta.
Comment #19
nicxvan commentedMarking this postponed since an alternate approach was suggested in the Meta
Comment #20
nicxvan commentedComment #21
dwwThanks for moving this forward. The parent meta has good discussion on why this is actually the right approach, after all. 😅
Left a bunch of suggestions on the MR. Mostly pedantic nits, but a few things of substance.
Comment #22
nicxvan commentedI applied all suggestions, I'll take care of the last suggestion and the cs issue tomorrow, thanks!
Comment #23
nicxvan commentedFixed last couple of comments!
Comment #24
smustgrave commentedLeft some comments. But think if we can see some phpcs violations we should try and address in the issue.
Phpstan different animal and agree breaking that out.
Comment #25
nicxvan commentedTook care of it here and the other one too.
Comment #26
smustgrave commentedThanks, believe feedback has been addressed
Comment #27
nicxvan commentedComment #28
nicxvan commentedWhoops cross posted.
Comment #29
dwwApologies, I missed a point in my previous feedback on the API docs.
Comment #31
nicxvan commentedComment #32
dwwI have a very minor lingering concern that INFO and OK are weird for update. They’re never displayed unless there’s an error or warning, in which case all update requirements, even OK and INFO, are shown to the user, distracting them from the thing they actually have to fix. 😕 This is not a problem to be solved in this issue. 😅 Maybe some of it can be addressed at #3500145: Deprecate REQUIREMENT_INFO in favor of REQUIREMENT_OK, but probably we’ll want yet another follow up for the UX of update.php when there are requirements problems.
That said, I think we’ve done everything we can to make this a clean and solid improvement. RTBC!
Thanks,
-Derek
Comment #33
dwwMinor formatting for title and summary.
Comment #35
alexpottCommitted 8798285 and pushed to 11.x. Thanks!