Change 422834 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/core@master] Exclude GlobalPreferences' local-exception prefs from cleanup

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

I've made a patch that adds a check for the *-local-exception preferences, but I rather think this should be done by adding a DeleteUnknownPreferences hook here that extensions could use to add their own bits to the where clause (just because it feels odd to have extension-specific code for Gadgets and GlobalPreferences in core).

I think a hook like this https://gerrit.wikimedia.org/r/#/c/423597/ would be better than listing exceptions for Gadgets and GlobalPreferences in core.

Alternatively, https://gerrit.wikimedia.org/r/#/c/422834/ is the one-line change that adds the exception to core. That doesn't cleanup the preferences if GlobalPreferences is uninstalled though (which is maybe okay, given that's the status quo with regard to Gadgets).

If the hook idea is accepted, I'll make the required change to Gadgets as well as GlobalPreferences.

Change 423600 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/GlobalPreferences@master] Hook into cleanupPreferences.php to prevent deletion of local exceptions

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

Change 423597 had a related patch set uploaded (by Nikerabbit; owner: Samwilson):
[mediawiki/core@master] Add DeleteUnknownPreferences hook

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

@Samwilson: Merged the hook patch (i.e. part 1). Please be sure to document the new hook on Mediawiki.org as well.

Change 422834 abandoned by Samwilson:
Exclude GlobalPreferences' local-exception prefs from cleanup

Reason:
A hook is a better fix for this.

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

Thanks. Will do.

Change 423597 merged by jenkins-bot:
[mediawiki/core@master] Add DeleteUnknownPreferences hook

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

Change 424491 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/Gadgets@master] Prevent gadget preferences from being cleaned up

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

Change 424507 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/core@master] Remove gadget special-case from preferences cleanup

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

Change 424491 merged by jenkins-bot:
[mediawiki/extensions/Gadgets@master] Prevent gadget preferences from being cleaned up

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

In T188966#4110556, @gerritbot wrote:

Change 423597 merged by jenkins-bot:
[mediawiki/core@master] Add DeleteUnknownPreferences hook

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

@Samwilson @kaldari I don't particularly mind the hook itself, but I it took me by surprise. The fact that the clean-up script doesn't know about all valid preferences sounds to me like a bug, and adding this hook works might be a workaround for a rather narrow and specific use case. I worry that may lead to tech debt.

Could you briefly summarise the how and why that results in core not knowing about all preferences? I haven't yet thought about it much , but at first glance, I would expect the existing GetPreferences hook to suffice in ensuring extra preferences are known. That hook isn't pretty, but I assumed the improvements from T178449 would cover this use case. Thanks for your time.

I think the main problem, and the (slightly indirect) reason that this hook needs to exist, is that it is not possible to determine valid preference names without doing so for a particular user. This is why the cleanup script treats anything that is $wgDefaultUserOptions as 'unknown' (along with userjs- preferences).

The script could conceivably iterate through all users and find all known preferences that way, but at some point preference definitions will not be tied to users, so that'd be a short-term solution too. I've still got doing that on my radar, but haven't yet got it all figured out.

You're right in that this hook is perhaps not the most elegant way to fix the problem, but it does at least remove knowledge of Gadgets (and potentially GlobalPreferences) from core, which feels like a bit of an improvement.

@Samwilson Thanks, treating them as patterns for "extra" preference keys, makes perfect sense. And indeed, if it remains a LIKE-pattern kind of thing, I agree it'd be improved to have extensions supply those patterns (instead of core). So consider my concern solved for the moment.

Aside from userjs- though (which indeed is entirely wildcard), it seems to me that Gadgets and GlobalPreferences keys are not wildcards? For Gadgets, all keys are centrally defined and known. For GlobalPreferences, I see a few static keys, and the rest are static additions to existing keys, all of which would be known to preferences factory on the current wiki. Just out of curiosity, which keys are there in GlobalPreferences that are only known in specific user's context?

Change 424507 merged by jenkins-bot:
[mediawiki/core@master] Remove gadget special-case from preferences cleanup

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

There aren't preferences any with variable names (that depend on the user), but the presence of a few depend on the user (e.g. those to do with email are not present for users who don't have sendemail) and so with the current system we can't retrieve all the preference names.

I have started on a patch that makes the $user parameter of PreferencesFactory::getFormDescriptor() optional (so that when it's not provided, one can get the list of all preferences, without the info ones, and with all of those that are conditional). Then, we'd be able to ask PreferencesFactory what preferences exist.

Change 423600 merged by jenkins-bot:
[mediawiki/extensions/GlobalPreferences@master] Hook into cleanupPreferences.php to prevent deletion of local exceptions

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