Needs review
Project:
Drupal core
Version:
main
Component:
extension system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
22 Jul 2026 at 09:22 UTC
Updated:
23 Jul 2026 at 03:43 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #3
quietone commentedComment #4
quietone commentedComment #5
quietone commentedComment #6
akshay kashyap commentedThanks for the patch, the underlying fix looks correct to me. A few small
things before this is RTBC:
1.
$checked = array_keys($modules['install']);followed byin_array($dependency, $checked)does an O(n) scan for every dependency checked. Since$modules['install']is already keyed by module name, it would be cheaper - and more consistent with the isset() checks used elsewhere in this method - to just copy the array and look up the key directly, e.g.:2. It would help to add a short comment explaining why the snapshot needs to be taken before the loop starts. It's not obvious from the code alone, and without an explanation a future refactor could simplify this back to checking $modules['install'] directly and reintroduce the bug this patch is fixing.
3. There's a stray blank line before the closing brace of
testModuleDependencyMessages()in DependencyTest.php, phpcs will likely flag it.Comment #7
quietone commented@akshay kashyap, thanks
1, 2: changes made
3: As far as I know, there isn't a Drupal coding standard about the line before the closing brace.
Comment #8
nicxvan commentedI have a minor comment on grammar, but I'm confused by this, I just compared this branch and main.
I ran drush si then tried installing search node and Json response validator:
Main:
This branch:
What case is this not showing the right list?
Is it about showing dependencies in more than one list? I uninstalled many modules so I could install block content and navigation which both depend on block.
Main:
This branch:
That seems to be the case so I updated the title.
The before and after in the IS don't load, but I have the before and after text here so I think that's ok.
I also can't run the test only, but the test does look good, @quietone, if you could runt the test only that would be great.