Problem/Motivation
Installing Drupal8 on Windows raises Fatal error: Declaration of Drupal\Core\Config\Entity\ThirdPartySettingsTrait::getThirdPartySetting($module, $key, $default = NULL) must be compatible with Drupal\Core\Config\Entity\ThirdPartySettingsInterface::getThirdPartySetting($module, $key, $default) in <...snip...> core\lib\Drupal\Core\Field\FieldConfigBase.php on line 465
. This code was introduced as part of #2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration.
I'm fairly sure this is a Windows-only issue. I have verified this bug on PHP for Windows, versions 5.4.12 and 5.5.16. PHP versions 5.4.32 and 5.5.16 on Linux (via VirtualBox) had no problems installing. Regardless, this does prevent any Windows-based developers (like me!) from contributing to core.
May be related to https://bugs.php.net/bug.php?id=64079.
Basically, the trait implementation of getThirdPartySetting
has an optional parameter while the interface does not.
Proposed resolution
Remove the optional parameter on the trait implementation.
Remaining tasks
Patch forthcoming...
User interface changes
None.
API changes
Changes ThirdPartySettingsTrait
by removing an optional parameter.
Comment | File | Size | Author |
---|---|---|---|
#7 | 2337591-7.patch | 634 bytes | swentel |
#3 | 2337591-3_install-error.patch | 703 bytes | mikeker |
Comments
Comment #1
mikeker CreditAttribution: mikeker commentedFull stack trace of the error:
Comment #2
mikeker CreditAttribution: mikeker commentedComment #3
mikeker CreditAttribution: mikeker commentedAttached patch removes the default parameter from the
getThirdPartySetting
trait implementation.Comment #5
swentel CreditAttribution: swentel commentedHmm, what if you remove the $default paramater from the interface and leave it on the implementation ? IIRC that should actually work as well.
Comment #6
swentel CreditAttribution: swentel commentedHmm reading that PHP bug report, it seems that won't work with traits, sigh.
Comment #7
swentel CreditAttribution: swentel commentedWould this work ?
Comment #8
xjmOh Windows.
Comment #9
xjmComment #10
xjmComment #11
xjmSorry for all the noise. :)
Comment #12
xjmAnd if Drupal can't be installed on Windows at all, that's critical.
Comment #13
mikeker CreditAttribution: mikeker commentedVerified that #7 also fixes the issue of installing Drupal on Windows without needing to update the unit tests.
Part of me wants to somehow insist that developers set reasonable
$default
values (eg: returnarray()
instead ofNULL
for multi-valued settings). But I guess that's beyond the scope of an interface/trait implementation...Comment #14
swentel CreditAttribution: swentel commented@mikeker thanks for verifying - you can set it to RTBC if you want.
Comment #15
alexpottLooks fine to me
Comment #16
catchMe too. Committed/pushed to 8.0.x, thanks!