TYPO3 Core

It is marked as optional, but you get an exception if you do not set it:

https://git.typo3.org/Packages/TYPO3.CMS.git/blob/TYPO3_6-2:/typo3/sysext/core/Classes/Utility/ExtensionManagementUtility.php#l1249

Look one line above.

One line above does not solve the problem.

There are possibilities where $_EXTKEY is not in the global scope.

One such possibility is when you call addPlugin from ext_localconf.php. This file will be loaded via ExtensionManagementUtility:: loadSingleExtLocalconfFiles:
https://git.typo3.org/Packages/TYPO3.CMS.git/blob/TYPO3_6-2:/typo3/sysext/core/Classes/Utility/ExtensionManagementUtility.php#l1511

Then $_EXTKEY is available in the local scope of loadSingleExtLocalconfFiles and in the required file. But it is not included in $GLOBALS['_EXTKEY'].

Better said, this is not a regression to #59770 but to an earlier patch that changed the ext_localconf handling

I tried to narrow down the problem and noticed something.

As you said, the handling of ext_localconf.php is a bit different than than that of ext_tables.php:
https://git.typo3.org/Packages/TYPO3.CMS.git/blob/TYPO3_6-2:/typo3/sysext/core/Classes/Utility/ExtensionManagementUtility.php#l1511
https://git.typo3.org/Packages/TYPO3.CMS.git/blob/TYPO3_6-2:/typo3/sysext/core/Classes/Utility/ExtensionManagementUtility.php#l1737

loadSingleExtLocalconfFiles does not define $_EXTKEY as global like loadSingleExtTablesFiles does.

Setting the same globals in loadSingleExtLocalconfFiles should resolve this problem and keep backwards compatibility.

But also my extension was incorrect. I called addPlugin in ext_localconf and in ext_tables. Calling it in ext_localconf is incorrect, because the tt_content TCA is not yet loaded.
Calling addPlugin in ext_tables.php works correctly in 6.2.4.

Yeah, main issue is that we did not expect that addPlugin() could reside in ext_localconf where $_EXTKEY is not set ... i'm currently undecided if we should disarm the core based check again, since we could say that this is a bug in the extension. OTOH it is not simple for ext devs to decide what exactly should go to ext_localconf and what to ext_tables, so the problem is home-made (imho both files should go away, but that is a different topic) ...

Furthermore, we are "only" talking about an icon here, so probably it would be best to kick this exception again from addPlugin() for better compatibility with extensions that are maybe not 100% clean at this point? Other opinions?

Hi!

Robert Vock wrote:

As you said, the handling of ext_localconf.php is a bit different than than that of ext_tables.php:
loadSingleExtLocalconfFiles does not define $_EXTKEY as global like loadSingleExtTablesFiles does.

Exactly.

Setting the same globals in loadSingleExtLocalconfFiles should resolve this problem and keep backwards compatibility.

I wouldn't do so.

But also my extension was incorrect. I called addPlugin in ext_localconf and in ext_tables. Calling it in ext_localconf is incorrect, because the tt_content TCA is not yet loaded.

Exactly!

Calling addPlugin in ext_tables.php works correctly in 6.2.4.

Exactly!

The breaking change we have is, that when using the API in a wrong way (comment of addPlugin says: "FOR USE IN ext_tables.php FILES or files in Configuration/TCA/Overrides/*.php") you get an exception instead of a silent failure.

While I understand the troubles you now have with a minor upgrade, I personal opinion is that this level of breaking is OK and wanted.

Christian Kuhn wrote:

OTOH it is not simple for ext devs to decide what exactly should go to ext_localconf and what to ext_tables,

Well I agree with that somehow, on the other hand it is documented like this for ages.

so the problem is home-made (imho both files should go away, but that is a different topic) ...

We all agree on that ;)

Furthermore, we are "only" talking about an icon here, so probably it would be best to kick this exception again from addPlugin() for better compatibility with extensions that are maybe not 100% clean at this point? Other opinions?

... on the other hand, misuse of API is also known to silently fail for ages. This is also why my first iteration of the patch did not have an exception.
I'm not strictly against removing the exception, but if we do so, we should write a warning to sys_log (or any other prominent log, not only deprecation log) as this is clearly a bug in an extension. Letting it fail silently again feels wrong (even it it was this way for long)

Discussion continued in slack and it seems we aligned on the following:

• Exception will stay both in master and 6.2
• Exception will get a minor patch hinting for the wrong API usage of addPlugin() in ext_localconf instead of ext_tables

Additionally, an according wiki entry was created if people search for this exception code: http://wiki.typo3.org/Exception/CMS/1404068038

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

Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/31562