Problem/Motivation

Currently, the `SystemRequirements` class contains two constants that do not logically belong there:

DRUPAL_CORE_REMOVED_MODULE_LIST
DRUPAL_CORE_REMOVED_THEME_LIST

These constants track modules and themes removed from Drupal core, but `SystemRequirements` is intended for install time system checks (PHP version, database, extensions, etc.).

Steps to reproduce

1. Inspect the `SystemRequirements` class in Drupal core (`core/includes/install.inc` or related location in 10.x/11.x).
2. Observe the presence of the following constants:

DRUPAL_CORE_REMOVED_MODULE_LIST
DRUPAL_CORE_REMOVED_THEME_LIST

Proposed resolution

Move the constants to a more appropriate location, such as:

* `core/lib/Drupal/Core/Extension/Extension.php` or another class responsible for module and theme management.
* A dedicated `RemovedCoreExtensionList` class or enum.

Remaining tasks

  • Update all references to the constants to point to the new location.
  • Add or update test coverage if necessary.
  • Ensure backward compatibility with deprecated access from `SystemRequirements` if required.

User interface changes

None. This is an internal refactoring.

Introduced terminology

API changes

CommentFileSizeAuthor
#18 3574156-nr-bot_hy5us18_.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3574156

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.

sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: [pp-1] Deprecate and more constants in SystemRequirements » Deprecate and more constants in SystemRequirements
Status: Postponed » Active

Linked issue has landed.

nicxvan’s picture

nicxvan commented
Issue summary: View changes

sivaji_ganesh_jojodae thank you for updating this, a couple of things I want to clarify:

`SystemRequirements` is intended for runtime system checks

SystemRequirements is for install time checks not runtime

`core/lib/Drupal/Core/Extension/Extension.php`

I don't think we want them here even if it makes sense on it's face

A dedicated `RemovedCoreExtensionList` class or similar.

I'm much more in favor of something dedicated whether it's a class or enum

dcam’s picture

dcam commented
Title: Deprecate and more constants in SystemRequirements » Deprecate and move constants in SystemRequirements

Title typo

mstrelan made their first commit to this issue’s fork.

mstrelan opened merge request !16074

mstrelan’s picture

mstrelan commented
Status: Active » Needs review

Enums can't hold arrays. Have gone with a single new class with two constants and helper methods.

smustgrave’s picture

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

Would be good to get this in as I'm like on the 1 yard line for being done with stable9 and hoping shortcut isn't far behind.

LGTM

nicxvan’s picture

nicxvan commented

Can't this land in 11.5?

mstrelan’s picture

mstrelan commented

Can't this land in 11.5?

Don't know, can it? I guess since 12.0.0 and 11.5.0 should be at the same time, then maybe yes? I chose 12.0.0 based on this:

$ grep -r "deprecated in drupal:12" core | wc -l
27
$ grep -r "deprecated in drupal:11.5" core | wc -l
0

But those 27 deprecations are all for this one CKEditor issue, so might not be relevant: https://www.drupal.org/node/3581531

nicxvan’s picture

nicxvan commented

I think that's just because 11.4 isn't out yet, if you run grep -r "deprecated in drupal:11.4" core | wc -l in sure there are many.

I'm on my phone though so I can't check.

mstrelan’s picture

mstrelan commented

Yeah there are. I'm just not sure what the cutoff is for deprecations in 11.x, I think it was originally going to be 11.4 if 12.0 was coming out at the same time, but that's not the case, so probably 11.5 is right.

smustgrave’s picture

smustgrave commented

This one seems non disruptive that would hope can make 11.4 but don’t make that call

catch’s picture

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

This looks fine except I'm not sure why it's in the core namespace given the only consumer in in system module - should it be in system module instead? Seems pretty internal to the requirements hooks overall.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For #15

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

Moved to system module. I think it seemed to make sense next to \Drupal\Core\Extension\ExtensionLifecycle, but since it's only used by system module you're right that it makes more sense there. It feels like it should go in to a subdir though rather than at the namespace root, but I don't think anything else made sense. Have updated the CR too.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3574156-nr-bot_hy5us18_.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.