r62914 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r62913‎ | r62914 | r62915 >
Date:13:18, 24 February 2010
Author:demon
Status:ok (Comments)
Tags:
Comment:
Rewrite svnImport to subclass Maintenance.php, rename "*" to "all" for all repos, also address concerns from Tim and Bryan on r62510
Modified paths:
  • /trunk/extensions/CodeReview/svnImport.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CodeReview/svnImport.php
——@@ -1,134 +1,141 @@
22 <?php
33
4 -$optionsWithArgs = array( 'precache' );
5 -
64 $IP = getenv( 'MW_INSTALL_PATH' );
7 -if ( $IP === false )
 5+if ( $IP === false ) {
86 $IP = dirname( __FILE__ ) . '/../..';
9 -require "$IP/maintenance/commandLine.inc";
10 -
11 -if ( !isset( $args[0] ) ) {
12 - echo "Usage: php svnImport.php <repo> [<start>] [--precache=<N>]\n";
13 - echo " <repo>\n";
14 - echo " The name of the repo. Use * to import from all defined repos.\n";
15 - echo " <start>\n";
16 - echo " The revision to begin the import from. If not specified then\n";
17 - echo " it starts from the last repo imported to the wiki. Ignored if\n";
18 - echo " * is specified for <repo>.\n";
19 - echo " --precache=<N>\n";
20 - echo " (default N=50) Pre-cache diffs for last N revisions. Use 0 to \n";
21 - echo " disable pre-caching, or -1 to pre-cache the entire repository.\n";
22 - echo " Already-cached revisions do not count as part of this number.\n";
23 - die;
247 }
 8+require( "$IP/maintenance/Maintenance.php" );
259
26 -$cacheSize = 50;
27 -if ( isset( $options['precache'] ) ) {
28 - if ( is_numeric( $options['precache'] ) && $options['precache'] >= -1 )
29 - $cacheSize = intval( $options['precache'] );
30 - else
31 - die( "Invalid argument for --precache (must be a positive integer, or -1 for all)" );
32 -}
 10+class SvnImport extends Maintenance {
3311
34 -if ( $args[0] == "*" ) {
35 - $repoList = CodeRepository::getRepoList();
36 - foreach ( $repoList as $repoInfo ) {
37 - importRepo( $repoInfo->getName() );
 12+ public function __construct() {
 13+ parent::__construct();
 14+ $this->mDescription = "Import revisions to Code Review from a Subversion repo";
 15+ $this->addOption( 'precache', 'Pre-cache diffs for last N revisions, -1 means entire repo', false, true );
 16+ $this->addArg( 'repo', 'The name of the repo. Use * to import from all defined repos' );
 17+ $this->addArg( 'start', "The revision to begin the import from. If not specified then\n" .
 18+ "\t\tit starts from the last repo imported to the wiki. Ignored if\n" .
 19+ "\t\t'all' is specified for <repo>", false );
3820 }
39 -} else {
40 - importRepo( $args[0], @$args[1] );
41 -}
4221
43 -function importRepo( $repoName, $start = null ) {
44 - global $wgCodeReviewImportBatchSize, $cacheSize;
 22+ public function execute() {
 23+ $cacheSize = null;
 24+ if ( $this->hasOption( 'precache' ) ) {
 25+ $cacheSize = $this->getOption( 'precache' );
 26+ if ( is_numeric( $cacheSize ) && $cacheSize >= -1 ) {
 27+ $cacheSize = intval( $cacheSize );
 28+ } else {
 29+ $this->error( "Invalid argument for --precache (must be a positive integer, or -1 for all)", true );
 30+ }
 31+ }
4532
46 - $repo = CodeRepository::newFromName( $repoName );
47 -
48 - if ( !$repo ) {
49 - echo "Invalid repo $repoName\n";
50 - die;
 33+ $repo = $this->getArg();
 34+ if ( $repo == "all" ) {
 35+ $repoList = CodeRepository::getRepoList();
 36+ foreach ( $repoList as $repoInfo ) {
 37+ $this->importRepo( $repoInfo->getName(), null, $cacheSize );
 38+ }
 39+ } else {
 40+ $this->importRepo( $repo, $this->getArg( 1 ), $cacheSize );
 41+ }
5142 }
5243
53 - $svn = SubversionAdaptor::newFromRepo( $repo->getPath() );
54 - $lastStoredRev = $repo->getLastStoredRev();
 44+ private function importRepo( $repoName, $start = null, $cacheSize = null ) {
 45+ global $wgCodeReviewImportBatchSize;
5546
56 - $chunkSize = $wgCodeReviewImportBatchSize;
 47+ $repo = CodeRepository::newFromName( $repoName );
5748
58 - $startTime = microtime( true );
59 - $revCount = 0;
60 - $start = isset( $start ) ? intval( $start ) : $lastStoredRev + 1;
61 - if ( $start > ( $lastStoredRev + 1 ) ) {
62 - echo "Invalid starting point r{$start}\n";
63 - die;
64 - }
 49+ if ( !$repo ) {
 50+ $this->error( "Invalid repo $repoName" );
 51+ return;
 52+ }
6553
66 - echo "Syncing repo $repoName from r$start to HEAD...\n";
 54+ $svn = SubversionAdaptor::newFromRepo( $repo->getPath() );
 55+ $lastStoredRev = $repo->getLastStoredRev();
6756
68 - if ( !$svn->canConnect() ) {
69 - die( "Unable to connect to repository.\n" );
70 - }
 57+ $chunkSize = $wgCodeReviewImportBatchSize;
7158
72 - while ( true ) {
73 - $log = $svn->getLog( '', $start, $start + $chunkSize - 1 );
74 - if ( empty( $log ) ) {
75 - # Repo seems to give a blank when max rev is invalid, which
76 - # stops new revisions from being added. Try to avoid this
77 - # by trying less at a time from the last point.
78 - if ( $chunkSize <= 1 ) {
79 - break; // done!
80 - }
81 - $chunkSize = max( 1, floor( $chunkSize / 4 ) );
82 - continue;
83 - } else {
84 - $start += $chunkSize;
 59+ $startTime = microtime( true );
 60+ $revCount = 0;
 61+ $start = $start === null ? intval( $start ) : $lastStoredRev + 1;
 62+ if ( $start > ( $lastStoredRev + 1 ) ) {
 63+ $this->error( "Invalid starting point r{$start}" );
 64+ return;
8565 }
86 - if ( !is_array( $log ) ) {
87 - var_dump( $log );
88 - die( 'wtf' );
 66+
 67+ $this->output( "Syncing repo $repoName from r$start to HEAD..." );
 68+
 69+ if ( !$svn->canConnect() ) {
 70+ $this->error( "Unable to connect to repository." );
 71+ return;
8972 }
90 - foreach ( $log as $data ) {
91 - $revCount++;
92 - $delta = microtime( true ) - $startTime;
93 - $revSpeed = $revCount / $delta;
9473
95 - $codeRev = CodeRevision::newFromSvn( $repo, $data );
96 - $codeRev->save();
 74+ while ( true ) {
 75+ $log = $svn->getLog( '', $start, $start + $chunkSize - 1 );
 76+ if ( empty( $log ) ) {
 77+ # Repo seems to give a blank when max rev is invalid, which
 78+ # stops new revisions from being added. Try to avoid this
 79+ # by trying less at a time from the last point.
 80+ if ( $chunkSize <= 1 ) {
 81+ break; // done!
 82+ }
 83+ $chunkSize = max( 1, floor( $chunkSize / 4 ) );
 84+ continue;
 85+ } else {
 86+ $start += $chunkSize;
 87+ }
 88+ if ( !is_array( $log ) ) {
 89+ var_dump( $log );
 90+ $this->error( 'wtf', true );
 91+ }
 92+ foreach ( $log as $data ) {
 93+ $revCount++;
 94+ $delta = microtime( true ) - $startTime;
 95+ $revSpeed = $revCount / $delta;
9796
98 - printf( "%d %s %s (%0.1f revs/sec)\n",
99 - $codeRev->mId,
100 - wfTimestamp( TS_DB, $codeRev->mTimestamp ),
101 - $codeRev->mAuthor,
102 - $revSpeed );
 97+ $codeRev = CodeRevision::newFromSvn( $repo, $data );
 98+ $codeRev->save();
 99+
 100+ $this->output( sprintf( "%d %s %s (%0.1f revs/sec)\n",
 101+ $codeRev->mId,
 102+ wfTimestamp( TS_DB, $codeRev->mTimestamp ),
 103+ $codeRev->mAuthor,
 104+ $revSpeed ) );
 105+ }
 106+ wfWaitForSlaves( 5 );
103107 }
104 - wfWaitForSlaves( 5 );
105 - }
106108
107 - if ( $cacheSize != 0 ) {
108 - if ( $cacheSize == -1 )
109 - echo "Pre-caching all uncached diffs...\n";
110 - elseif ( $cacheSize == 1 )
111 - echo "Pre-caching the latest diff...\n";
112 - else
113 - echo "Pre-caching the latest $cacheSize diffs...\n";
 109+ if ( $cacheSize !== null ) {
 110+ if ( $cacheSize == -1 ) {
 111+ $this->output( "Pre-caching all uncached diffs..." );
 112+ } elseif ( $cacheSize == 1 ) {
 113+ $this->output( "Pre-caching the latest diff..." );
 114+ } else {
 115+ $this->output( "Pre-caching the latest $cacheSize diffs..." );
 116+ }
114117
115 - $dbw = wfGetDB( DB_MASTER );
116 - $options = array( 'ORDER BY' => 'cr_id DESC' );
117 - if ( $cacheSize > 0 )
118 - $options['LIMIT'] = $cacheSize;
 118+ $dbw = wfGetDB( DB_MASTER );
 119+ $options = array( 'ORDER BY' => 'cr_id DESC' );
 120+ if ( $cacheSize > 0 )
 121+ $options['LIMIT'] = $cacheSize;
119122
120 - $res = $dbw->select( 'code_rev', 'cr_id',
121 - array( 'cr_repo_id' => $repo->getId(), 'cr_diff IS NULL OR cr_diff = ""' ),
122 - __METHOD__,
123 - $options
124 - );
125 - while ( $row = $dbw->fetchObject( $res ) ) {
126 - $rev = $repo->getRevision( $row->cr_id );
127 - $diff = $repo->getDiff( $row->cr_id ); // trigger caching
128 - echo "Diff r{$row->cr_id} done\n";
 123+ $res = $dbw->select( 'code_rev', 'cr_id',
 124+ array( 'cr_repo_id' => $repo->getId(), 'cr_diff IS NULL OR cr_diff = ""' ),
 125+ __METHOD__,
 126+ $options
 127+ );
 128+ while ( $row = $dbw->fetchObject( $res ) ) {
 129+ $rev = $repo->getRevision( $row->cr_id );
 130+ $diff = $repo->getDiff( $row->cr_id ); // trigger caching
 131+ $this->output( "Diff r{$row->cr_id} done" );
 132+ }
129133 }
 134+ else {
 135+ $this->output( "Pre-caching skipped." );
 136+ }
 137+ $this->output( "Done!" );
130138 }
131 - else
132 - echo "Pre-caching skipped.\n";
 139+}
133140
134 - echo "Done!\n";
135 -}
\ No newline at end of file
 141+$maintClass = "SvnImport";
 142+require_once( DO_MAINTENANCE );

Follow-up revisions

RevisionCommit summaryAuthorDate
r62917Cleanup r62914, Consistently refer to all instead of *demon14:21, 24 February 2010
r62996Fixed backwards logic mistake (r62914)aaron06:43, 26 February 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r62510CodeReview: Modified the first argument to svnImport.php so it is now possibl...happydog10:38, 15 February 2010

Comments

#Comment by HappyDog (talk | contribs)   14:02, 24 February 2010
  • is a better way to specify all repositories, because 'all' is a valid repository name.

Either way, the usage notes need to be made consistent (currently * is used on line 15 and 'all' on line 18).

#Comment by 😂 (talk | contribs)   14:19, 24 February 2010

Using * didn't work for me, as it matched the first file in the directory, I tried: 

$ php svnImport.php * --precache=20

And it read * as CodeReview.alias.php. I didn't like 

$ php svnImport.php '*' --precache=20

So I opted for 'all' instead. Open to other suggestions.

#Comment by HappyDog (talk | contribs)   15:00, 24 February 2010

I see your point... I forgot about wild-card expansion.

With that in mind I don't really have a better suggestion, so let's leave it as 'all'. It's not terribly likely that someone will have a repo with this name (and if they do, it is likely to be their only one so you would get the desired effect anyway).

Removed the 'fixme' tag as the inconsistency problem reported in my first comment has been fixed (r62917). Leaving it for someone else to review the rest of the changes.

#Comment by HappyDog (talk | contribs)   19:38, 24 February 2010

Also, I notice that precache now defaults to 0, rather than 50 as before. Was this deliberate?

#Comment by 😂 (talk | contribs)   19:54, 24 February 2010

Yes. I defaulted it to null. -1 still means do the entire repo, and a positive number will do that many revisions.

It would make most sense to me to not precache anything, unless specifically requested.

#Comment by HappyDog (talk | contribs)   13:43, 26 February 2010

Fair enough, as long as it was intentional.

Status & tagging log