1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Allow repo updates to recover after force push + garbage collection

Summary:
Fixes T5839. If a repository has been force pushed and garbage collected, we might have a ref cursor in the database which still points at the old commit (which no longer exists).

We'll then run a command like `git log <new hash> --not <old hash>` to figure out which commits are newly pushed, and this will bomb out because `<old hash>` is invalid.

Instead, validate all the `<old hash>` values before we try to make use of them.

Test Plan:
  - Forced a repository into a bad state by mucking with the datbase, generating a reproducible failure similar to the one in T5839.
  - Applied patch.
  - `bin/repository update <callsign> --trace` filtered the bad commit and put the repository into the right state.
  - Saw new commits recognized correctly.
  - Ran `bin/repository update <callsign>` for a Mercurial and SVN repo as a sanity check.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5839

Differential Revision: https://secure.phabricator.com/D10226
This commit is contained in:
epriestley 2014-08-12 12:25:24 -07:00
parent 394250397e
commit ec9eaabfbd
3 changed files with 36 additions and 6 deletions

View file

@ -135,12 +135,6 @@ final class DiffusionLowLevelResolveRefsQuery
$futures = array();
foreach ($this->refs as $ref) {
// TODO: There was a note about `--rev 'a b'` not working for branches
// with spaces in their names in older code, but I suspect this was
// misidentified and resulted from the branch name being interpeted as
// a revset. Use hgsprintf() to avoid that. If this doesn't break for a
// bit, remove this comment. Otherwise, consider `-b %s --limit 1`.
$futures[$ref] = $repository->getLocalCommandFuture(
'log --template=%s --rev %s',
'{node}',

View file

@ -63,6 +63,7 @@ final class PhabricatorRepositoryRefEngine
}
}
$all_closing_heads = array_unique($all_closing_heads);
$all_closing_heads = $this->removeMissingCommits($all_closing_heads);
foreach ($maps as $type => $refs) {
$cursor_group = idx($cursor_groups, $type, array());
@ -105,6 +106,35 @@ final class PhabricatorRepositoryRefEngine
return $this;
}
/**
* Remove commits which no longer exist in the repository from a list.
*
* After a force push and garbage collection, we may have branch cursors which
* point at commits which no longer exist. This can make commands issued later
* fail. See T5839 for discussion.
*
* @param list<string> List of commit identifiers.
* @return list<string> List with nonexistent identifiers removed.
*/
private function removeMissingCommits(array $identifiers) {
if (!$identifiers) {
return array();
}
$resolved = id(new DiffusionLowLevelResolveRefsQuery())
->setRepository($this->getRepository())
->withRefs($identifiers)
->execute();
foreach ($identifiers as $key => $identifier) {
if (empty($resolved[$identifier])) {
unset($identifiers[$key]);
}
}
return $identifiers;
}
private function updateCursors(
array $cursors,
array $new_refs,

View file

@ -42,6 +42,7 @@ final class PhabricatorRepositoryManagementUpdateWorkflow
public function execute(PhutilArgumentParser $args) {
$this->setVerbose($args->getArg('verbose'));
$console = PhutilConsole::getConsole();
$repos = $this->loadRepositories($args, 'repos');
if (count($repos) !== 1) {
@ -103,6 +104,11 @@ final class PhabricatorRepositoryManagementUpdateWorkflow
$lock->unlock();
$console->writeOut(
pht(
'Updated repository **%s**.',
$repository->getMonogram())."\n");
return 0;
}