From d28ddc21a5509627f7cffe079fb969d518b35bc6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Jan 2018 15:14:07 -0800 Subject: [PATCH] Don't error when trying to mirror or observe an empty repository Summary: Fixes T5965. Fixes two issues: - Observing an empty repository could write a warning to the log. - Mirroring an empty repository to a remote could fail. For observing: If newly-created with `git init --bare`, `git ls-remote` will return the empty string. Properly return an empty set of refs, rather than attempting to parse the single "line" that is produced by splitting that on newlines: ``` [2018-01-23 18:47:00] ERROR 8: Undefined offset: 1 at [/phab_path/phabricator/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:405] arcanist(head=master, ref.master=5634f8410176), phabricator(head=master, ref.master=12551a1055ce), phutil(head=master, ref.master=4755785517cf) #0 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343] #1 PhabricatorRepositoryPullEngine::executeGitUpdate() called at [/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:126] #2 PhabricatorRepositoryPullEngine::pullRepositoryWithLock() called at [/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:40] #3 PhabricatorRepositoryPullEngine::pullRepository() called at [/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php:59] ... ``` For mirroring: `git` treats `git push --mirror` specially when a repository is empty. Detect this case by seeing if `git for-each-ref --count 1` does anything. If the repository is empty, just bail. Test Plan: - Observed an empty and non-empty repository. - Mirrored an empty and non-empty repository. Reviewers: alexmv, amckinley Reviewed By: alexmv Subscribers: Korvin, epriestley Maniphest Tasks: T5965 Differential Revision: https://secure.phabricator.com/D18920 --- .../engine/PhabricatorRepositoryMirrorEngine.php | 14 ++++++++++++++ .../engine/PhabricatorRepositoryPullEngine.php | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php b/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php index b18f6b1bf8..ed856957f0 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryMirrorEngine.php @@ -76,6 +76,20 @@ final class PhabricatorRepositoryMirrorEngine PhabricatorRepository $repository, PhabricatorRepositoryURI $mirror_uri) { + // See T5965. Test if we have any refs to mirror. If we have nothing, git + // will exit with an error ("No refs in common and none specified; ...") + // when we run "git push --mirror". + + // If we don't have any refs, we just bail out. (This is arguably sort of + // the wrong behavior: to mirror an empty repository faithfully we should + // delete everything in the remote.) + + list($stdout) = $repository->execxLocalCommand( + 'for-each-ref --count 1 --'); + if (!strlen($stdout)) { + return; + } + $argv = array( 'push --verbose --mirror -- %P', $mirror_uri->getURIEnvelope(), diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php index 10eddd38d4..208a3792b3 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -399,6 +399,11 @@ final class PhabricatorRepositoryPullEngine 'ls-remote %P', $remote_envelope); + // Empty repositories don't have any refs. + if (!strlen(rtrim($stdout))) { + return array(); + } + $map = array(); $lines = phutil_split_lines($stdout, false); foreach ($lines as $line) {