Closed yaronkoren closed 1 year ago
The conversation in the code has become chaotic (five separate discussions), so let me try to bring it all inline.
As I understand it, there are five extensions that are being discussed here, because they all include an "autoload" call in their composer.json file, which means that they need to be called with "composer update": Semantic Compound Queries (SCQ), Semantic Extra Special Properties (SESP), Semantic Scribunto (SS), Semantic Tasks (ST) and Semantic Forms Select (SFS). (There are other SMW-based extensions that also include the Composer "autoload", like SRF, SBL and SMW itself, but these need to get a "composer update" regardless because of their dependencies, so it doesn't matter.)
I think the whole "Composer autoload" thing is dumb, but it's not my decision. At this point, from what I understand, we have to deal with it in one of two ways: add all five of these extensions to composer.local.json or composer.canasta.json (ST and SFS are already there), or patch all five extensions to add an "autoload" to their extension.json.
@cicalese and @vedmaka - is that description correct? And if so, any preferences on what to do? My own inclination is to go with Cindy's recommendation to patch all five extensions - the less we have to deal with Composer, the better, I think.
@cicalese and @vedmaka - is that description correct? And if so, any preferences on what to do? My own inclination is to go with Cindy's recommendation to patch all five extensions - the less we have to deal with Composer, the better, I think.
One correction. Of the five extensions in question, three of them already specify autoloading in their extension.json files, so we can safely remove them from composer.canasta.json and remove any patches created in this PR. Those extensions are Semantic Extra Special Properties (SESP), Semantic Tasks (ST) and Semantic Forms Select (SFS).
The other two extensions are missing autoload sections in their extension.json files: Semantic Compound Queries (SCQ) and Semantic Scribunto (SS). Those are the only two that I suggest we patch their extension.json files.
@cicalese and @vedmaka - is that description correct?
yep (+taking into account @cicalese comment)
And if so, any preferences on what to do? My own inclination is to go with Cindy's recommendation to patch all five extensions
That's right, in this situation, we either patch the extensions extension.json
or the composer.json
the less we have to deal with Composer, the better, I think.
I am unsure if dealing less with Composer is the right thing to do in the long term. The discussion about using Composer as a main extensions management tool [1] is still in progress here. It could be that managing all the extensions via composer (even non-Composer ones) is a good idea, especially considering the update
problem is somewhat solved here https://github.com/wikimedia/composer-merge-plugin/commit/1b1afcb69fe9fae36eec90d4f3c865e0e79dfff1 and here https://github.com/wikimedia/composer-merge-plugin/pull/257
In theory, this provides a good number of benefits, like having a parsable composer file on the image instead of tons of git clone ..
commands and a free version and extensions dependencies checks on extensions being installed
Thanks to both of you for your comments. @vedmaka - you may be right that, in the future, a pure Composer-based approach will be the way to go. For now, though, this kind of hybrid approach seems to be causing unnecessary complexity and difficulty, both for admins and for us developers.
I have been making changes to the code and to this PR, based on the recommendations. I think the big thing left to do now is to create patches for SCQ and SS. I'm not sure what to do add to extension.json for either extension - can anyone offer a hint?
I have been making changes to the code and to this PR, based on the recommendations. I think the big thing left to do now is to create patches for SCQ and SS. I'm not sure what to do add to extension.json for either extension - can anyone offer a hint?
I was going to work on patches for those two, but I'm currently having trouble running with this patch. I see that there is a merge conflict in the composer.canasta.json file that needs to be resolved. Could you please fix that? The error I'm getting is that it is missing Vector, though, so I'm not sure that's related. I'll let you know if I determine there's another issue.
Oh yeah, I should have noticed that merge conflict. I just fixed it, I think.
I think the big thing left to do now is to create patches for SCQ and SS. I'm not sure what to do add to extension.json for either extension - can anyone offer a hint?
For SemanticCompoundQueries/extension.json:
"load_composer_autoloader":true,
"AutoloadNamespaces": {
"SCQ\\": "src/"
},
"AutoloadClasses": {
"SemanticCompoundQueries": "SemanticCompoundQueries.php"
},
For SemanticScribunto/extension.json:
"load_composer_autoloader":true,
"AutoloadNamespaces": {
"SMW\\Scribunto\\": "src/"
},
"AutoloadClasses": {
"SemanticScribunto": "SemanticScribunto.php"
},
I successfully tested these modifications on my local instance.
Okay, thanks, that's great! Are you sure that the "load_composer_autoloader" lines to be removed, though? They're there for some of the other extensions:
https://github.com/SemanticMediaWiki/SemanticTasks/blob/master/extension.json#L35 https://github.com/SemanticMediaWiki/SemanticFormsSelect/blob/master/extension.json#L66
Okay, thanks, that's great! Are you sure that the "load_composer_autoloader" lines to be removed, though? They're there for some of the other extensions:
https://github.com/SemanticMediaWiki/SemanticTasks/blob/master/extension.json#L35 https://github.com/SemanticMediaWiki/SemanticFormsSelect/blob/master/extension.json#L66
Yes. See https://www.mediawiki.org/wiki/Manual:Extension.json/Schema#load_composer_autoloader. Since those extensions do not have any dependencies on libraries that are specified in composer.json, that line can/should be removed. It should actually be removed from the other extensions you referenced as well, but that's a problem for another day.
By the way, I believe the load_composer_autoloader documentation is not well worded. This flag forces local ./vendor/autoload.php
from the extension directory to be loaded. This has no use when the merge-plugin is in place because when the extension Composer file is loaded through the merge-plugin all the dependencies are added to the main autoload file and no vendor
is present at the extension directory
I just added the autoload patches for SCQ and SS to this PR - including the "load_composer_autoloader" removal. If it turns out that this line is harmless, then it will be easy to, er, remove that removal. In any case, assuming this all works, it will be good to at least add those new autoload lines "upstream" to the SCQ and SS extensions.
:whale: The image based on d73675ae commit has been built with 1.39.1-20230824-280
tag as ghcr.io/canastawiki/canasta:1.39.1-20230824-280
Tested. LGTM.
@vedmaka - it looks like this PR is ready to be merged in, but I can't merge it in unless you cancel your requested change. Could you do that, assuming you think this current code is fine?
The only extensions that are still downloaded via Composer are "helper"/library extensions like DataValues, ParserHooks and Bootstrap (which is to some extent a helper extension).
This PR adds patches for four SMW-based extensions: two (SBL and SRF) get their composer.json requirement for Semantic MediaWiki removed, so that calling "composer install/update" on them does not lead to unnecessary downloads; and the other two (SCQ and SS) get "autoload" added to their extension.json, taking the place of autoload that was called via Composer.
This change should happen concurrently with https://github.com/CanastaWiki/Canasta-DockerCompose/pull/48, which removes all extensions and skins from the file composer.local.json.