CanastaWiki / Canasta

MediaWiki Docker image for Canasta, an all-in-one MediaWiki stack for easy deployment and management of enterprise-ready MediaWiki on production environments.
https://www.canasta.wiki
MIT License
38 stars 28 forks source link

Get rid of cfLoad...() functions #133

Closed yaronkoren closed 2 years ago

yaronkoren commented 2 years ago

Some people (maybe many) will be happy about this, but: I have been convinced that switching away from the cfLoadSkin() and cfLoadExtension() functions is the best course of action for Canasta. I admit that it's not actually the technical arguments that convinced me, but just the seeming consensus on the other side - both among developers and regular users. So, the new plan of action is to keep the four directories (now called extensions/, skins/, canasta-extensions/ and canasta-skins/ - maybe this will change as well?), but to just use wfLoadExtension() and wfLoadSkin() to do that. It seems like there are two main possible approaches for this:

1) Just symlink every built-in extension and skin, so that the main extensions/ and skins/ directories start out with a ton of symlinks each. 2) Something like @jeffw16's suggested approach, of having Canasta, on startup, figure out which extension and skin is contained in which directory, and thus create something like internal symlinks, to enable wfLoadSkin() and wfLoadSkin() to always call the "right" version - the user-installed one if it exists, and the Canasta-packaged one otherwise. (Jeff - please correct me if I'm wrong, and feel free to offer more information in any case.)

No matter what approach is taken, there will still be issues if an admin installs their own version of a Canasta-packaged skin or extension, and then wants to toggle back and forth between the two versions, maybe for testing purposes, but clearly this problem does not outweigh the benefits of getting rid of the cfLoad... functions, for most people.

jeffw16 commented 2 years ago

Here's the suggestion I made a year ago that Yaron is referring to:

  1. In the Docker container, there will be four directories: canasta-extensions, canasta-skins, user-extensions, and user-skins.
  2. canasta- directories have extensions and skins bundled within them. user- directories are actually bind mounts to the host OS where they will simply be known as extensions/ and skins/.
  3. In the Docker container, there will also be the regular extensions/ and skins/ directories, but they will be empty in the image.
  4. During the container's startup initialization, all directories in canasta-extensions/ will be symlinked into extensions/, regardless of whether they are being invoked in LocalSettings or not. Then, all user-extensions/ directories will be symlinked into extensions/, overriding anything that might've already been symlinked before from canasta-extensions/. This allows the sysadmin to bring their own version of the extension rather than using Canasta's. (The analogous operation will be done for skins too.)

The biggest benefit of this will be the end of the numerous problems we've had using extensions and skins that weren't located in the extensions/ or skins/ directories. Unfortunately, many extensions and skins assume they will always be placed in directories called extensions/ and skins/, but this poses a problem with our current setup, where lots of skins don't get called from skins/, but rather, from canasta-skins/, and this causes their image assets to not load properly. (Example: The user icon in Vector at the top of the page next to the username.)

We had a concern in the most recent Canasta cadence meeting about having too many symlinks being a bad idea. I agree with the sentiment that having symlinks all over the place is bad, especially when circular pointers get established in the filesystem, but I think if we carefully use them, it should be fine. And in the Canasta container environment, we are very deliberately choosing what should be symlinked. So, if we are architecturally making these decisions, I think it should be fine.

jeffw16 commented 2 years ago

Will be fixed in #135