Page MenuHomePhabricator

RfC: Introduce PlatformSettings.php
Closed, ResolvedPublic

Description

Summary

Introduce a standardized and recommended way for re-distributors and packagers of MediaWiki to be able to tune DefaultSettings.php as appropriate.

Full text at https://www.mediawiki.org/wiki/Requests_for_comment/PlatformSettings.php

Problem

Packagers of MediaWiki often have some customizations to DefaultSettings.php that they want to do.

This is currently done by using an mw-config/overrides file to register a custom LocalSettingsGenerator subclass, so when the installer is run, that needed settings are appended or prepended to the generated LocalSettings.php file.

Because these settings are written into LocalSettings.php, existing installations won't see any changes to these defaults unless they regenerate their LocalSettings.php file (which rarely happens).

Solution 1

The distributor/packager will add an extra PHP file at $IP/includes/PlatformSettings.php. MediaWiki will check, at run-time, if that file exists. If it exists, it would be loaded right after DefaultSettings.php.

For example:

--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -52,6 +52,11 @@
 // Load default settings
 require_once "$IP/includes/DefaultSettings.php";
 
+// Load platform settings if they exist
+if ( file_exists( "$IP/includes/PlatformSettings.php" ) ) {
+       require_once "$IP/includes/PlatformSettings.php";
+}
+

Standardisation of this file path would make adding such file in a package, not a violation of the Do not hack core convention.

Solution 2

Standardise the file location includes/PlatformSettings.php as being safe for packagers to add in their distribution. But make the LocalSettings.php file responsible for including it.

For example, a distributor could use a mw-config/overrides file to register a custom LocalSettingsGenerator subclass and make sure something like require '/usr/share/mediawiki/includes/PlatformSettings.php' is prepended to the generated LocalSettings.php file.

Standardisation of this file path would make adding such file in a package, not a violation of the Do not hack core convention.

Compared to Solution 1:

  • Benefits:
    • Does not require changes to MediaWiki core, since it can be implemented with the existing installer overrides system.
    • No overhead of conditional checks.
    • Strict inclusion (and failure if expected file does not exist).
  • Downsides:
    • Once this is implemented, people with existing MediaWiki installs need to add that line to their LocalSettings.php, that's a one time cost, which @Legoktm says is acceptable.

Event Timeline

Problem:
[..] that snippet is added to the generated LocalSettings.php.
[..] won't see any changes to these defaults unless they regenerate their LocalSettings.php (rare)
Proposal:
[..] MediaWiki will check if that file exists, and if so load it, right after DefaultSettings.php

I agree this is a problem that needs solving, in a way that 1) doesn't involve patching MediaWiki, 2) doesn't require users re-generating their LocalSettings.php file.

I'm not sure this justifies adding complexity to MediaWiki core, though. I have an alternate proposal I'd like to hear your thoughts. Perhaps you considered this already, in which case let me know.

Would it make sense to incorporate this abstraction into the local installation? E.g. rather than effectively writing the generated local settings file to $IP/LocalSettings.php and having that file also be the one that users modify, instead write the generated file elsewhere and have the "real" LocalSettings.php be a package-owned bootstrap file. From the end-user perspective, "their" LocalSettings.php would be the generated one still, e.g. in /etc/ somewhere, and the package/platform settings would be in the bootstrap file, which can be overwritten during upgrades, and can include platform defaults before including the "main" one.

Upon closer look, I see the mediawiki-debian already uses a /etc/mediawiki/LocalSettings.php file and other /etc/ files. The only difference would be that the one in $IP would become a PHP file instead of a symlink.

To clarify, your proposal is basically:

$IP/LocalSettings.php
<?php
# Some Debian-specific settings here...
require_once '/etc/mediawiki/LocalSettings.php';

I'm not really sure how that would work given that the installer would not start since $IP/LocalSettings.php already exists.

FYI afaik file_exists() is not cached if the file does not exist.

FYI afaik file_exists() is not cached if the file does not exist.

Sorry, I'm not sure how that's relevant here?

Sorry, I'm not sure how that's relevant here?

It's in the proposed code change:
https://www.mediawiki.org/wiki/Requests_for_comment/PlatformSettings.php#Proposal

If the file doesn't exist, it will have to find out that it doesn't exist on every request. It's a micro-optimization for sure, but just thought I'd share.

It's not cached if the file exists, either (or at least that used to be the case). If you just use include that will cache it though, if APC is configured the right way.

It's not cached if the file exists, either (or at least that used to be the case). If you just use include that will cache it though, if APC is configured the right way.

include will still trigger warnings if the file doesn't exist, so it could be wrapped with suppressWarnings()/restoreWarnings()? But if there were any problems in PlatformSettings.php, it would quietly silence those too...

To clarify, your proposal is basically:

$IP/LocalSettings.php
<?php
# Some Debian-specific settings here...
require_once '/etc/mediawiki/LocalSettings.php';

Yes, something like that.

I'm not really sure how that would work given that the installer would not start since $IP/LocalSettings.php already exists.

Good point! I didn't realise that's how it works. I thought the "installed already" check was based on $wgUpgradeKey being non-false, but it's actually based on LocalSettings.php existing. That's tricky indeed.

I think InstallerOverrides could still solve this problem though, in reverse. Instead of a bootstrapping file including the user's settings, the generated file for the user can start by explicitly including the Debian settings from a fixed location. (Somewhat similar to how Apache conf includes bundled settings by default?)

I suspect that still leaves the issue of setting applying during the installation process (e.g. the Debian icon in the footer). I don't know if that was intentional, though. I suppose that case would be better solved through overrides/*.php setting the wg-var directly, or through sub-classed WebInstaller method.

I think InstallerOverrides could still solve this problem though, in reverse. Instead of a bootstrapping file including the user's settings, the generated file for the user can start by explicitly including the Debian settings from a fixed location. (Somewhat similar to how Apache conf includes bundled settings by default?)

So this would look like:

/etc/mediawiki/LocalSettings.php
<?php
# This file was automatically generated by the MediaWiki 1.27.4
# installer...
require_once '/usr/share/mediawiki/includes/PlatformSettings.php';
# Installer generated settings below...

I think that would work. The only downside is that current users of the package will need to do this manually, but that's a one-time cost.

I suspect that still leaves the issue of setting applying during the installation process (e.g. the Debian icon in the footer). I don't know if that was intentional, though. I suppose that case would be better solved through overrides/*.php setting the wg-var directly, or through sub-classed WebInstaller method.

That was unintentional, though a nice side-effect. I agree about using the overrides/ system to customize the installer itself.

This sounds like a good idea. I'm not sure about it living at $IP/includes/PlatformSettings.php though; it seems odd to have a file in there that's not in core. Couldn't it just go next to LocalSettings.php?

Also, should PlatformSettings.php only be used for configuration variables? — things like adding to $wgFooterIcons seems to me to belong in an extension (which would of course be loaded via PlatformSettings.php).

If this is specifically about the debian package, wouldn't it be simpler to just to have require_once "$IP/PlatformSettings.php"; at the top of LocalSettings.php (i.e. when it's created)? If it's about generally being able to more easily make a 'distribution' of MediaWiki, I think there needs to be more details about what that means — for example, it'd be cool to be able to set up a wiki (with extensions and [don't worry I'm being silly] maybe even gadgets, templates, and pre-filled pages) and make the whole thing available to others as a easily-installable "thing". In that situation, I think more than just an adjunct LocalSettings.php would be needed.

This sounds like a good idea. I'm not sure about it living at $IP/includes/PlatformSettings.php though; it seems odd to have a file in there that's not in core. Couldn't it just go next to LocalSettings.php?

+1

The less we mix user-editable and non-editable files the better. :)

This sounds like a good idea. I'm not sure about it living at $IP/includes/PlatformSettings.php though; it seems odd to have a file in there that's not in core. Couldn't it just go next to LocalSettings.php?

$IP/PlatformSettings.php? Yes, I'm not tied to any specific location or name. Conceptually I view it more like DefaultSettings.php, which is why I put it next to that instead of LocalSettings.php.

Also, should PlatformSettings.php only be used for configuration variables? — things like adding to $wgFooterIcons seems to me to belong in an extension (which would of course be loaded via PlatformSettings.php).

Why would $wgFooterIcons be more suited for an extension? But yes, my intention was only to tune configuration variables.

If this is specifically about the debian package, wouldn't it be simpler to just to have require_once "$IP/PlatformSettings.php"; at the top of LocalSettings.php (i.e. when it's created)? If it's about generally being able to more easily make a 'distribution' of MediaWiki, I think there needs to be more details about what that means — for example, it'd be cool to be able to set up a wiki (with extensions and [don't worry I'm being silly] maybe even gadgets, templates, and pre-filled pages) and make the whole thing available to others as a easily-installable "thing". In that situation, I think more than just an adjunct LocalSettings.php would be needed.

That was Krinkle's suggestion. My main usecase and intention in this RfC is to make the experience better for the Debian package. I'm also interested in how to pre-populate wiki pages with content, but that already has some tasks and probably deserves its own RfC :)

This sounds like a good idea. I'm not sure about it living at $IP/includes/PlatformSettings.php though; it seems odd to have a file in there that's not in core. Couldn't it just go next to LocalSettings.php?

+1

The less we mix user-editable and non-editable files the better. :)

PlatformSettings.php is not supposed to be a user-editable file.

PlatformSettings.php is not supposed to be a user-editable file.

Oh, great! then it ought to be in includes and there is no reason it wouldn't exist, so no need for file_exists(). :)

@dbarratt I think @Legoktm means that it's not user editable, but it's also an optional file and will not exist in core (hence the file_exists).

That was Krinkle's suggestion. My main usecase and intention in this RfC is to make the experience better for the Debian package.

What about doing it the other way around then, and having a separate /etc/mediawiki/Settings.php file that gets included by the package-installed LocalSettings.php? That way, people get a blank file to modify only if they need to, and the package can do whatever it wants over time with LocalSettings.php.

On a related note, what is the plan for extensions being installed for the debian package? Would they be packaged in their own separate packages (or maybe bundles of)? If so, would they be added e.g. /etc/mediawiki/conf.d/BlahExtension.php containing wfLoadExtension()? Or appending to PlatformSettings.php?

@dbarratt I think @Legoktm means that it's not user editable, but it's also an optional file and will not exist in core (hence the file_exists).

Yes.

That was Krinkle's suggestion. My main usecase and intention in this RfC is to make the experience better for the Debian package.

What about doing it the other way around then, and having a separate /etc/mediawiki/Settings.php file that gets included by the package-installed LocalSettings.php? That way, people get a blank file to modify only if they need to, and the package can do whatever it wants over time with LocalSettings.php.

Errr, we discussed that above, it won't work because the installer won't run if the package has already provided a LocalSettings.php: T182020#3810322

On a related note, what is the plan for extensions being installed for the debian package? Would they be packaged in their own separate packages (or maybe bundles of)? If so, would they be added e.g. /etc/mediawiki/conf.d/BlahExtension.php containing wfLoadExtension()? Or appending to PlatformSettings.php?

That's pretty unrelated to this RfC but non-tarball bundled extensions could be packaged and installed to the right place on the filesystem, but it's up to the sysadmin to manually add the wfLoadExtension(...) to LocalSettings.php, which I think is the best experience until there's a proper extension management system.

How about making it more flexible and having the installer do the file_exists check on PlatformSettings (or mw-config/InstallerHooks.php or what have you) and maybe define a simple interface for that include so that it can both prepend/append to LocalSettings and tweak the installation process (so that people can brand the installation process, for example)?

@dbarratt I think @Legoktm means that it's not user editable, but it's also an optional file and will not exist in core (hence the file_exists).

Yes.

If it's not part of the repo, then it's technically user-editable (even if it's a generated file). So then it ought to be in the root alongside LocalSettings.php (or it could be in a new folder all-together).

Errr, we discussed that above, it won't work because the installer won't run if the package has already provided a LocalSettings.php: T182020#3810322

Perhaps it would be better to fix the installer instead?
T172555: If LocalSettings.php exists, but database is empty, user should be prompted to install and not download new LocalSettings.php

What about doing it the other way around then, and having a separate /etc/mediawiki/Settings.php file that gets included by the package-installed LocalSettings.php?

This is basically exactly what Drupal does:
https://github.com/drupal/drupal/blob/8.4.2/sites/default/default.settings.php#L772-L785
err... you have a single settings file (settings.php) and you can load other files from within that file (and those other files could be dynamically updated externally, but the main settings file (which is LocalSettings.php for us) is only generated once and then edited by the user after that.

What about doing it the other way around then, and having a separate /etc/mediawiki/Settings.php file that gets included by the package-installed LocalSettings.php?

Errr, we discussed that above, it won't work because the installer won't run if the package has already provided a LocalSettings.php: T182020#3810322

You mean the MediaWiki installer yeah? Why does that need to run when you make a change to PlatformSettings.php?

I think it's an okay way to go, given that it's a one-time upgrade change for the sysadmin to make. Although, perhaps the package can modify it? Or rather move it to /etc/mediawiki/Settings.php (or whatever) and provide a new /etc/mediawiki/LocalSettings.php that loads the former.

I'm just not sure core MediaWiki should be modified to account for short-comings in any particular distribution process.

That's pretty unrelated to this RfC [...]

I think it is a bit related. This RFC is about re-distributors and packagers of MediaWiki, and I reckon how they redistribute and package extensions is pretty on-topic. If we can make a change now that makes it easier for that stuff, then that'd be great. You say it's up to sysadmins to install their own extensions and edit LocalSettings, but if they're doing that why are they bothering with the debian package?

Are there other packaging systems (e.g. Softaculous or any of those one-click-install systems) that would benefit from PlatformSettings.php?

@Legoktm This looks pretty straight forward to me. We discussed this at the TechCom meeting the other day, and were wondering whether we could even put this on last call without an IRC discussion. I see three options for now: just continue the discussion here, request an IRC meeting (probably for January), or go streight for a last call. Which do you prefer?

@Legoktm This looks pretty straight forward to me. We discussed this at the TechCom meeting the other day, and were wondering whether we could even put this on last call without an IRC discussion. I see three options for now: just continue the discussion here, request an IRC meeting (probably for January), or go streight for a last call. Which do you prefer?

I added Krinkle's proposal as "Idea 2" to the wiki page. If @Samwilson is on board with that proposal, I think this can go to last call. Since that idea requires no changes to MediaWiki, we would basically be agreeing on the following statement to resolve the RfC:

Re-distributors and packagers of MediaWiki can create $IP/includes/PlatformSettings.php, which will contain defaults that are tuned to the specific distribution. The distributor is responsible for creating their own LocalSettingsGenerator to require that file from the installer generated LocalSettings.php.

If this is approved, I'll document that somewhere (probably in a "guide for packagers" that I want to write eventually).

How about making it more flexible and having the installer do the file_exists check on PlatformSettings (or mw-config/InstallerHooks.php or what have you) and maybe define a simple interface for that include so that it can both prepend/append to LocalSettings and tweak the installation process (so that people can brand the installation process, for example)?

This already exists, see https://phabricator.wikimedia.org/source/mediawiki/browse/master/mw-config/overrides/. The Debian package's override is linked in the RfC: https://phabricator.wikimedia.org/diffusion/MDEB/browse/master/debian/patches/debian_installer_overrides.diff

What about doing it the other way around then, and having a separate /etc/mediawiki/Settings.php file that gets included by the package-installed LocalSettings.php?

Errr, we discussed that above, it won't work because the installer won't run if the package has already provided a LocalSettings.php: T182020#3810322

You mean the MediaWiki installer yeah? Why does that need to run when you make a change to PlatformSettings.php?

I think it's an okay way to go, given that it's a one-time upgrade change for the sysadmin to make. Although, perhaps the package can modify it? Or rather move it to /etc/mediawiki/Settings.php (or whatever) and provide a new /etc/mediawiki/LocalSettings.php that loads the former.

I'm just not sure core MediaWiki should be modified to account for short-comings in any particular distribution process.

To be honest, I've completely lost track of exactly what you're proposing, and what would go where. Let me try and clarify what the 3 relevant files are here:

  1. DefaultSettings.php - shipped by MediaWiki, not to be edited by packagers or users.
  2. PlatformSettings.php (proposed) - shipped by the packager, not to be edited by users.
  3. LocalSettings.php - created by the installer, editable by users, but not packagers

File paths (using the Debian layout as an example):

  1. /usr/share/mediawiki/includes/DefaultSettings.php - in a place that suggests it shouldn't be edited by users (obviously root can edit anything, and we can't stop that)
  2. /usr/share/mediawiki/includes/PlatformSettings.php - follows path of DefaultSettings.php, since users should not edit it
  3. /etc/mediawiki/LocalSettings.php - users are supposed to edit it, and this /etc is where configuration should go.

There is some discussion of whether PlatformSettings.php should be in includes/ or not but it definitely should not be in /etc.

The current proposal ("Idea 2") that was suggested by Krinkle was that when the installer generates LocalSettings.php, it will add at the very top a line like require '/usr/share/mediawiki/includes/PlatformSettings.php'; to load those, and then process the rest of the user-specified settings.

This RFC is about re-distributors and packagers of MediaWiki, [...]

No, it's not. This RfC is about what I wrote in the introduction of this task: "Introduce a standardized and recommended way for re-distributors and packagers of MediaWiki to be able to tune DefaultSettings.php as appropriate.". If you want to have a larger discussion about re-distributors and packagers of MediaWiki, please file a separate RfC.

You say it's up to sysadmins to install their own extensions and edit LocalSettings, but if they're doing that why are they bothering with the debian package?

I can't tell if you're joking or not. In any case:

  • Ease of downloading, just run sudo apt install mediawiki instead of needing to download tarballs from random web servers. apt automatically handles verification using GPG, and the Debian project takes care of ensuring geographically convenient mirrors.
  • Ease of updating, just run sudo apt update instead of needing to download and apply patch files or upload tarballs over existing directories
  • Security support from Debian project
  • PHP plus extensions and other dependencies automatically installed/setup
  • apache configuration provided and installed for basic wiki setups
  • Assurance that it has gone through review to only contain free software and can be re-distributed, and meets Debian's high standards in this regard

Should I go on? Even at Wikimedia the standard is that it needs to have a Debian package to get deployed/installed in production (with MediaWiki and other invented here software as the exception).

So yes, extension management is not ideal, but that's an entirely separate discussion (that I also plan to work on soon, entirely unrelatedly).

Are there other packaging systems (e.g. Softaculous or any of those one-click-install systems) that would benefit from PlatformSettings.php?

Yes. This RfC uses the Debian package as an example and was my impetus for starting this, but the proposed solution is entirely generic. An older experiment I had to tune MediaWiki for GoogleAppEngine comes to mind here: https://phabricator.wikimedia.org/diffusion/EGAE/browse/master/GoogleAppEngine.php

@Legoktm Hm.. if the Debian package ensures the inclusion happens from the generated LocalSettings.php, and the Debian package creates/maintains PlatformSettings.php, then it might make more sense for that file to live elsewhere (e.g. in /etc or /lib). The proposed standard location in included/ worries me given that is meant to be a part of MediaWiki core not intended to be modified by users (RE: "Don't hack core"), which arguably should apply to packagers as well. I can understand the occasional patch file, but it seems odd to standardise. Especially if nothing in the design warrants that as the location, and could potentially conflict with something in the future.

@Legoktm Hm.. if the Debian package ensures the inclusion happens from the generated LocalSettings.php, and the Debian package creates/maintains PlatformSettings.php, then it might make more sense for that file to live elsewhere (e.g. in /etc or /lib).

Per https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard#Directory_structure it could potentially go in /etc yes, but as I stated earlier, I don't want to give users the impression that they can/should edit PlatformSettings.php, so I'd rather place it with MediaWiki.

The proposed standard location in included/ worries me given that is meant to be a part of MediaWiki core not intended to be modified by users (RE: "Don't hack core"), which arguably should apply to packagers as well. I can understand the occasional patch file, but it seems odd to standardise. Especially if nothing in the design warrants that as the location, and could potentially conflict with something in the future.

Err, the whole point of this RfC is to standardize. If we're not going to standardize on anything, then I can close this as declined. I expect that once this is decided and documented, that other redistributors of MediaWiki will take advantage of it as well. Also this is specifically scoped to changing configuration settings - not patching MediaWiki code.

I also did say earlier that it doesn't have to be in includes/, it could be in /usr/share/mediawiki/PlatformSettings.php, so it would be right next to LocalSettings.php. Also if the solution is to put it in some other top-level directory, that doesn't work for non-OS level packages where MediaWiki is just installed into one directory (standard installation layout).

Good points. includes/ sounds good to me after re-evaluating this.

I'm still somewhat uncertain how well we can standardise it, given there'd be no actual code in core for it. But I suppose formal agreement in form of an RFC would suffice as future-reference to ensure we never create a file by this name in core, and to serve as non-normative recommendation to packagers.

As for actual reference in core, I suppose there is one thing we can do: Add to gitignore. That should help a little in avoiding creation of such file (in a future if we forgot about this RFC).

Krinkle triaged this task as Medium priority.Dec 12 2017, 2:43 AM
Krinkle added a project: MediaWiki-General.

I'm still somewhat uncertain how well we can standardise it, given there'd be no actual code in core for it. But I suppose formal agreement in form of an RFC would suffice as future-reference to ensure we never create a file by this name in core, and to serve as non-normative recommendation to packagers.

As for actual reference in core, I suppose there is one thing we can do: Add to gitignore. That should help a little in avoiding creation of such file (in a future if we forgot about this RFC).

We can also document it in the existing docs/distributors.txt.

I think not having the conditional inclusion in includes/Setup.php sounds great, and those are good points re documentation and gitignore. Regarding the location of PlatformSettings.php, if it must go in /includes then I'm sure that's fine, but if the reasoning is that things in /etc are all user-editable, I think there are lots of package-provided files there that are generally considered not 'very' user-editable. I mean things like anything with a conf.d directory (e.g. PHP) where there's a 'master' config file and a set of module/plugin/whatever config files that all get loaded — usually, the end user would be encouraged to add their own small config file and not edit the master one. A similar thing could be done here.

@Samwilson I had the same impression, but I wasn't sure whether or not those packages (PHP indeed, but also Apache) overwrite their master file during upgrades, or whether it is only created once upon initial installation. If it is socially acceptable for Debian packages to overwrite their master files in /etc/ then I agree it could make sense to place it there. But that would require expanding the directory structure used in /etc/ used by mediawiki-debian. Right now it's a flat directory that just contains two files (LocalSettings and apache.conf).

As I explained in T182020#3829876 I want this solution to also work for non-OS level packages that put MediaWiki into one directory, which is the normal way of installing it.

I've updated the task description with a summary, problem statement and copy of the two ideas described.

TechCom proposes to approve this RFC, with no strong preference for either of the suggested solutions. If no pertinent issues remain unaddressed by January 17, this RFC will be approved for implementation.

This RFC has been approved for implementation (with no preference for either of the options presented).

Thanks, I'll most likely implement option #2 early next month.

Change 423577 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Introduce PlatformSettings.php

https://gerrit.wikimedia.org/r/423577

I made a slight change in the implementation:

For example, a distributor could use a mw-config/overrides file to register a custom LocalSettingsGenerator subclass and make sure something like require '/usr/share/mediawiki/includes/PlatformSettings.php' is prepended to the generated LocalSettings.php file.

Instead of requiring a custom LocalSettingsGenerator, I just implemented it in the default LocalSettingsGenerator if the file exists.

Change 423577 merged by jenkins-bot:
[mediawiki/core@master] installer: Add support for PlatformSettings.php

https://gerrit.wikimedia.org/r/423577

Any final changes before we declare this Resolved? Documentation updates? An example of use in the Debian package?

Legoktm claimed this task.

Any final changes before we declare this Resolved? Documentation updates? An example of use in the Debian package?

I can't find any other documentation on-wiki that is relevant for distributors that needs updating. I'll update the Debian package as an example whenever the first beta/rc is released.

Change 568423 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/core@master] Add includes/PlatformSettings.php to gitignore

https://gerrit.wikimedia.org/r/568423

Change 568423 merged by jenkins-bot:
[mediawiki/core@master] Add includes/PlatformSettings.php to .gitignore

https://gerrit.wikimedia.org/r/568423