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

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:

Comments

nicxvan created an issue. See original summary.

nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Title: Create class to replace hook_update_requirements » Investigate whether hook_update_requirements can be a hook or needs to be a special class.
Issue summary: View changes
nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Title: Investigate whether hook_update_requirements can be a hook or needs to be a special class. » Create hook_update_requirements
Issue summary: View changes
nicxvan’s picture

Issue summary: View changes

nicxvan’s picture

Status: Active » Needs review
nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Status: Needs review » Needs work

Adding tests

nicxvan’s picture

Status: Needs work » Needs review

Test added

nicxvan’s picture

Status: Needs review » Needs work
nicxvan’s picture

Status: Needs work » Needs review
nicxvan’s picture

alexpott’s picture

The 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.

nicxvan’s picture

The 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.

nicxvan’s picture

Status: Needs review » Postponed

Marking this postponed since an alternate approach was suggested in the Meta

nicxvan’s picture

Title: Create hook_update_requirements » Create hook_update_requirements and hook_update_requirements_alter
Status: Postponed » Needs review
dww’s picture

Status: Needs review » Needs work

Thanks 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.

nicxvan’s picture

I applied all suggestions, I'll take care of the last suggestion and the cs issue tomorrow, thanks!

nicxvan’s picture

Status: Needs work » Needs review

Fixed last couple of comments!

smustgrave’s picture

Status: Needs review » Needs work

Left 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.

nicxvan’s picture

Status: Needs work » Needs review

Took care of it here and the other one too.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, believe feedback has been addressed

nicxvan’s picture

Status: Reviewed & tested by the community » Needs review
Related issues: +#3490841: Create hook_runtime_requirements() and hook_runtime_requirements_alter()
nicxvan’s picture

Status: Needs review » Reviewed & tested by the community

Whoops cross posted.

dww’s picture

Status: Reviewed & tested by the community » Needs work

Apologies, I missed a point in my previous feedback on the API docs.

nicxvan changed the visibility of the branch 3490841-create-hookruntimerequirements to hidden.

nicxvan’s picture

Status: Needs work » Needs review
dww’s picture

Status: Needs review » Reviewed & tested by the community

I 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

dww’s picture

Title: Create hook_update_requirements and hook_update_requirements_alter » Create hook_update_requirements() and hook_update_requirements_alter()
Issue summary: View changes

Minor formatting for title and summary.

nicxvan changed the visibility of the branch 11.x to hidden.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 8798285 and pushed to 11.x. Thanks!

  • alexpott committed 87982857 on 11.x
    Issue #3490842 by nicxvan, alexpott, dww, smustgrave: Create...

Status: Fixed » Closed (fixed)

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