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.

CommentFileSizeAuthor
#7 2337591-7.patch634 bytesswentel
#3 2337591-3_install-error.patch703 bytesmikeker

Comments

mikeker’s picture

mikeker commented

Full stack trace of the error:

( ! ) 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 C:\Users\Mike\Documents\dev\drupal-core\d8\core\lib\Drupal\Core\Field\FieldConfigBase.php on line 465
Call Stack
#	Time	Memory	Function	Location
1	0.0011	260104	{main}( )	..\install.php:0
2	0.0165	1094960	install_drupal( )	..\install.php:32
3	4.7343	16866976	install_run_tasks( )	..\install.core.inc:101
4	5.2927	20811048	install_run_task( )	..\install.core.inc:482
5	5.2927	20811048	install_base_system( )	..\install.core.inc:588
6	17.5317	26366224	Drupal\Core\Extension\ModuleHandler->install( )	..\install.core.inc:965
7	20.7016	26742736	Drupal\Core\Config\ConfigInstaller->installDefaultConfig( )	..\ModuleHandler.php:837
8	20.7239	26756424	Drupal\Core\Config\ConfigInstaller->createConfiguration( )	..\ConfigInstaller.php:116
9	22.7863	28623264	Drupal\Core\Entity\Entity->save( )	..\ConfigInstaller.php:227
10	22.7864	28623376	Drupal\Core\Config\Entity\ConfigEntityStorage->save( )	..\Entity.php:303
11	22.7864	28623600	Drupal\Core\Entity\EntityStorageBase->save( )	..\ConfigEntityStorage.php:232
12	22.8169	28647416	Drupal\user\Entity\Role->postSave( )	..\EntityStorageBase.php:406
13	22.8206	28647888	entity_render_cache_clear( )	..\Role.php:153
14	22.8305	29052800	Drupal\Core\Entity\EntityManager->hasHandler( )	..\entity.inc:18
15	22.8305	29052848	Drupal\Core\Entity\EntityManager->getDefinition( )	..\EntityManager.php:234
16	22.8305	29052896	class_exists ( )	..\EntityManager.php:220
17	22.8305	29053344	Composer\Autoload\ClassLoader->loadClass( )	..\EntityManager.php:0
18	22.8307	29053512	Composer\Autoload\includeFile( )	..\ClassLoader.php:274
19	22.8310	29100528	include( 'C:\Users\Mike\Documents\dev\drupal-core\d8\core\lib\Drupal\Core\Field\Entity\BaseFieldOverride.php' )	..\ClassLoader.php:382
20	22.8310	29100920	Composer\Autoload\ClassLoader->loadClass( )	..\ClassLoader.php:0
21	22.8312	29101080	Composer\Autoload\includeFile( )	..\ClassLoader.php:274
22	22.8319	29170104	include( 'C:\Users\Mike\Documents\dev\drupal-core\d8\core\lib\Drupal\Core\Field\FieldConfigBase.php' )	..\ClassLoader.php:382
mikeker’s picture

mikeker commented
Title: ThirdPartySettingsTrait::getThirdPartySetting not compatible with ThirdPartySettingsInterface::getThirdPartySetting » ThirdPartySettingsTrait::getThirdPartySetting not compatible with ThirdPartySettingsInterface::getThirdPartySetting error during install
Issue summary: View changes
mikeker’s picture

mikeker commented
Status: Active » Needs review
Related issues: +#2224761: Add a generic way to add third party configuration on configuration entities and implement for field configuration
StatusFileSize
new 2337591-3_install-error.patch703 bytes

Attached patch removes the default parameter from the getThirdPartySetting trait implementation.

Status: Needs review » Needs work

The last submitted patch, 3: 2337591-3_install-error.patch, failed testing.

swentel’s picture

swentel commented

Hmm, what if you remove the $default paramater from the interface and leave it on the implementation ? IIRC that should actually work as well.

swentel’s picture

swentel commented

Hmm reading that PHP bug report, it seems that won't work with traits, sigh.

swentel’s picture

swentel commented
Status: Needs work » Needs review
StatusFileSize
new 2337591-7.patch634 bytes

Would this work ?

xjm’s picture

xjm
she/her
Primary language English
commented
Related issues: +#2246775: Suggest 5.4.11 as minimum PHP version for Windows and MacOS if XCache is enabled

Oh Windows.

xjm’s picture

xjm
she/her
Primary language English
commented
Component: install system » configuration entity system
Issue tags: +Configurables, +Configuration system
xjm’s picture

xjm
she/her
Primary language English
commented
Title: ThirdPartySettingsTrait::getThirdPartySetting not compatible with ThirdPartySettingsInterface::getThirdPartySetting error during install » Install failure on Windows: ThirdPartySettingsTrait::getThirdPartySetting not compatible with ThirdPartySettingsInterface::getThirdPartySetting error during install
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +d8 dev environment

Sorry for all the noise. :)

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Critical

And if Drupal can't be installed on Windows at all, that's critical.

mikeker’s picture

mikeker commented

Verified 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: return array() instead of NULL for multi-valued settings). But I guess that's beyond the scope of an interface/trait implementation...

swentel’s picture

swentel commented

@mikeker thanks for verifying - you can set it to RTBC if you want.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

Looks fine to me

catch’s picture

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

Me too. Committed/pushed to 8.0.x, thanks!

  • catch committed 1e95f63 on 8.0.x 
    Issue #2337591 by swentel, mikeker: Fixed Install failure on Windows:...

Status: Fixed » Closed (fixed)

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