Page MenuHomePhabricator

Document $IP is internal (not to change in LocalSettings)
Open, MediumPublic

Description

The $IP variable is used in DefaultSettings.php, which means if you change it in LocalSettings.php, you must also override any DefaultSettings.php variable that depends on it.

It seems we we should either:

  1. Remove all references to $IP in DefaultSettings.php, and use Setup.php (e.g. how $wgStyleDirectory works).
  2. Document that $IP is now an internal variable and should not be set in LocalSettings.php.

Version: 1.22.0
Severity: normal

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:12 AM
bzimport set Reference to bz54483.
bzimport added a subscriber: Unknown Object (MLST).

$IP has already been used to load a bunch of code in WebStart.php long before LocalSettings.php is loaded -- it's WAY too late to change $IP there.

The only semi-correct way I can see to change it is to set the MW_INSTALL_PATH environment variable which can override the dirname(FILE) path, and I'm not 100% convinced that's actually useful for anything in the first place.

Comments in the code seem to indicate it can be used to override expansion of symlinks, but that seems like a wonky idea -- you've already hit at least index.php and includes/WebStart.php from the base directory, so you can't really flip to another version or something.

Krinkle renamed this task from $IP used in DefaultSettings to Document $IP is internal (not to change in LocalSettings).Dec 26 2018, 6:18 AM
Krinkle removed a subscriber: wikibugs-l-list.

Given that we don't actually support changing $IP, should we move it to a constant instead like we recently did for MW_VERSION / $wgVersion?

Yes, though beware of mocking in tests. Code that isn't yet injecting its (in)direct use of $IP should likely continue to use the global.

Also, it'd be neat perhaps if we can align this with the ENV variable MW_INSTALL_PATH for consistency.

This is documented at https://www.mediawiki.org/wiki/Manual:$IP, where it was added by @valerio.bozzolan in 2019:

However, at least since MediaWiki 1.18, manually defining $IP in LocalSettings.php is no longer needed and no longer works as you may expect. It will default to the current working directory, and it can be used without the need to define it manually.

There is now also the MW_INSTALL_PATH runtime constant, defined in Setup.php, which almost all relevant code uses today. The $IP variable is essentially frozen and kept for compatiblity but not used much by default. I suspect most if not all remaining uses can be changed to MW_INSTALL_PATH.

It seems that $IP was never documented in core itself, so there's not a natural place where, someone who might be thinking of using it, would learn about and be discouraged from using it. I suppose de-facto, as being undocumented, it is implied that it shouldn't be used, as is the case for all internal/undocumented global variables. Perhaps for IDEs we can add an /** @internal */ doc block which might be used, although it's not clear that an IDE would treat Setup.php as "the" place that defines it, given that there are various test/bootstrap-related assignments elsewhere as well.

Beyond adding an @internal docblock I suggest we continue adopting MW_INSTALL_PATH, and continue erasing mentions of $IP from wherever it is mentioned so that it isn't considered or thought of in the first place.

Change #1270270 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] Setup: Document `$IP` as internal

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

Change #1270272 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] maintenance: Fix inconsistent path ref in findDeprecated.php

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

Change #1270275 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] maintenance: Replace remaining use of $IP with MW_INSTALL_PATH

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

Change #1270270 merged by jenkins-bot:

[mediawiki/core@master] Setup: Document `$IP` as internal

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

Change #1270272 merged by jenkins-bot:

[mediawiki/core@master] maintenance: Fix inconsistent path ref in findDeprecated.php

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

Change #1270275 merged by jenkins-bot:

[mediawiki/core@master] maintenance: Replace remaining use of $IP with MW_INSTALL_PATH

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