The logic in Drupal\libraries\Extension\Extension::getLibraryDependencies() fetches any library_dependencies data from each extension. It has the potential however to raise some issues when the extension is the install profile. In this case the extension reports its type as "profile", but when that type is use in the info lookup via system_get_info() as empty result is returned. It seems that system_get_info() only knows how to work with "module" and "theme" types so it will always return an empty result for type "profile". This results in several logged errors after cache clears, and in the case of a dev setup (using settings.local.php), the potential for fatal errors given the assertion enforcement within getLibraryDependencies() e.g.:

Notice: Undefined index: profile in system_list()
AssertionError: !empty($info)
Warning: Invalid argument supplied for foreach() in system_get_info()

It seems however that system_get_info() is in fact able to get info for install profiles, it just treats them as type "module". So the fix may be to simply conditionally adjust the extension type for profiles before calling system_get_info().

I'm not sure why automated tests don't pick up on this.... maybe the behavior is different for installs run under an older version of core?

CommentFileSizeAuthor
#2 2815189-2.patch734 bytesrjacobs
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rjacobs created an issue. See original summary.

rjacobs’s picture

Status: Active » Needs review
FileSize
734 bytes

Here is a quick patch that seems to clear this up.

It still occurs to me that there may be additional room to simplify this detection a bit more (in parallel to addressing this very small bug). For example I'm not sure I understand why we are doing the extension info detection via a custom method on a custom wrapped Extension object. Could we instead do it directly in the LibraryManager without the need to extend the Extension class in a custom way? I assume there was good forward-compatible thinking behind this current design though.

rjacobs’s picture

I guess this is not an issues for tests as they use the empty "testing" profile (which must not enable a profile-specific extension). So the assertion error in getLibraryDependencies() does not lead to a test failure.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, feel free to commit.

Thanks!

No idea why tests don't pick up on this, good question.

  • rjacobs committed fa93a5a on 8.x-3.x
    Issue #2815189 by rjacobs, tstoeckler: Library dependency checking can...
rjacobs’s picture

Status: Reviewed & tested by the community » Fixed

Thanks! Committed.

I also opened a follow-up regarding the simplification question from #2: #2816115: Consider refactoring to remove custom Extension Handler service. With this issues addressed that one becomes pretty low-priority, but I still wanted to capture it somewhere.

Status: Fixed » Closed (fixed)

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