This upsets IDE's, as multiple declarations of them
Options:
- JsonData should either depend on EventLogging, and reuse the classes
- Rename the classes in JsonData
- Namespace the classes in JsonData
Description
Description
Event Timeline
Comment Actions
@Nuria The JsonData and EventLogging mediawiki extensions both define a class by the same name:
- https://github.com/wikimedia/mediawiki-extensions-EventLogging/blob/REL1_28/includes/JsonSchema.php
- JsonSchemaException, JsonUtil, TreeRef, JsonTreeRef, JsonSchemaIndex
- https://github.com/wikimedia/mediawiki-extensions-JsonData/blob/REL1_28/JsonSchema.php
- JsonSchemaException, JsonUtil, TreeRef, JsonTreeRef, JsonSchemaIndex
Normally this would cause a fatal error and would never have made it past Jenkins or otherwise close to production. However, due to how PHP autoloading works, a class is only looked for once, and whichever extension wins the load-order race to $wgAutoloadClasses will be used those those class names.
terbium$ mwscript eval.php > foreach( get_included_files() as $file ) if (strpos($file, 'Json')) echo "$file\n"; /srv/mediawiki/php-1.29.0-wmf.14/includes/json/FormatJson.php /srv/mediawiki/php-1.29.0-wmf.14/extensions/EventLogging/includes/JsonSchema.php
Looks like the EventLogging one currently is "winning". So any changes made to JsonData will be ignored. However, it is not unlikely that the Beta cluster or (some of the) Jenkins jobs load them in a different order. Effectively, this is unpredictable and likely to miss potential fatals during development that then pop up later in prod.
@Reedy is suggesting that either one extension depends on the other (which seems undesirable as they're quite unrelated), or that at least one of the sets of classes is renamed or namespaced.
Alternatively:
- We could move this to a separate PHP library https://www.mediawiki.org/wiki/Manual:Developing_libraries and use it in both places using Composer.
- We could drop these in favour of an existing composer package instead of inventing our own.
Comment Actions
I assume you mean JsonConfig? That hasn't seen much recent activity, as far as I can see. However I just noticed that my premise was wrong.
I thought this issue was with JsonConfig (which we deploy in production alongside EventLogging). The aforementioned classes were first implemented in that extension and later copied to EventLogging. But, they've been long removed from JsonConfig (I can't even find it in the history now).
The problem I found in this task was with an extension called "JsonData", which is a third-party extension (not maintained or otherwise used by WMF) that actually copied the classes from EventLogging after the fact.
- JsonData (project info – commits)
- JsonConfig (project info – commits)
As such, the problem lies with that repository (should've used a different class name, or namespace). Either way, there's nothing we can do. The classes used by EventLogging have been adopted and gained references in dozens of repositories, so that wouldn't make much sense to rename at this point.
Comment Actions
Just to clarify for history, these classes were originally written by robla for the JsonData extension (never deployed), and then ori copied them into EventLogging.