Skip to content

Commit 1a11297

Browse files
author
epriestley
committed
Allow parsing of rare extra-broken non-UTF8 messages.
1 parent c22124d commit 1a11297

File tree

6 files changed

+51
-20
lines changed

6 files changed

+51
-20
lines changed

scripts/daemon/phabricator_daemon_launcher.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,20 @@
192192
throw new Exception('Unknown commit.');
193193
}
194194

195+
$workers = array();
196+
197+
195198
switch ($repo->getVersionControlSystem()) {
196199
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
197-
$worker = new PhabricatorRepositoryGitCommitChangeParserWorker(
200+
$workers[] = new PhabricatorRepositoryGitCommitMessageParserWorker(
201+
$commit->getID());
202+
$workers[] = new PhabricatorRepositoryGitCommitChangeParserWorker(
198203
$commit->getID());
199204
break;
200205
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
201-
$worker = new PhabricatorRepositorySvnCommitChangeParserWorker(
206+
$workers[] = new PhabricatorRepositorySvnCommitMessageParserWorker(
207+
$commit->getID());
208+
$workers[] = new PhabricatorRepositorySvnCommitChangeParserWorker(
202209
$commit->getID());
203210
break;
204211
default:
@@ -207,7 +214,10 @@
207214

208215
ExecFuture::pushEchoMode(true);
209216

210-
$worker->doWork();
217+
foreach ($workers as $worker) {
218+
echo "Running ".get_class($worker)."...\n";
219+
$worker->doWork();
220+
}
211221

212222
echo "Done.\n";
213223

src/applications/repository/worker/base/PhabricatorRepositoryCommitParserWorker.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,38 @@ abstract protected function parseCommit(
5050
PhabricatorRepository $repository,
5151
PhabricatorRepositoryCommit $commit);
5252

53+
/**
54+
* This method is kind of awkward here but both the SVN message and
55+
* change parsers use it.
56+
*/
57+
protected function getSVNLogXMLObject($uri, $revision) {
58+
59+
try {
60+
list($xml) = execx(
61+
'svn log --xml --limit 1 --non-interactive %s@%d',
62+
$uri,
63+
$revision);
64+
} catch (CommandException $ex) {
65+
// HTTPS is generally faster and more reliable than svn+ssh, but some
66+
// commit messages with non-UTF8 text can't be retrieved over HTTPS, see
67+
// Facebook rE197184 for one example. Make an attempt to fall back to
68+
// svn+ssh if we've failed outright to retrieve the message.
69+
$fallback_uri = new PhutilURI($uri);
70+
if ($fallback_uri->getProtocol() != 'https') {
71+
throw $ex;
72+
}
73+
$fallback_uri->setProtocol('svn+ssh');
74+
list($xml) = execx(
75+
'svn log --xml --limit 1 --non-interactive %s@%d',
76+
$fallback_uri,
77+
$revision);
78+
}
79+
80+
// Subversion may send us back commit messages which won't parse because
81+
// they have non UTF-8 garbage in them. Slam them into valid UTF-8.
82+
$xml = phutil_utf8ize($xml);
83+
84+
return new SimpleXMLElement($xml);
85+
}
86+
5387
}

src/applications/repository/worker/base/__init__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
phutil_require_module('phabricator', 'applications/repository/storage/repository');
1111
phutil_require_module('phabricator', 'infrastructure/daemon/workers/worker');
1212

13+
phutil_require_module('phutil', 'future/exec');
14+
phutil_require_module('phutil', 'parser/uri');
1315
phutil_require_module('phutil', 'utils');
1416

1517

src/applications/repository/worker/commitchangeparser/svn/PhabricatorRepositorySvnCommitChangeParserWorker.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,8 @@ protected function parseCommit(
4848

4949
// Pull the top-level path changes out of "svn log". This is pretty
5050
// straightforward; just parse the XML log.
51-
list($xml) = execx(
52-
'svn log --verbose --xml --limit 1 --non-interactive %s@%d',
53-
$uri,
54-
$svn_commit);
51+
$log = $this->getSVNLogXMLObject($uri, $svn_commit);
5552

56-
$log = new SimpleXMLElement($xml);
5753
$entry = $log->logentry[0];
5854

5955
if (!$entry->paths) {

src/applications/repository/worker/commitmessageparser/svn/PhabricatorRepositorySvnCommitMessageParserWorker.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,8 @@ public function parseCommit(
2525

2626
$uri = $repository->getDetail('remote-uri');
2727

28-
list($xml) = execx(
29-
'svn log --xml --limit 1 --non-interactive %s@%d',
30-
$uri,
31-
$commit->getCommitIdentifier());
28+
$log = $this->getSVNLogXMLObject($uri, $commit->getCommitIdentifier());
3229

33-
// Subversion may send us back commit messages which won't parse because
34-
// they have non UTF-8 garbage in them. Slam them into valid UTF-8.
35-
$xml = phutil_utf8ize($xml);
36-
37-
$log = new SimpleXMLElement($xml);
3830
$entry = $log->logentry[0];
3931

4032
$author = (string)$entry->author;

src/applications/repository/worker/commitmessageparser/svn/__init__.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,5 @@
99
phutil_require_module('phabricator', 'applications/repository/worker/commitmessageparser/base');
1010
phutil_require_module('phabricator', 'infrastructure/daemon/workers/storage/task');
1111

12-
phutil_require_module('phutil', 'future/exec');
13-
phutil_require_module('phutil', 'utils');
14-
1512

1613
phutil_require_source('PhabricatorRepositorySvnCommitMessageParserWorker.php');

0 commit comments

Comments
 (0)