git p4: fail gracefully on sync with no master branch

If --branch was used to build a repository with no
refs/remotes/p4/master, future syncs will not know
which branch to sync.  Notice this situation and
print a helpful error message.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 31 insertions, 7 deletions

diff --git a/git-p4.py b/git-p4.py
index 2924496cd4..7b71ceb288 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -579,6 +579,17 @@ def p4BranchesInGit(branchesAreInRemotes=True):
 
     return branches
 
+def branch_exists(branch):
+    """Make sure that the given ref name really exists."""
+
+    cmd = [ "git", "rev-parse", "--symbolic", "--verify", branch ]
+    p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    out, _ = p.communicate()
+    if p.returncode:
+        return False
+    # expect exactly one line of output: the branch name
+    return out.rstrip() == branch
+
 def findUpstreamBranchPoint(head = "HEAD"):
     branches = p4BranchesInGit()
     # map from depot-path to branch name
@@ -2768,6 +2779,7 @@ class P4Sync(Command, P4UserMap):
                     print 'Syncing with origin first, using "git fetch origin"'
                 system("git fetch origin")
 
+        branch_arg_given = bool(self.branch)
         if len(self.branch) == 0:
             self.branch = self.refPrefix + "master"
             if gitBranchExists("refs/heads/p4") and self.importIntoRemotes:
@@ -2961,8 +2973,21 @@ class P4Sync(Command, P4UserMap):
             else:
                 # catch "git p4 sync" with no new branches, in a repo that
                 # does not have any existing p4 branches
-                if len(args) == 0 and not self.p4BranchesInGit:
-                    die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.");
+                if len(args) == 0:
+                    if not self.p4BranchesInGit:
+                        die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.")
+
+                    # The default branch is master, unless --branch is used to
+                    # specify something else.  Make sure it exists, or complain
+                    # nicely about how to use --branch.
+                    if not self.detectBranches:
+                        if not branch_exists(self.branch):
+                            if branch_arg_given:
+                                die("Error: branch %s does not exist." % self.branch)
+                            else:
+                                die("Error: no branch %s; perhaps specify one with --branch." %
+                                    self.branch)
+
                 if self.verbose:
                     print "Getting p4 changes for %s...%s" % (', '.join(self.depotPaths),
                                                               self.changeRange)
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index c0d44337d2..a51f1221ed 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -40,14 +40,13 @@ test_expect_success 'clone --branch should checkout master' '
 	)
 '
 
-test_expect_failure 'sync when branch is not called master should work' '
-	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot@2 &&
+test_expect_success 'sync when no master branch prints a nice error' '
 	test_when_finished cleanup_git &&
+	git p4 clone --branch=refs/remotes/p4/sb --dest="$git" //depot@2 &&
 	(
 		cd "$git" &&
-		git p4 sync &&
-		git show -s --format=%s refs/remotes/p4/sb >show &&
-		grep "change 3" show
+		test_must_fail git p4 sync 2>err &&
+		grep "Error: no branch refs/remotes/p4/master" err
 	)
 '