diff --git a/resources/sql/patches/006.repository.sql b/resources/sql/patches/006.repository.sql index 6ea9ee3f5f..29ce4aa76e 100644 --- a/resources/sql/patches/006.repository.sql +++ b/resources/sql/patches/006.repository.sql @@ -41,4 +41,14 @@ create table phabricator_repository.repository_filesystem ( primary key (repositoryID, parentID, svnCommit, pathID) ); -alter table repository_filesystem add key (repositoryID, svnCommit); \ No newline at end of file +alter table repository_filesystem add key (repositoryID, svnCommit); + +truncate phabricator_repository.repository_commit; +alter table phabricator_repository.repository_commit + change repositoryPHID repositoryID int unsigned not null; +alter table phabricator_repository.repository_commit drop key repositoryPHID; +alter table phabricator_repository.repository_commit add unique key + (repositoryID, commitIdentifier(16)); +alter table phabricator_repository.repository_commit add key + (repositoryID, epoch); + diff --git a/src/applications/diffusion/controller/home/DiffusionHomeController.php b/src/applications/diffusion/controller/home/DiffusionHomeController.php index 9989cb3b66..c51c034e89 100644 --- a/src/applications/diffusion/controller/home/DiffusionHomeController.php +++ b/src/applications/diffusion/controller/home/DiffusionHomeController.php @@ -27,24 +27,49 @@ class DiffusionHomeController extends DiffusionController { $commit = new PhabricatorRepositoryCommit(); $conn_r = $commit->establishConnection('r'); - // TODO: Both these queries are basically bogus and have total trash for - // query plans, and don't return the right results. Build a cache instead. - // These are just pulling data with approximately the right look to it. - $commits = $commit->loadAllWhere( - '1 = 1 GROUP BY repositoryPHID'); - $commits = mpull($commits, null, 'getRepositoryPHID'); + // TODO: These queries are pretty bogus. - $commit_counts = queryfx_all( - $conn_r, - 'SELECT repositoryPHID, count(*) N FROM %T - GROUP BY repositoryPHID', + $commits = array(); + $commit_counts = array(); + + $max_epoch = queryfx_all( + $commit->establishConnection('r'), + 'SELECT repositoryID, MAX(epoch) maxEpoch FROM %T GROUP BY repositoryID', $commit->getTableName()); - $commit_counts = ipull($commit_counts, 'N', 'repositoryPHID'); + + if ($max_epoch) { + $sql = array(); + foreach ($max_epoch as $head) { + $sql[] = '('.(int)$head['repositoryID'].', '.(int)$head['maxEpoch'].')'; + } + + // NOTE: It's possible we'll pull multiple commits for some repository + // here but it reduces query cost around 3x to unique them in PHP rather + // than apply GROUP BY in MySQL. + $commits = $commit->loadAllWhere( + '(repositoryID, epoch) IN (%Q)', + implode(', ', $sql)); + $commits = mpull($commits, null, 'getRepositoryID'); + + $commit_counts = queryfx_all( + $conn_r, + 'SELECT repositoryID, count(*) N FROM %T + GROUP BY repositoryID', + $commit->getTableName()); + $commit_counts = ipull($commit_counts, 'N', 'repositoryID'); + } $rows = array(); foreach ($repositories as $repository) { - $phid = $repository->getPHID(); - $commit = idx($commits, $phid); + $id = $repository->getID(); + $commit = idx($commits, $id); + $date = null; + $time = null; + if ($commit) { + $date = date('M j, Y', $commit->getEpoch()); + $time = date('g:i A', $commit->getEpoch()); + } + $rows[] = array( phutil_render_tag( 'a', @@ -53,13 +78,14 @@ class DiffusionHomeController extends DiffusionController { ), phutil_escape_html($repository->getName())), $repository->getVersionControlSystem(), - idx($commit_counts, $phid, 0), + idx($commit_counts, $id, 0), $commit - ? $commit->getCommitIdentifier() - : null, // TODO: Link/format - $commit - ? phabricator_format_timestamp($commit->getEpoch()) + ? DiffusionView::linkCommit( + $repository, + $commit->getCommitIdentifier()) : null, + $date, + $time, ); } @@ -70,11 +96,17 @@ class DiffusionHomeController extends DiffusionController { 'VCS', 'Size', 'Last', - 'Committed', + 'Date', + 'Time', )); $table->setColumnClasses( array( 'wide', + '', + 'n', + 'n', + '', + 'right', )); $panel = new AphrontPanelView(); diff --git a/src/applications/diffusion/controller/repository/DiffusionRepositoryController.php b/src/applications/diffusion/controller/repository/DiffusionRepositoryController.php index 7b14ccdd24..b62b3fd11d 100644 --- a/src/applications/diffusion/controller/repository/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/repository/DiffusionRepositoryController.php @@ -35,8 +35,16 @@ class DiffusionRepositoryController extends DiffusionController { $history_table->setDiffusionRequest($drequest); $history_table->setHistory($history); + $callsign = $drequest->getRepository()->getCallsign(); + $all = phutil_render_tag( + 'a', + array( + 'href' => "/diffusion/{$callsign}/history/", + ), + 'View Full Commit History'); + $panel = new AphrontPanelView(); - $panel->setHeader('Recent Commits'); + $panel->setHeader("Recent Commits · {$all}"); $panel->appendChild($history_table); $content[] = $panel; diff --git a/src/applications/diffusion/data/pathchange/DiffusionPathChange.php b/src/applications/diffusion/data/pathchange/DiffusionPathChange.php index 652ab82d08..ebd38224ef 100644 --- a/src/applications/diffusion/data/pathchange/DiffusionPathChange.php +++ b/src/applications/diffusion/data/pathchange/DiffusionPathChange.php @@ -19,6 +19,8 @@ final class DiffusionPathChange { private $commitIdentifier; + private $commit; + private $commitData; final public function setCommitIdentifier($commit) { $this->commitIdentifier = $commit; @@ -29,5 +31,49 @@ final class DiffusionPathChange { return $this->commitIdentifier; } + final public function setCommit($commit) { + $this->commit = $commit; + return $this; + } + + final public function getCommit() { + return $this->commit; + } + + final public function setCommitData($commit_data) { + $this->commitData = $commit_data; + return $this; + } + + final public function getCommitData() { + return $this->commitData; + } + + + final public function getEpoch() { + if ($this->getCommit()) { + return $this->getCommit()->getEpoch(); + } + return null; + } + + final public function getAuthorName() { + if ($this->getCommitData()) { + return $this->getCommitData()->getAuthorName(); + } + return null; + } + + final public function getSummary() { + if (!$this->getCommitData()) { + return null; + } + $message = $this->getCommitData()->getCommitMessage(); + $first = idx(explode("\n", $message), 0); + return substr($first, 0, 80); + } + + + } diff --git a/src/applications/diffusion/data/pathchange/__init__.php b/src/applications/diffusion/data/pathchange/__init__.php index 1e0dcbd347..0256f567bf 100644 --- a/src/applications/diffusion/data/pathchange/__init__.php +++ b/src/applications/diffusion/data/pathchange/__init__.php @@ -6,5 +6,7 @@ +phutil_require_module('phutil', 'utils'); + phutil_require_source('DiffusionPathChange.php'); diff --git a/src/applications/diffusion/query/browse/svn/DiffusionSvnBrowseQuery.php b/src/applications/diffusion/query/browse/svn/DiffusionSvnBrowseQuery.php index 37da292afa..cd3dfe5ffc 100644 --- a/src/applications/diffusion/query/browse/svn/DiffusionSvnBrowseQuery.php +++ b/src/applications/diffusion/query/browse/svn/DiffusionSvnBrowseQuery.php @@ -37,6 +37,12 @@ final class DiffusionSvnBrowseQuery extends DiffusionBrowseQuery { $paths = ipull($paths, 'id', 'path'); $path_id = $paths[$path_normal]; + if ($commit) { + $slice_clause = 'AND svnCommit <= '.(int)$commit; + } else { + $slice_clause = ''; + } + $index = queryfx_all( $conn_r, 'SELECT pathID, max(svnCommit) maxCommit FROM %T WHERE @@ -45,7 +51,7 @@ final class DiffusionSvnBrowseQuery extends DiffusionBrowseQuery { PhabricatorRepository::TABLE_FILESYSTEM, $repository->getID(), $path_id, - ''); + $slice_clause); if (!$index) { // TODO: ! diff --git a/src/applications/diffusion/query/history/base/DiffusionHistoryQuery.php b/src/applications/diffusion/query/history/base/DiffusionHistoryQuery.php index 15e33280ff..8cb1c1d719 100644 --- a/src/applications/diffusion/query/history/base/DiffusionHistoryQuery.php +++ b/src/applications/diffusion/query/history/base/DiffusionHistoryQuery.php @@ -20,6 +20,7 @@ abstract class DiffusionHistoryQuery { private $request; private $limit = 100; + private $offset = 0; final private function __construct() { // @@ -66,5 +67,14 @@ abstract class DiffusionHistoryQuery { return $this->limit; } + final public function setOffset($offset) { + $this->offset = $offset; + return $this; + } + + final public function getOffset() { + return $this->offset; + } + abstract protected function executeQuery(); } diff --git a/src/applications/diffusion/query/history/git/DiffusionGitHistoryQuery.php b/src/applications/diffusion/query/history/git/DiffusionGitHistoryQuery.php index 9226d98069..a30ce88f25 100644 --- a/src/applications/diffusion/query/history/git/DiffusionGitHistoryQuery.php +++ b/src/applications/diffusion/query/history/git/DiffusionGitHistoryQuery.php @@ -23,7 +23,7 @@ final class DiffusionGitHistoryQuery extends DiffusionHistoryQuery { $repository = $drequest->getRepository(); $path = $drequest->getPath(); - $commit = $drequest->getCommit(); + $commit_hash = $drequest->getCommit(); $local_path = $repository->getDetail('local-path'); $git = $drequest->getPathToGitBinary(); @@ -32,31 +32,49 @@ final class DiffusionGitHistoryQuery extends DiffusionHistoryQuery { '(cd %s && %s log '. '--skip=%d '. '-n %d '. - '-M '. - '-C '. - '-B '. - '--find-copies-harder '. - '--raw '. '-t '. '--abbrev=40 '. - '--pretty=format:%%x1c%%H%%x1d '. + '--pretty=format:%%H '. '%s -- %s)', $local_path, $git, - $offset = 0, + $this->getOffset(), $this->getLimit(), - $commit, + $commit_hash, $path); - $commits = explode("\x1c", $stdout); - array_shift($commits); // \x1c character is first, remove empty record + $hashes = explode("\n", $stdout); + $hashes = array_filter($hashes); + + $commits = array(); + $commit_data = array(); + + if ($hashes) { + $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( + 'repositoryID = %d AND commitIdentifier IN (%Ls)', + $repository->getID(), + $hashes); + $commits = mpull($commits, null, 'getCommitIdentifier'); + if ($commits) { + $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere( + 'commitID in (%Ld)', + mpull($commits, 'getID')); + $commit_data = mpull($commit_data, null, 'getCommitID'); + } + } $history = array(); - foreach ($commits as $commit) { - list($hash, $raw) = explode("\x1d", $commit); - + foreach ($hashes as $hash) { $item = new DiffusionPathChange(); $item->setCommitIdentifier($hash); + $commit = idx($commits, $hash); + if ($commit) { + $item->setCommit($commit); + $data = idx($commit_data, $commit->getID()); + if ($data) { + $item->setCommitData($data); + } + } $history[] = $item; } diff --git a/src/applications/diffusion/query/history/git/__init__.php b/src/applications/diffusion/query/history/git/__init__.php index c2a505f6df..a1f26cd3ff 100644 --- a/src/applications/diffusion/query/history/git/__init__.php +++ b/src/applications/diffusion/query/history/git/__init__.php @@ -8,8 +8,11 @@ phutil_require_module('phabricator', 'applications/diffusion/data/pathchange'); phutil_require_module('phabricator', 'applications/diffusion/query/history/base'); +phutil_require_module('phabricator', 'applications/repository/storage/commit'); +phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); phutil_require_module('phutil', 'future/exec'); +phutil_require_module('phutil', 'utils'); phutil_require_source('DiffusionGitHistoryQuery.php'); diff --git a/src/applications/diffusion/query/history/svn/DiffusionSvnHistoryQuery.php b/src/applications/diffusion/query/history/svn/DiffusionSvnHistoryQuery.php index a95b588c8a..92106793c1 100644 --- a/src/applications/diffusion/query/history/svn/DiffusionSvnHistoryQuery.php +++ b/src/applications/diffusion/query/history/svn/DiffusionSvnHistoryQuery.php @@ -47,10 +47,36 @@ final class DiffusionSvnHistoryQuery extends DiffusionHistoryQuery { $commit ? $commit : 0x7FFFFFFF, $this->getLimit()); + $commits = array(); + $commit_data = array(); + + $commit_ids = ipull($history_data, 'commitID'); + if ($commit_ids) { + $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( + 'id IN (%Ld)', + $commit_ids); + if ($commits) { + $commit_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere( + 'commitID in (%Ld)', + $commit_ids); + $commit_data = mpull($commit_data, null, 'getCommitID'); + } + } + + + $history = array(); foreach ($history_data as $row) { $item = new DiffusionPathChange(); - $item->setCommitIdentifier($row['commitID']); + $commit = idx($commits, $row['commitID']); + if ($commit) { + $item->setCommit($commit); + $item->setCommitIdentifier($commit->getCommitIdentifier()); + $data = idx($commit_data, $commit->getID()); + if ($data) { + $item->setCommitData($data); + } + } $history[] = $item; } diff --git a/src/applications/diffusion/view/historytable/DiffusionHistoryTableView.php b/src/applications/diffusion/view/historytable/DiffusionHistoryTableView.php index c1960a7403..bf31331419 100644 --- a/src/applications/diffusion/view/historytable/DiffusionHistoryTableView.php +++ b/src/applications/diffusion/view/historytable/DiffusionHistoryTableView.php @@ -30,6 +30,16 @@ final class DiffusionHistoryTableView extends DiffusionView { $rows = array(); foreach ($this->history as $history) { + $epoch = $history->getEpoch(); + + if ($epoch) { + $date = date('M j, Y', $epoch); + $time = date('g:i A', $epoch); + } else { + $date = null; + $time = null; + } + $rows[] = array( $this->linkBrowse( $drequest->getPath(), @@ -39,10 +49,11 @@ final class DiffusionHistoryTableView extends DiffusionView { self::linkCommit( $drequest->getRepository(), $history->getCommitIdentifier()), - '?', - '?', - '', - '', + '-', + $date, + $time, + phutil_escape_html($history->getAuthorName()), + phutil_escape_html($history->getSummary()), // TODO: etc etc ); } @@ -54,6 +65,7 @@ final class DiffusionHistoryTableView extends DiffusionView { 'Commit', 'Change', 'Date', + 'Time', 'Author', 'Details', )); @@ -63,6 +75,7 @@ final class DiffusionHistoryTableView extends DiffusionView { 'n', '', '', + 'right', '', 'wide wrap', )); diff --git a/src/applications/diffusion/view/historytable/__init__.php b/src/applications/diffusion/view/historytable/__init__.php index f6e0c07164..5607901c5f 100644 --- a/src/applications/diffusion/view/historytable/__init__.php +++ b/src/applications/diffusion/view/historytable/__init__.php @@ -8,6 +8,9 @@ phutil_require_module('phabricator', 'applications/diffusion/view/base'); phutil_require_module('phabricator', 'view/control/table'); +phutil_require_module('phabricator', 'view/utils'); + +phutil_require_module('phutil', 'markup'); phutil_require_source('DiffusionHistoryTableView.php'); diff --git a/src/applications/repository/daemon/commitdiscovery/base/PhabricatorRepositoryCommitDiscoveryDaemon.php b/src/applications/repository/daemon/commitdiscovery/base/PhabricatorRepositoryCommitDiscoveryDaemon.php index b3f42e094a..3b92f78a68 100644 --- a/src/applications/repository/daemon/commitdiscovery/base/PhabricatorRepositoryCommitDiscoveryDaemon.php +++ b/src/applications/repository/daemon/commitdiscovery/base/PhabricatorRepositoryCommitDiscoveryDaemon.php @@ -47,8 +47,8 @@ abstract class PhabricatorRepositoryCommitDiscoveryDaemon } $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryPHID = %s AND commitIdentifier = %s', - $this->getRepository()->getPHID(), + 'repositoryID = %s AND commitIdentifier = %s', + $this->getRepository()->getID(), $target); if (!$commit) { @@ -67,7 +67,7 @@ abstract class PhabricatorRepositoryCommitDiscoveryDaemon $repository = $this->getRepository(); $commit = new PhabricatorRepositoryCommit(); - $commit->setRepositoryPHID($repository->getPHID()); + $commit->setRepositoryID($repository->getID()); $commit->setCommitIdentifier($commit_identifier); $commit->setEpoch($epoch); diff --git a/src/applications/repository/daemon/committask/PhabricatorRepositoryCommitTaskDaemon.php b/src/applications/repository/daemon/committask/PhabricatorRepositoryCommitTaskDaemon.php index 65874fed0f..9e0aab76cc 100644 --- a/src/applications/repository/daemon/committask/PhabricatorRepositoryCommitTaskDaemon.php +++ b/src/applications/repository/daemon/committask/PhabricatorRepositoryCommitTaskDaemon.php @@ -38,9 +38,8 @@ class PhabricatorRepositoryCommitTaskDaemon } // TODO: Cache these. - $repository = id(new PhabricatorRepository())->loadOneWhere( - 'phid = %s', - $commit->getRepositoryPHID()); + $repository = id(new PhabricatorRepository())->load( + $commit->getRepositoryID()); $vcs = $repository->getVersionControlSystem(); switch ($vcs) { diff --git a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php index b0efefaf63..b3dce63f0d 100644 --- a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php @@ -18,7 +18,7 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { - protected $repositoryPHID; + protected $repositoryID; protected $phid; protected $commitIdentifier; protected $epoch; diff --git a/src/applications/repository/worker/base/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/base/PhabricatorRepositoryCommitParserWorker.php index 04043e5e77..822c612246 100644 --- a/src/applications/repository/worker/base/PhabricatorRepositoryCommitParserWorker.php +++ b/src/applications/repository/worker/base/PhabricatorRepositoryCommitParserWorker.php @@ -36,9 +36,8 @@ abstract class PhabricatorRepositoryCommitParserWorker $this->commit = $commit; - $repository = id(new PhabricatorRepository())->loadOneWhere( - 'phid = %s', - $commit->getRepositoryPHID()); + $repository = id(new PhabricatorRepository())->load( + $commit->getRepositoryID()); if (!$repository) { return; diff --git a/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php index c61250990b..e9e871c94f 100644 --- a/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/base/PhabricatorRepositoryCommitChangeParserWorker.php @@ -19,6 +19,12 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker extends PhabricatorRepositoryCommitParserWorker { + public function getRequiredLeaseTime() { + // It can take a very long time to parse commits; some commits in the + // Facebook repository affect many millions of paths. Acquire 24h leases. + return 60 * 60 * 24; + } + protected function lookupOrCreatePaths(array $paths) { $repository = new PhabricatorRepository(); $conn_w = $repository->establishConnection('w'); @@ -37,7 +43,7 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker } queryfx( $conn_w, - 'INSERT INTO %T (path) VALUES %Q', + 'INSERT IGNORE INTO %T (path) VALUES %Q', PhabricatorRepository::TABLE_PATH, implode(', ', $sql)); }