Page MenuHomePhabricator
Files F79160

D7428.diff
All Users
Actions

D7428.diff
View Options

diff --git a/resources/sql/patches/20131026.commitstatus.sql b/resources/sql/patches/20131026.commitstatus.sql
new file mode 100644
--- /dev/null
+++ b/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 = 15;
+
+ALTER TABLE {$NAMESPACE}_repository.repository_commit
+ ADD KEY (repositoryID, importStatus);
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/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);
+ }
+
}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -15,6 +15,13 @@
protected $authorPHID;
protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE;
protected $summary = '';
+ protected $importStatus = 0;
+
+ const IMPORTED_MESSAGE = 1;
+ const IMPORTED_CHANGE = 2;
+ const IMPORTED_OWNERS = 4;
+ const IMPORTED_HERALD = 8;
+ const IMPORTED_ALL = 15;
private $commitData = self::ATTACHABLE;
private $audits;
@@ -39,6 +46,20 @@
return $this->isUnparsed;
}
+ public function isImported() {
+ return ($this->getImportStatus() == self::IMPORTED_ALL);
+ }
+
+ public function writeImportStatusFlag($flag) {
+ queryfx(
+ $this->establishConnection('w'),
+ 'UPDATE %T SET importStatus = (importStatus | %d) WHERE id = %d',
+ $this->getTableName(),
+ $flag,
+ $this->getID());
+ return $this;
+ }
+
public function getConfiguration() {
return array(
self::CONFIG_AUX_PHID => true,
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -12,6 +12,18 @@
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
+ $result = $this->applyHeraldRules($repository, $commit);
+
+ $commit->writeImportStatusFlag(
+ PhabricatorRepositoryCommit::IMPORTED_HERALD);
+
+ return $result;
+ }
+
+ private function applyHeraldRules(
+ PhabricatorRepository $repository,
+ PhabricatorRepositoryCommit $commit) {
+
$data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
'commitID = %d',
$commit->getID());
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -58,6 +58,9 @@
$commit->save();
}
+ $commit->writeImportStatusFlag(
+ PhabricatorRepositoryCommit::IMPORTED_OWNERS);
+
if ($this->shouldQueueFollowupTasks()) {
PhabricatorWorker::scheduleTask(
'PhabricatorRepositoryCommitHeraldWorker',
diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
--- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
@@ -58,6 +58,9 @@
protected function finishParse() {
$commit = $this->commit;
+ $commit->writeImportStatusFlag(
+ PhabricatorRepositoryCommit::IMPORTED_CHANGE);
+
id(new PhabricatorSearchIndexer())
->indexDocumentByPHID($commit->getPHID());
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -220,6 +220,9 @@
}
$data->save();
+
+ $commit->writeImportStatusFlag(
+ PhabricatorRepositoryCommit::IMPORTED_MESSAGE);
}
private function loadUserName($user_phid, $default, PhabricatorUser $actor) {
@@ -249,7 +252,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)
diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
--- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
+++ b/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/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/2p/su/wr5gcckr4n6okdfy
Default Alt Text
D7428.diff (9 KB)

Event Timeline

Phabricator · Built by Phacility