Page MenuHomePhabricator

D7428.id16722.diff
No OneTemporary

D7428.id16722.diff

Index: resources/sql/patches/20131026.commitstatus.sql
===================================================================
--- /dev/null
+++ resources/sql/patches/20131026.commitstatus.sql
@@ -0,0 +1,8 @@
+ALTER TABLE {$NAMESPACE}_repository.repository_commit
+ ADD COLUMN importStatus INT UNSIGNED NOT NULL COLLATE utf8_bin;
+
+UPDATE {$NAMESPACE}_repository.repository_commit
+ SET importStatus = 1;
+
+ALTER TABLE {$NAMESPACE}_repository.repository_commit
+ ADD KEY (repositoryID, importStatus);
Index: src/applications/diffusion/controller/DiffusionCommitController.php
===================================================================
--- src/applications/diffusion/controller/DiffusionCommitController.php
+++ src/applications/diffusion/controller/DiffusionCommitController.php
@@ -76,7 +76,6 @@
$this->auditAuthorityPHIDs =
PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user);
-
$is_foreign = $commit_data->getCommitDetail('foreign-svn-stub');
$changesets = null;
if ($is_foreign) {
@@ -154,10 +153,14 @@
$hard_limit = 1000;
- $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
- $drequest);
- $change_query->setLimit($hard_limit + 1);
- $changes = $change_query->loadChanges();
+ if ($commit->isImported()) {
+ $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
+ $drequest);
+ $change_query->setLimit($hard_limit + 1);
+ $changes = $change_query->loadChanges();
+ } else {
+ $changes = array();
+ }
$was_limited = (count($changes) > $hard_limit);
if ($was_limited) {
@@ -206,37 +209,29 @@
}
if ($bad_commit) {
- $error_panel = new AphrontErrorView();
- $error_panel->setTitle(pht('Bad Commit'));
- $error_panel->appendChild($bad_commit['description']);
-
- $content[] = $error_panel;
+ $content[] = $this->renderStatusMessage(
+ pht('Bad Commit'),
+ $bad_commit['description']);
} else if ($is_foreign) {
// Don't render anything else.
+ } else if (!$commit->isImported()) {
+ $content[] = $this->renderStatusMessage(
+ pht('Still Importing...'),
+ pht(
+ 'This commit is still importing. Changes will be visible once '.
+ 'the import finishes.'));
} else if (!count($changes)) {
- $no_changes = new AphrontErrorView();
- $no_changes->setSeverity(AphrontErrorView::SEVERITY_WARNING);
- $no_changes->setTitle(pht('Not Yet Parsed'));
- // TODO: This can also happen with weird SVN changes that don't do
- // anything (or only alter properties?), although the real no-changes case
- // is extremely rare and might be impossible to produce organically. We
- // should probably write some kind of "Nothing Happened!" change into the
- // DB once we parse these changes so we can distinguish between
- // "not parsed yet" and "no changes".
- $no_changes->appendChild(
- pht("This commit hasn't been fully parsed yet (or doesn't affect any ".
- "paths)."));
- $content[] = $no_changes;
+ $content[] = $this->renderStatusMessage(
+ pht('Empty Commit'),
+ pht(
+ 'This commit is empty and does not affect any paths.'));
} else if ($was_limited) {
- $huge_commit = new AphrontErrorView();
- $huge_commit->setSeverity(AphrontErrorView::SEVERITY_WARNING);
- $huge_commit->setTitle(pht('Enormous Commit'));
- $huge_commit->appendChild(
+ $content[] = $this->renderStatusMessage(
+ pht('Enormous Commit'),
pht(
'This commit is enormous, and affects more than %d files. '.
'Changes are not shown.',
$hard_limit));
- $content[] = $huge_commit;
} else {
// The user has clicked "Show All Changes", and we should show all the
// changes inline even if there are more than the soft limit.
@@ -253,6 +248,7 @@
'href' => '?show_all=true',
),
pht('Show All Changes'));
+
$warning_view = id(new AphrontErrorView())
->setSeverity(AphrontErrorView::SEVERITY_WARNING)
->setTitle('Very Large Commit')
@@ -1084,4 +1080,11 @@
return $parser->processCorpus($corpus);
}
+ private function renderStatusMessage($title, $body) {
+ return id(new AphrontErrorView())
+ ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
+ ->setTitle($title)
+ ->appendChild($body);
+ }
+
}
Index: src/applications/repository/storage/PhabricatorRepositoryCommit.php
===================================================================
--- src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -15,6 +15,7 @@
protected $authorPHID;
protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE;
protected $summary = '';
+ protected $importStatus = 0;
private $commitData = self::ATTACHABLE;
private $audits;
@@ -39,6 +40,10 @@
return $this->isUnparsed;
}
+ public function isImported() {
+ return ($this->getImportStatus() > 0);
+ }
+
public function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
Index: src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
===================================================================
--- src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+++ src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
@@ -57,6 +57,14 @@
protected function finishParse() {
$commit = $this->commit;
+ $conn_w = $commit->establishConnection('w');
+
+ // Mark this commit as imported.
+ queryfx(
+ $conn_w,
+ 'UPDATE %T SET importStatus = 1 WHERE id = %d',
+ $commit->getTableName(),
+ $commit->getID());
id(new PhabricatorSearchIndexer())
->indexDocumentByPHID($commit->getPHID());
Index: src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
===================================================================
--- src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -249,7 +249,17 @@
->loadRawDiff();
// TODO: Support adds, deletes and moves under SVN.
- $changes = id(new ArcanistDiffParser())->parseDiff($raw_diff);
+ if (strlen($raw_diff)) {
+ $changes = id(new ArcanistDiffParser())->parseDiff($raw_diff);
+ } else {
+ // This is an empty diff, maybe made with `git commit --allow-empty`.
+ // NOTE: These diffs have the same tree hash as their ancestors, so
+ // they may attach to revisions in an unexpected way. Just let this
+ // happen for now, although it might make sense to special case it
+ // eventually.
+ $changes = array();
+ }
+
$diff = DifferentialDiff::newFromRawChanges($changes)
->setRevisionID($revision->getID())
->setAuthorPHID($actor_phid)
Index: src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
===================================================================
--- src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
+++ src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
@@ -1708,6 +1708,10 @@
'type' => 'sql',
'name' => $this->getPatchPath('20131025.repopush.sql'),
),
+ '20131026.commitstatus.sql' => array(
+ 'type' => 'sql',
+ 'name' => $this->getPatchPath('20131026.commitstatus.sql'),
+ ),
);
}
}

File Metadata

Mime Type
text/plain
Expires
Fri, Sep 20, 10:20 PM (18 h, 43 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6624307
Default Alt Text
D7428.id16722.diff (7 KB)

Event Timeline