r55130 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r55129‎ | r55130 | r55131 >
Date:08:31, 16 August 2009
Author:tstarling
Status:ok
Tags:
Comment:
Fixes for Shinjiman's Special:Version updates r49995-r50121:
* Fixed formal parameter bloat in formatCredits() by passing the whole $extension array to the function and interpreting it there instead of in the caller.
* Fixed the overloading of getSvnRevision() with boolean parameters by splitting it into getSvnInfo(), which gets an associative array, and getSvnRevision(), which provides only the old behaviour as used by external callers like ApiQuerySiteinfo.
* A few tweaks and rewrites for other inelegant code
Modified paths:
  • /trunk/phase3/includes/specials/SpecialVersion.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialVersion.php
@@ -12,6 +12,13 @@
1313 class SpecialVersion extends SpecialPage {
1414 private $firstExtOpened = true;
1515
 16+ static $viewvcUrls = array(
 17+ 'svn+ssh://svn.wikimedia.org/svnroot/mediawiki' => 'http://svn.wikimedia.org/viewvc/mediawiki',
 18+ 'http://svn.wikimedia.org/svnroot/mediawiki' => 'http://svn.wikimedia.org/viewvc/mediawiki',
 19+ # Doesn't work at the time of writing but maybe some day:
 20+ 'https://svn.wikimedia.org/viewvc/mediawiki' => 'http://svn.wikimedia.org/viewvc/mediawiki',
 21+ );
 22+
1623 function __construct(){
1724 parent::__construct( 'Version' );
1825 }
@@ -125,35 +132,49 @@
126133 public static function getVersion( $flags = '' ) {
127134 global $wgVersion, $IP;
128135 wfProfileIn( __METHOD__ );
129 - $svn = self::getSvnRevision( $IP, false, false , false );
130 - $svnCo = self::getSvnRevision( $IP, true, false , false );
131 - if ( !$svn ) {
 136+
 137+ $info = self::getSvnInfo( $IP );
 138+ if ( !$info ) {
132139 $version = $wgVersion;
133140 } elseif( $flags === 'nodb' ) {
134 - $version = "$wgVersion (r$svnCo)";
 141+ $version = "$wgVersion (r{$info['checkout-rev']})";
135142 } else {
136 - $version = $wgVersion . wfMsg( 'version-svn-revision', $svn, $svnCo );
 143+ $version = $wgVersion .
 144+ wfMsg(
 145+ 'version-svn-revision',
 146+ isset( $info['directory-rev'] ) ? $info['directory-rev'] : '',
 147+ $info['checkout-rev']
 148+ );
137149 }
 150+
138151 wfProfileOut( __METHOD__ );
139152 return $version;
140153 }
141154
142155 /**
143 - * Return a string of the MediaWiki version with a link to SVN revision if
144 - * available
 156+ * Return a wikitext-formatted string of the MediaWiki version with a link to
 157+ * the SVN revision if available
145158 *
146159 * @return mixed
147160 */
148161 public static function getVersionLinked() {
149162 global $wgVersion, $IP;
150163 wfProfileIn( __METHOD__ );
151 - $svn = self::getSvnRevision( $IP, false, false, false );
152 - $svnCo = self::getSvnRevision( $IP, true, false, false );
153 - $svnDir = self::getSvnRevision( $IP, true, false, true );
154 - $viewvcStart = 'http://svn.wikimedia.org/viewvc/mediawiki/';
155 - $viewvcEnd = '/?pathrev=';
156 - $viewvc = $viewvcStart . $svnDir . $viewvcEnd;
157 - $version = $svn ? $wgVersion . " [{$viewvc}{$svnCo} " . wfMsg( 'version-svn-revision', $svn, $svnCo ) . ']' : $wgVersion;
 164+ $info = self::getSvnInfo( $IP );
 165+ if ( isset( $info['checkout-rev'] ) ) {
 166+ $linkText = wfMsg(
 167+ 'version-svn-revision',
 168+ isset( $info['directory-rev'] ) ? $info['directory-rev'] : '',
 169+ $info['checkout-rev']
 170+ );
 171+ if ( isset( $info['viewvc-url'] ) ) {
 172+ $version = "$wgVersion [{$info['viewvc-url']} $linkText]";
 173+ } else {
 174+ $version = "$wgVersion $linkText";
 175+ }
 176+ } else {
 177+ $version = $wgVersion;
 178+ }
158179 wfProfileOut( __METHOD__ );
159180 return $version;
160181 }
@@ -184,32 +205,7 @@
185206 usort( $wgExtensionCredits[$type], array( $this, 'compare' ) );
186207
187208 foreach ( $wgExtensionCredits[$type] as $extension ) {
188 - $version = null;
189 - $subVersion = null;
190 - $subVersionCo = null;
191 - $viewvc = null;
192 - if ( isset( $extension['path'] ) ) {
193 - $subVersion = self::getSvnRevision(dirname($extension['path']), false, true, false);
194 - $subVersionCo = self::getSvnRevision(dirname($extension['path']), true, true, false);
195 - $subVersionDir = self::getSvnRevision(dirname($extension['path']), false, true, true);
196 - if ($subVersionDir)
197 - $viewvc = $subVersionDir . $subVersionCo;
198 - }
199 - if ( isset( $extension['version'] ) ) {
200 - $version = $extension['version'];
201 - }
202 -
203 - $out .= $this->formatCredits(
204 - isset ( $extension['name'] ) ? $extension['name'] : '',
205 - $version,
206 - $subVersion,
207 - $subVersionCo,
208 - $viewvc,
209 - isset ( $extension['author'] ) ? $extension['author'] : '',
210 - isset ( $extension['url'] ) ? $extension['url'] : null,
211 - isset ( $extension['description'] ) ? $extension['description'] : '',
212 - isset ( $extension['descriptionmsg'] ) ? $extension['descriptionmsg'] : null
213 - );
 209+ $out .= $this->formatCredits( $extension );
214210 }
215211 }
216212 }
@@ -251,15 +247,46 @@
252248 }
253249 }
254250
255 - function formatCredits( $name, $version = null, $subVersion = null, $subVersionCo = null, $subVersionURL = null, $author = null, $url = null, $description = null, $descriptionMsg = null ) {
256 - $haveSubversion = $subVersion;
257 - $extension = isset( $url ) ? "[$url $name]" : $name;
258 - $version = isset( $version ) ? '<span class="mw-version-ext-version">' . wfMsg( 'version-version', $version ) . '</span>' : '';
259 - $subVersion = isset( $subVersion ) ? wfMsg( 'version-svn-revision', $subVersion, $subVersionCo ) : '';
260 - $subVersion = isset( $subVersionURL ) ? "[$subVersionURL $subVersion]" : $subVersion;
 251+ function formatCredits( $extension ) {
 252+ $name = isset( $extension['name'] ) ? $extension['name'] : '[no name]';
 253+ if ( isset( $extension['path'] ) ) {
 254+ $svnInfo = self::getSvnInfo( dirname($extension['path']) );
 255+ $directoryRev = isset( $svnInfo['directory-rev'] ) ? $svnInfo['directory-rev'] : null;
 256+ $checkoutRev = isset( $svnInfo['checkout-rev'] ) ? $svnInfo['checkout-rev'] : null;
 257+ $viewvcUrl = isset( $svnInfo['viewvc-url'] ) ? $svnInfo['viewvc-url'] : null;
 258+ } else {
 259+ $directoryRev = null;
 260+ $checkoutRev = null;
 261+ $viewvcUrl = null;
 262+ }
261263
262 - # Look for a localized description
263 - if( isset( $descriptionMsg ) ) {
 264+ # Make main link (or just the name if there is no URL)
 265+ if ( isset( $extension['url'] ) ) {
 266+ $mainLink = "[{$extension['url']} $name]";
 267+ } else {
 268+ $mainLink = $name;
 269+ }
 270+ if ( isset( $extension['version'] ) ) {
 271+ $versionText = '<span class="mw-version-ext-version">' .
 272+ wfMsg( 'version-version', $extension['version'] ) .
 273+ '</span>';
 274+ } else {
 275+ $versionText = '';
 276+ }
 277+
 278+ # Make subversion text/link
 279+ if ( $checkoutRev ) {
 280+ $svnText = wfMsg( 'version-svn-revision', $directoryRev, $checkoutRev );
 281+ $svnText = isset( $viewvcUrl ) ? "[$viewvcUrl $svnText]" : $svnText;
 282+ } else {
 283+ $svnText = false;
 284+ }
 285+
 286+ # Make description text
 287+ $description = isset ( $extension['description'] ) ? $extension['description'] : '';
 288+ if( isset ( $extension['descriptionmsg'] ) ) {
 289+ # Look for a localized description
 290+ $descriptionMsg = $extension['descriptionmsg'];
264291 if( is_array( $descriptionMsg ) ) {
265292 $descriptionMsgKey = $descriptionMsg[0]; // Get the message key
266293 array_shift( $descriptionMsg ); // Shift out the message key to get the parameters only
@@ -273,19 +300,19 @@
274301 }
275302 }
276303
277 - if ( $haveSubversion ) {
278 - $extNameVer = "<tr>
279 - <td><em>$extension $version</em></td>
280 - <td><em>$subVersion</em></td>";
 304+ if ( $svnText !== false ) {
 305+ $extNameVer = "<tr>
 306+ <td><em>$mainLink $versionText</em></td>
 307+ <td><em>$svnText</em></td>";
281308 } else {
282 - $extNameVer = "<tr>
283 - <td colspan=\"2\"><em>$extension $version</em></td>";
 309+ $extNameVer = "<tr>
 310+ <td colspan=\"2\"><em>$mainLink $versionText</em></td>";
284311 }
 312+ $author = isset ( $extension['author'] ) ? $extension['author'] : array();
285313 $extDescAuthor = "<td>$description</td>
286 - <td>" . $this->listToText( (array)$author ) . "</td>
287 - </tr>\n";
288 - return $ret = $extNameVer . $extDescAuthor;
289 - return $ret;
 314+ <td>" . $this->listToText( (array)$author ) . "</td>
 315+ </tr>\n";
 316+ return $extNameVer . $extDescAuthor;
290317 }
291318
292319 /**
@@ -384,18 +411,20 @@
385412 }
386413
387414 /**
388 - * Retrieve the revision number of a Subversion working directory.
 415+ * Get an associative array of information about a given path, from its .svn
 416+ * subdirectory. Returns false on error, such as if the directory was not
 417+ * checked out with subversion.
389418 *
390 - * @param String $dir Directory of the svn checkout
391 - * @param Boolean $coRev optional to return value whether is Last Modified
392 - * or Checkout revision number
393 - * @param Boolean $extension optional to check the path whether is from
394 - * Wikimedia SVN server or not
395 - * @param Boolean $relPath optional to get the end part of the checkout path
396 - * @return mixed revision number as int, end part of the checkout path,
397 - * or false if not a SVN checkout
 419+ * Returned keys are:
 420+ * Required:
 421+ * checkout-rev The revision which was checked out
 422+ * Optional:
 423+ * directory-rev The revision when the directory was last modified
 424+ * url The subversion URL of the directory
 425+ * repo-url The base URL of the repository
 426+ * viewvc-url A ViewVC URL pointing to the checked-out revision
398427 */
399 - public static function getSvnRevision( $dir, $coRev = false, $extension = false, $relPath = false) {
 428+ public static function getSvnInfo( $dir ) {
400429 // http://svnbook.red-bean.com/nightly/en/svn.developer.insidewc.html
401430 $entries = $dir . '/.svn/entries';
402431
@@ -403,10 +432,13 @@
404433 return false;
405434 }
406435
407 - $content = file( $entries );
 436+ $lines = file( $entries );
 437+ if ( !count( $lines ) ) {
 438+ return false;
 439+ }
408440
409441 // check if file is xml (subversion release <= 1.3) or not (subversion release = 1.4)
410 - if( preg_match( '/^<\?xml/', $content[0] ) ) {
 442+ if( preg_match( '/^<\?xml/', $lines[0] ) ) {
411443 // subversion is release <= 1.3
412444 if( !function_exists( 'simplexml_load_file' ) ) {
413445 // We could fall back to expat... YUCK
@@ -423,51 +455,59 @@
424456 if( $xml->entry[0]['name'] == '' ) {
425457 // The directory entry should always have a revision marker.
426458 if( $entry['revision'] ) {
427 - return intval( $entry['revision'] );
 459+ return array( 'checkout-rev' => intval( $entry['revision'] ) );
428460 }
429461 }
430462 }
431463 }
432464 return false;
433 - } else {
434 - // subversion is release 1.4 or above
435 - if ($relPath) {
436 - $endPath = strstr( $content[4], 'tags' );
437 - if (!$endPath) {
438 - $endPath = strstr( $content[4], 'branches' );
439 - if (!$endPath) {
440 - $endPath = strstr( $content[4], 'trunk' );
441 - if (!$endPath)
442 - return false;
443 - }
444 - }
445 - $endPath = trim ( $endPath );
446 - if ($extension) {
447 - $wmSvnPath = 'svn.wikimedia.org/svnroot/mediawiki';
448 - $isWMSvn = strstr($content[5],$wmSvnPath);
449 - if (!strcmp($isWMSvn,null)) {
450 - return false;
451 - } else {
452 - $viewvcStart = 'http://svn.wikimedia.org/viewvc/mediawiki/';
453 - if (strstr( $content[4], 'trunk' ))
454 - $viewvcEnd = '/?pathrev=';
455 - else
456 - // Avoids 404 error using pathrev when it does not found
457 - $viewvcEnd = '/?revision=';
458 - $viewvc = $viewvcStart . $endPath . $viewvcEnd;
459 - return $viewvc;
460 - }
461 - }
462 - return $endPath;
 465+ }
 466+
 467+ // subversion is release 1.4 or above
 468+ if ( count( $lines ) < 11 ) {
 469+ return false;
 470+ }
 471+ $info = array(
 472+ 'checkout-rev' => intval( trim( $lines[3] ) ),
 473+ 'url' => trim( $lines[4] ),
 474+ 'repo-url' => trim( $lines[5] ),
 475+ 'directory-rev' => intval( trim( $lines[10] ) )
 476+ );
 477+ if ( isset( self::$viewvcUrls[$info['repo-url']] ) ) {
 478+ $viewvc = str_replace(
 479+ $info['repo-url'],
 480+ self::$viewvcUrls[$info['repo-url']],
 481+ $info['url']
 482+ );
 483+ $pathRelativeToRepo = substr( $info['url'], strlen( $info['repo-url'] ) );
 484+ if ( substr( $pathRelativeToRepo, 0, 6 ) == '/trunk' ) {
 485+ $viewvc .= '/?pathrev=';
 486+ } else {
 487+ // Avoids 404 error using pathrev when it does not found
 488+ $viewvc .= '/?revision=';
463489 }
464 - if ($coRev)
465 - // get the directory checkout revsion number
466 - return intval( $content[3]) ;
467 - else
468 - // get the directory last modified revision number
469 - return intval( $content[10] );
 490+ $viewvc .= urlencode( $info['checkout-rev'] );
 491+ $info['viewvc-url'] = $viewvc;
470492 }
 493+ return $info;
471494 }
472495
 496+ /**
 497+ * Retrieve the revision number of a Subversion working directory.
 498+ *
 499+ * @param String $dir Directory of the svn checkout
 500+ * @return int revision number as int
 501+ */
 502+ public static function getSvnRevision( $dir ) {
 503+ $info = self::getSvnInfo( $dir );
 504+ if ( $info === false ) {
 505+ return false;
 506+ } elseif ( isset( $info['checkout-rev'] ) ) {
 507+ return $info['checkout-rev'];
 508+ } else {
 509+ return false;
 510+ }
 511+ }
 512+
473513 /**#@-*/
474514 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r49995* (bug 18593) Unify svn-revision references in Special:Version, in addition w...shinjiman05:24, 28 April 2009
r50121Change to show the checkout revision number by default in Special:Version, pe...shinjiman07:44, 2 May 2009

Status & tagging log