The list of modules returned from _system_rebuild_module_data() has extraneous array keys with values that do not have an info property, which causes system_sort_modules_by_info_name() to throw notices

Notice: Undefined property: stdClass::$info in system_sort_modules_by_info_name() (line 917 of /Users/Ben/projects/d7/head/modules/system/system.admin.inc).

Despite the key for drupal_system_listing() being 'name' the modules ends up with elements like the following, where you can see that the dependency information is not merged into the skinr module because the keys differ:

   [block] => stdClass Object
        (
            [uri] => modules/block/block.module
            [filename] => modules/block/block.module
            [name] => block
            [info] => Array
                (
                    [name] => Block
                    [description] => Controls the visual building blocks a page is constructed with. Blocks are boxes of content rendered into an area, or region, of a web page.
                    [package] => Core
                    [version] => 7.0-dev
                    [core] => 7.x
                    [files] => Array
                        (
                            [0] => block.module
                            [1] => block.admin.inc
                            [2] => block.install
                            [3] => block.test
                        )

                    [configure] => admin/structure/block
                    [dependencies] => Array
                        (
                        )

                    [php] => 5.2.4
                    [bootstrap] => 0
                )

            [type] => module
            [status] => 0
            [schema_version] => -1
            [weight] => 0
            [required_by] => Array
                (
                    [dashboard] => Array
                        (
                            [name] => block
                        )

                )

            [requires] => Array
                (
                )

            [sort] => 0
        )
...

 [sites/all/modules/contrib/skinr/skinr.module] => stdClass Object
        (
            [uri] => sites/all/modules/contrib/skinr/skinr.module
            [filename] => sites/all/modules/contrib/skinr/skinr.module
            [name] => skinr
            [info] => Array
                (
                    [name] => Skinr
                    [description] => Provides a way to define and/or skin bits of Drupal output from the UI.
                    [package] => Skinr
                    [core] => 7.x
                    [files] => Array
                        (
                            [0] => tests/skinr.test
                        )

                    [dependencies] => Array
                        (
                        )

                    [version] => 
                    [php] => 5.2.4
                    [bootstrap] => 0
                )

            [type] => module
            [status] => 0
            [schema_version] => -1
        )
...

 [skinr] => stdClass Object
        (
            [required_by] => Array
                (
                    [skinr_ui] => Array
                        (
                            [name] => skinr
                        )

                    [skinr_test] => Array
                        (
                            [name] => skinr
                        )

                )

            [requires] => Array
                (
                )

            [sort] => 0
        )

This only happens during testing and if the module's test specifies the testing profile, it doesn't occur when you just visit /admin/modules on a standard profile install yourself.

To reproduce this:

1. Place a contrib module at sites/all/modules/
2. Comment out hidden = TRUE in profiles/testing/testing.info
3. Install Drupal
4. Visit admin/modules
5. See notices

CommentFileSizeAuthor
#6 999072-drupal-system-listing.patch1.39 KBDamien Tournoud
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Cross-linking #985578: Duplicate key error in file registry when running tests using testing profile

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

The question is who or what sets this bogus key:

 [sites/all/modules/contrib/skinr/skinr.module] => stdClass Object
        (
            [uri] => sites/all/modules/contrib/skinr/skinr.module
            [filename] => sites/all/modules/contrib/skinr/skinr.module
            [name] => skinr

Can you try to squeeze some debugging lines into [_]system_rebuild_module_data() in order to figure out at which exact point the bogus key is introduced?

coltrane’s picture

coltrane
he/him
CreditAttribution: coltrane commented

after drupal_system_listing() in _system_rebuild_module_data() we have

[user] => stdClass Object
        (
            [uri] => modules/user/user.module
            [filename] => user.module
            [name] => user
        )
...
[sites/all/modules/contrib/skinr/tests/skinr_test.module] => stdClass Object
        (
            [uri] => sites/all/modules/contrib/skinr/tests/skinr_test.module
            [filename] => skinr_test.module
            [name] => skinr_test
        )

drupal_system_listing() get's the key 'name'

Array
(
    [0] => /^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\.module$/
    [1] => modules
    [2] => name
    [3] => 0
)

so I believe the problem is in this block of drupal_system_listing() where $key must be getting overridden. 


foreach ($searchdir as $dir) {
    $files_to_add = file_scan_directory($dir, $mask, array('key' => $key, 'min_depth' => $min_depth));

    // Duplicate files found in later search directories take precedence over
    // earlier ones, so we want them to overwrite keys in our resulting
    // $files array.
    // The exception to this is if the later file is from a module or theme not
    // compatible with Drupal core. This may occur during upgrades of Drupal
    // core when new modules exist in core while older contrib modules with the
    // same name exist in a directory such as sites/all/modules/.
    foreach (array_intersect_key($files_to_add, $files) as $key => $file) {
      // If it has no info file, then we just behave liberally and accept the
      // new resource on the list for merging.
      if (file_exists($info_file = dirname($file->uri) . '/' . $file->name . '.info')) {
        // Get the .info file for the module or theme this file belongs to.
        $info = drupal_parse_info_file($info_file);

        // If the module or theme is incompatible with Drupal core, remove it
        // from the array for the current search directory, so it is not
        // overwritten when merged with the $files array.
        if (isset($info['core']) && $info['core'] != DRUPAL_CORE_COMPATIBILITY) {
          unset($files_to_add[$key]);
        }
      }
    }
    $files = array_merge($files, $files_to_add);
  }
Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

So this got broken by #895140: _system_rebuild_module_data() and _system_rebuild_theme_data() will choose 6.x versions of modules and themes over 7.x versions.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: _system_rebuild_module_data() keyed off name and uri causes notices on admin/modules in test » drupal_system_listing() returns items keyed by 'uri' instead of 'name' when using testing profile
Priority: Normal » Major

Interestingly, that was also my suspicion, even though I'm not able to see an error in that code.

However, the change that introduced the snippet you pasted was done only a few weeks ago, and around that time, the testing profile started to blow up.

Better title.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Active » Needs review
FileSize
999072-drupal-system-listing.patch1.39 KB

The minimal patch is this.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Active

Interestingly, David Rothstein already mentioned in a comment over there that "for some reason $profile = 'testing' can't be used anymore", which has been happily ignored. ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Active » Needs review

hah, nice one! :)

I wonder whether it is possible to test this...?

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

It only blows in the testing profile because it's the only profile that has a modules directory with modules in it.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
Status: Needs review » Reviewed & tested by the community

I just tested and the patch in #6 fixes the problem. Thanks! :)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Hm. Would be nice if we could somehow test this but I can't really imagine a way to do it.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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