From a15130b47c6db4d29d252ecc29a7cf09c6fb9d0f Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 2 Feb 2012 16:03:50 -0800 Subject: [PATCH] Add a maintenance script for reconciling repositories to disk state Summary: @rguerin ran into an issue in his install where Phabricator appears to have discovered commits which no longer exist, and thus is failing to proceed with its repository import. It's not clear how we got into this state. Previously, it was possible by, e.g., parsing a different repository's working copy and then switching them back, but there are now safeguards against that. I'm taking a three-pronged approach to try to sort this out: - Provide a script to get out of this state (this script) and reconcile Phabricator's view of a repository with an authoritative copy of it. This basically "un-discovers" any discovered commits which don't actually exist (any queued tasks to parse them will fail permanently when they fail to load the commit object). - Add more logging to the discovery daemon so we can figure out where commits came from. - Improve Diffusion's UI when stuff is partially discovered (T776). (This script should also clean up some nonsense on secure.phabricator.com from a botched Diviner import.) Test Plan: Ran "reconcile.php" with bogus commits and bogus differential/commit links, had them expunged. Will work with @rguerin to see if this resolves things. Reviewers: btrahan, rguerin Reviewed By: btrahan CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1552 --- scripts/repository/reconcile.php | 165 ++++++++++++++++++ .../commit/PhabricatorRepositoryCommit.php | 24 ++- .../repository/storage/commit/__init__.php | 3 + 3 files changed, 191 insertions(+), 1 deletion(-) create mode 100755 scripts/repository/reconcile.php diff --git a/scripts/repository/reconcile.php b/scripts/repository/reconcile.php new file mode 100755 index 0000000000..6e45f034a2 --- /dev/null +++ b/scripts/repository/reconcile.php @@ -0,0 +1,165 @@ +#!/usr/bin/env php +setTagline('reconcile Phabricator state after repository changes'); +$args->setSynopsis(<<parseStandardArguments(); +$args->parse( + array( + array( + 'name' => 'more', + 'wildcard' => true, + ), + )); + +$more = $args->getArg('more'); +if (count($more) !== 1) { + $args->printHelpAndExit(); +} +$callsign = reset($more); + + +$repository = id(new PhabricatorRepository())->loadOneWhere( + 'callsign = %s', + $callsign); +if (!$repository) { + throw new Exception("No repository exists with callsign '{$callsign}'!"); +} + +switch ($repository->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + default: + throw new Exception("For now, you can only reconcile git repositories."); +} + +echo "Loading commits...\n"; +$all_commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( + 'repositoryID = %d', + $repository->getID()); + +echo "Updating repository..\n"; +try { + // Sanity-check the repository working copy and make sure we're up to date. + $repository->execxLocalCommand('fetch --all'); +} catch (Exception $ex) { + echo "Unable to `git fetch` the working copy to update it. Reconciliation ". + "requires an up-to-date working copy.\n"; + throw $ex; +} + +echo "Verifying commits (this may take some time if the repository is large)"; +$futures = array(); +foreach ($all_commits as $id => $commit) { + $futures[$id] = $repository->getLocalCommandFuture( + 'rev-parse --verify %s', + $commit->getCommitIdentifier()); +} + +$bad = array(); +foreach (Futures($futures)->limit(8) as $id => $future) { + list($err) = $future->resolve(); + if ($err) { + $bad[$id] = $all_commits[$id]; + echo "#"; + } else { + echo "."; + } +} +echo "\nDone.\n"; + +if (!count($bad)) { + echo "No bad commits found!\n"; +} else { + echo "Found ".count($bad)." bad commits:\n\n"; + echo ' '.implode("\n ", mpull($bad, 'getCommitIdentifier')); + $ok = phutil_console_confirm("Do you want to delete these commits?"); + if (!$ok) { + echo "OK, aborting.\n"; + exit(1); + } + + echo "Deleting commits"; + foreach ($bad as $commit) { + echo "."; + $commit->delete(); + } + echo "\nDone.\n"; +} + +//// Clean Up Links //////////////////////////////////////////////////////// + +$table = new PhabricatorRepositoryCommit(); + +$valid_phids = queryfx_all( + $table->establishConnection('r'), + 'SELECT phid FROM %T', + $table->getTableName()); +$valid_phids = ipull($valid_phids, null, 'phid'); + +//////// Differential <-> Diffusion Links ////////////////////////////////// + +$dx_conn = id(new DifferentialRevision())->establishConnection('w'); +$dx_table = DifferentialRevision::TABLE_COMMIT; +$dx_phids = queryfx_all( + $dx_conn, + 'SELECT commitPHID FROM %T', + $dx_table); + +$bad_phids = array(); +foreach ($dx_phids as $dx_phid) { + if (empty($valid_phids[$dx_phid['commitPHID']])) { + $bad_phids[] = $dx_phid['commitPHID']; + } +} + +if ($bad_phids) { + echo "Deleting ".count($bad_phids)." bad Diffusion links...\n"; + queryfx( + $dx_conn, + 'DELETE FROM %T WHERE commitPHID IN (%Ls)', + $dx_table, + $bad_phids); + echo "Done.\n"; +} else { + echo "Diffusion links are clean.\n"; +} + +// TODO: There are some links in owners that we should probably clean up too. diff --git a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php index 67daa340e5..9984f64a89 100644 --- a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php @@ -1,7 +1,7 @@ getID()) { + return null; + } + return id(new PhabricatorRepositoryCommitData())->loadOneWhere( + 'commitID = %d', + $this->getID()); + } + public function attachCommitData(PhabricatorRepositoryCommitData $data) { $this->commitData = $data; return $this; @@ -49,4 +58,17 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { return $this->commitData; } + public function delete() { + $data = $this->loadCommitData(); + $this->openTransaction(); + + if ($data) { + $data->delete(); + } + $result = parent::delete(); + + $this->saveTransaction(); + return $result; + } + } diff --git a/src/applications/repository/storage/commit/__init__.php b/src/applications/repository/storage/commit/__init__.php index 9d410d797e..64ca0a27b8 100644 --- a/src/applications/repository/storage/commit/__init__.php +++ b/src/applications/repository/storage/commit/__init__.php @@ -9,6 +9,9 @@ phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); phutil_require_module('phabricator', 'applications/repository/storage/base'); +phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); + +phutil_require_module('phutil', 'utils'); phutil_require_source('PhabricatorRepositoryCommit.php');