#2205271-153: Project namespace for dependencies has discovered issue with _system_rebuild_module_data_ensure_required. This function assumes that the dependency is always just the module name. It is not. It can contain project information and version information.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because correct usage of module dependencies can result in php warnings.
Unfrozen changes Unfrozen because it is just a normal bug fix.
Disruption none
CommentFileSizeAuthor
#7 2410151.7.patch2.1 KBalexpott
#7 5-7-interdiff.txt4.74 KBalexpott
#5 2410151.5.patch6.84 KBalexpott
#5 1-5-interdiff.txt5.65 KBalexpott
#1 2410151.1-test-only.patch489 bytesalexpott
#1 2410151.1.patch2.04 KBalexpott

Comments

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
StatusFileSize
new 2410151.1.patch2.04 KB
new 2410151.1-test-only.patch489 bytes

Note that #2205271: Project namespace for dependencies did not introduce this. Dependencies could have version strings before that change.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
xano’s picture

xano
he/they
Primary language English
Location Southampton
commented 
+++ b/core/modules/system/system.module
@@ -927,11 +928,12 @@ function _system_rebuild_module_data() {
+      $dependency_name = ModuleHandler::parseDependency($dependant)['name'];

Nitpick: for code clarity, I suggest using $dependent_name, because a dependent and a dependency are each other's opposites.

Status: Needs review » Needs work

The last submitted patch, 1: 2410151.1-test-only.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 1-5-interdiff.txt5.65 KB
new 2410151.5.patch6.84 KB

re #3 gone the other way...

Status: Needs review » Needs work

The last submitted patch, 5: 2410151.5.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 5-7-interdiff.txt4.74 KB
new 2410151.7.patch2.1 KB

There was an unexpected item in the bagging area...

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Needs review » Reviewed & tested by the community

You had this issue patch in the previous code :P https://www.drupal.org/node/2266991#comment-9527049

Works fine!

trobey’s picture

trobey commented

It appears that this bug is also in Drupal 7.

trobey’s picture

trobey commented

I had a typo that generated an error similar to this issue. Also I did not not see in system_rebuild_module_data() or _system_rebuild_module_data() any call to drupal_parse_dependency(). It took a while to trace through the code to figure out that _module_build_dependencies() calls drupal_parse_dependency() and then system_modules() calls_drupal_check_incompatibility(). So I do not think I need to backport this to Drupal 7.

keopx queued 7: 2410151.7.patch for re-testing.

catch’s picture

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

Committed/pushed to 8.0.x, thanks!

  • catch committed ce910dc on 8.0.x 
    Issue #2410151 by alexpott: _system_rebuild_module_data_ensure_required...

Status: Fixed » Closed (fixed)

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