TYPO3 Core

So you're using the ConfigurationManager in your code somewhere to retrieve configuration, right?

Yes, only for read access - is it not meant to be done that way? Since the core seems to do it, too

There is one place on the extension manager using this method (fetching the current extension configuration under EXT/extConf/extkey), and several instances in the install tool.

The usage in the extension manager prevents changed extension settings from AdditionalConfiguration.php to be shown in the configuration area of the extension manager. This could be a problem, since the value that is actually used and the value that is shown value are different. To reproduce, add this line to your AdditionalConfiguration.php

$GLOBALS['TYPO3_CONF_VARS']['EXT']['extConf']['rsaauth'] = 'a:1:{s:18:"temporaryDirectory";s:0:"";}';

and change the setting via extension manager. Then the changed setting is shown, but the above setting is used.

I'm not sure about the usages in the install tool, some seem legit (at least under certain assumptions, for example that there is no AdditionalConfiguration.php to be respected while the initial setup is running), about some others I'm not so sure. For example, a configuration preset is marked 'inactive', if its settings are there, but only in AdditionalConfiguration.php.

Well, yeah.

The core reads some information directly from LocalConfiguration to be sure that the overload file AdditionalConfiguration is ignored at some places.

The problem with AdditionalConfiguration is, that we can not realiably detect or find out what exactly is changed there:
The related issue #52557 tries to detect in "all configuration" in install tool if a value is changed by Additional configuration by comparing the setting within LocalConfiguration and the one in $GLOBALS['TYPO3_CONF_VARS']. If it finds a difference, then it knows "ok, this value was probably changed in AdditionalConfiguration". But this is not reliable since there may be code that only hits if the system is production or dev, or if it is 1 o'clock in the morning or other stuff. Even "grepping" in AdditionalConfiguration for code may not be reliable since we do not know if someone included other files.
So neither executing nor scanning this file gives us a definite knowledge if someone change something in this file.

Therefor, AdditionalConfiguration is currently "ignored" by the core. Core basically says: If you have funny code in this file, that change settings, then those settings may not be settable by core handling code anymore, since we never know what and how you are doing things in there. Thus, it is in developers hands. If a developer overwrites settings there (eg. based on context environment variable), then these settings are basically out-of-support by core code like install tool or em. If a dev forgets about overwritten stuff in this file, and then wonders why his settings changed by install tool or em does not work, well, i just see no good option to help there. In my opinion we can not even help to detect wether an option was changed or not, any code we would come up with is probably doomed to fail in some situations.

Thus, I tend to say: If you use AdditionalConfiguration, then you must take care of those options on your own, together with the according side effects it may have.

I'd close this issue as rejected for these reasons, and I tend to also vote down Sascha's pending patch for #52557.

Maybe, if you align with this view, we could think about how to hint devs better about this fact. Maybe we could add hints (to install tool or/and em) that settings changed within AdditionalConfiguration are ignored by the install tool / em code and changing them within these modules may not have the desired effect. This way, we would at least achieve a better information strategy about this situation.

Hi Christian,

I still have some concerns:

1. Because of some blog posts and other stuff easily found with Google, many people use the ConfigurationManager::getConfigurationValueByPath() method (and also the setters). That's how I came to use it, too. There is no clear indication that it should not be used outside the core, and there is obviously the need for a service like it, which behaves as expected, and which can also be mocked easily.

Is it a good idea to add another "configuration manager" to the core, but for use in extensions to query and modify the GLOBALS['TYPO3_CONF_VARS'] array? Maybe even replacing the current ConfigurationManager with that one, and moving the current ConfigurationManager to some place else and giving it a more speaking name, like LocalConfigurationFileManager?

2. Its clear that the install tool can't ever analyze which options are changed by the AdditionalConfiguration.php. That problem is probably equivalent to the halting problem. But I think what Sascha does makes (partly) sense. There should be a warning that a settings value is not taken from the current LocalConfiguration.php, but comes from somewhere else (in the current context). Just a warning, to make debugging easier. Same thing for the extension manager.

1. I think writing the LocalConfiguration file should be a task done by em / install tool (=core) only. Extensions should rely on $GLOBALS['TYPO3_CONF_VARS'] only, not on using some API (BTW: this array can be easily set/mocked within unit test, core tests do that at a lot of places). Maybe we should mark the class and its methods as "@internal, do not use in extensions, use $GLOBALS['TYPO3_CONF_VARS'] directly" to hint devs about this core-internal class, and is not public API.

2. Yep, better hinting in the em / install tool that stuff in AdditionalConfiguration may break "All configuration" (and maybe more?) is a good idea.

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35409

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35409

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35409