On Tue, Aug 11, 2009 at 10:59 AM, dan nessettdnessett@yahoo.com wrote:
--- On Mon, 8/10/09, Tim Starling tstarling@wikimedia.org wrote:
No, the reason is because LocalSettings.php is in the directory pointed to by $IP, so you have to work out what $IP is before you can include it.
Web entry points need to locate WebStart.php, and command line scripts need to locate maintenance/commandLine.inc. Then either of those two entry scripts can locate the rest of MediaWiki.
Fair enough, but consider the following.
I did a global search over the phase3 directory and got these hits for the string "$IP =" :
.../phase3/config/index.php:30: $IP = dirname( dirname( __FILE__ ) ); .../phase3/config/index.php:1876: $IP = MW_INSTALL_PATH; .../phase3/config/index.php:1878: $IP = dirname( __FILE__ ); .../phase3/includes/WebStart.php:61: $IP = getenv( 'MW_INSTALL_PATH' ); .../phase3/includes/WebStart.php:63: $IP = realpath( '.' ); .../phase3/js2/mwEmbed/php/noMediaWikiConfig.php:11: $IP = realpath(dirname(__FILE__).'/../'); .../phase3/LocalSettings.php:17: $IP = MW_INSTALL_PATH; .../phase3/LocalSettings.php:19: $IP = dirname( __FILE__ ); .../phase3/maintenance/language/validate.php:16: $IP = dirname( __FILE__ ) . '/../..'; .../phase3/maintenance/Maintenance.php:336: $IP = strval( getenv('MW_INSTALL_PATH') ) !== ''
So, it appears that $IP computation is occurring in 6 files. In addition, $IP is adjusted by the relative place of the file in the MW source tree (e.g., in validate.php, $IP is set to dirname( __FILE__ ) . '/../..';) Adjusting paths according to where a file exists in a source tree is fraught with danger. If you ever move the file for some reason, the code breaks.
Why not isolate at least $IP computation in a single function? (Perhaps breaking up LocalSettings.php into two parts is overkill, but certainly cleaning up $IP computation isn't too radical an idea.) Of course, there is the problem of locating the file of the function that does this. One approach is to recognize that php.ini already requires potential modification for MW use. Specifically, the path to PEAR must occur in 'include_path'. It would be a simple matter to add another search directory for locating the initialization code.
Or maybe there is a better way of locating MW initialization code. How its done is an open issue. I am simply arguing that computing the value of $IP by relying on the position of the php file in a source tree is not good software architecture. Experience shows that this kind of thing almost always leads to bugs.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
The problem with "putting it in a single function" is you still have to find where that function is to begin with (I'd assume either GlobalFunctions or install-utils would define this). At which point you're back to the original problem: defining $IP yourself so you can find this.
Yes, we should probably do this all a little more cleanly (at least one unified style would be nice), but constructing it manually is pretty much a given for anything trying to find an entry point, as Tim points out.
-Chad