CanastaWiki / Canasta-CLI

The Canasta command line interface, written in Go
MIT License
5 stars 14 forks source link

Allow modification to default composer installed extensions #94

Open cicalese opened 1 year ago

cicalese commented 1 year ago

Move composer installed extensions to volumed in composer.local.json file. Add support for specifying an alternative composer.local.json file at create/import time.

Optionally extends CanastaWiki/Canasta#270 and CanastaWiki/Canasta-DockerCompose#47 Should only be merged if the other two pull requests are merged first.

yaronkoren commented 1 year ago

We've already discussed this patch a little elsewhere, but we should talk about it here. I know this flag is analogous to the existing "override" flag (which places a file at docker-compose.override.yml, just like this one places a file at composer.local.json), but the "override" command seems more useful than this one. There is no existing file at docker-compose.override.yml, which means that anyone who wants to do an override can indeed simply create a new file and put it there. On the other hand, composer.local.json already exists - and it's populated with all the Canasta-recommended stuff - so it seems less likely that someone will want to wipe out that file and put in their own file - much more likely, I would think, is that someone would want to keep the existing file but just want to add one or two lines, change a version number, etc. But I look forward to hearing other opinions.

(If this flag does get added, by the way, I think it would be better named "composerFile" than "composer", but that's a more minor issue. (Arguably the same is true for "override".))

cicalese commented 1 year ago

I can think of two admittedly edge cases where one might want to provide the composer file.

You might want to create a wiki using an older version of one of the extensions that has an incompatible database schema. Since most extensions do not support downgrading a database schema, you would want to create the database with the earlier schema initially.

In the case of importing an existing wiki from an instance running an older version of the extensions, you might want to import with the existing versions followed by upgrading to the newer versions, rather than having these happen in a single step.

Neither of those cases are particularly strong, but I would ask why you would want to prevent them? One argument could be that you want to limit the number of command line options.

yaronkoren commented 1 year ago

Okay, I think I understand these two cases you're describing.... but are they correct? That is, all the extensions and skins in the "Jeroen bundle" will get loaded by default, via Composer. But they will not be enabled by default, I don't think - which means that update.php will not apply to them, until a wfLoad...() call is added manually. Or am I missing something?

You do make a reasonable point that, regardless of the benefits, there's no harm in adding another flag. Yes, all things being equal, I do think it makes sense to keep the set of options smaller. Of course, if there's a good reason to add an option, it should be added.

cicalese commented 1 year ago

Okay, I think I understand these two cases you're describing.... but are they correct? That is, all the extensions and skins in the "Jeroen bundle" will get loaded by default, via Composer. But they will not be enabled by default, I don't think - which means that update.php will not apply to them, until a wfLoad...() call is added manually. Or am I missing something?

In the case of import, it is possible that the LocalSettings.php file provided enables the extensions

You are absolutely correct in the case of creation of a new wiki. Since the extensions are not enabled, they will not affect the database schema in that case.

So, maybe this is only really helpful in the case of import, and we'd want to discourage running with out of date extensions for newly created wikis by not providing the option on create. The only exception to that is if you wanted to create an empty wiki and do the database import manually rather than letting CLI do it for you, maybe if the wiki is large? It is a tradeoff between hiding the complexity behind CLI commands and letting the user have more flexibility.

yaronkoren commented 1 year ago

Okay, that makes sense. But that same problem (unwanted automatic updating of DB tables when importing a wiki) holds for all the non-Composer extensions also, no? Hopefully there's a way to get around it for those, like commenting out all the wfLoadExtension() calls first - if so, I don't see a need to have a 2nd option for just the Composer extensions. (Though you could argue that SMW is a special case, due to its importance.)

cicalese commented 1 year ago

Maybe I'm mistaken, but I had thought that putting different versions of the non-composer extensions in the volumed in extensions directory prior to running import would result in them being used rather than the built-in versions. Wouldn't they be symbolically linked from extensions to user-extensions rather than the ones in canasta-extensions?

yaronkoren commented 1 year ago

Oh yes, that's true... there is a second way, for the non-Composer extensions, to defer a DB upgrade: install the previous version of the extension in the user-extensions/ directory, and keep it there until you want to do the upgrade. I hadn't thought about that.

But is this even a reasonable approach, for either group of extensions? There's no guarantee that an older version of any extension will work when doing a big MediaWiki version upgrade - whether the wiki is populated or empty. Can't we just tell users to "bite the bullet"?

cicalese commented 1 year ago

If you have an existing wiki that you want to migrate to Canasta that is running a couple of older extensions, it seems not unreasonable to do the migration, make sure things work under Canasta, then upgrade the extensions (by removing the older versions from the volumed in extension directory and restarting the container) as a second step. You already know that the older extensions work together, because you're migrating an existing wiki that is working with those extension versions. You're just trying to be able to isolate any issues that come from the Canasta migration from any issues that might come from the extension upgrades. Besides, unless you are proposing a way to prevent users from putting older versions of an extension in the volumed in extensions directory, this is indeed possible (if not advisable or supported) behavior.

yaronkoren commented 1 year ago

Okay, everything you're saying makes sense, unfortunately. :) Still, this whole issue of making all sorts of preparations before calling "canasta import" in order to avoid extension DB updates - overriding composer.json, installing one's own version of the non-Composer extensions - seems like a real hassle: you basically have to "get all your ducks in a row" before doing the import, or else it's too late. (Actually, the handling for the non-Composer extensions seems even more extreme.) Maybe it would be better to add a "no-update" flag to "canasta import", so that admins can call update.php later, when they're ready to do it? Or would that introduce its own complications?

cicalese commented 1 year ago

I can't think of any complications aside from folks forgetting to run the update later. I don't have a strong opinion either way.

yaronkoren commented 1 year ago

Okay, it's good to know that this might be feasible. A "no-update" flag for "canasta import" seems like a much more straightfoward solution than requiring the admin to try to make sure everything is set up perfectly, as far as the "old" version of extensions, before they do the import.