Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
_focal_point_check_crop_version() always returns FALSE when modules have been installed via composer and specifying prefer source. Debugging this led me to uncover that system_get_info() is returns version NULL for all modules in this setup. This issue and the contrib-related effects are discussed in this core issue: #1013302: Versioned dependencies fail with dev versions and git clones
Is there a better way to implement these requirements checks that would accommodate composer workflows? Could we just set a warning instead of an error if we are unable to determine the version?
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2941484-11.patch | 1.78 KB | markdorison |
| #5 | 2941484-5.patch | 2.42 KB | markdorison |
Comments
Comment #2
markdorisonComment #3
markdorisonComment #4
markdorisonComment #5
markdorisonIs there a specific function or class in the Crop API module that we could check for presence of instead of relying on
system_get_info()? By doing a check like that we wouldn't need to worry about whether a site is using composer or not.I believe there has to be a better way to manage this hence my question above, but attached is a patch within the current framework of requirement checks in
focal_point.install. It adds a check for whether or not the version number can be obtained; if the module is determined present, but the version number can not be obtained, aREQUIREMENT_WARNINGis raised instead ofREQUIREMENT_ERROR.Comment #7
markdorisonThis issue has become much more visible with this change in Drush 8.1.16, which is also included in Drush 9.0.
Comment #8
bleen CreditAttribution: bleen at NBCUniversal commentedhave you had a chance to look at those test failures
Comment #9
markdorison@bleen I am not even sure if my approach is the correct one, as I describe in #5. I was looking for feedback on whether this was the best approach before I moved on to looking at the test failures.
Comment #10
bleen CreditAttribution: bleen at NBCUniversal commentedI honestly cannot remember what was introduced in crop alpha2 that focal point is relying on so it would be hard to simply look for the existence of a particular function or something liked that. I'm fine simply adding a REQUIREMENT_WARNING if the version of crop cant be ascertained.
Actually, based on the usage data here, I'd be ok removing the version check altogether. 25 sites out of 22k is not worth the hassle.
Comment #11
markdorison@bleen Attached patch removes the check.
Comment #13
BerdirCan confirm this works. Would be nice to have this committed and then also a new release so that users get the update the next time they run composer update.
Can't imagine those unit test fails being related, probably a change in core or so? HEAD tests didn't run for a while.
Comment #15
bleen CreditAttribution: bleen at NBCUniversal commented