Merge branch 'pw/p4-branch-fixes'

Fix "git p4" around branch handling.

* pw/p4-branch-fixes:
  git p4: fix submit when no master branch
  git p4 test: keep P4CLIENT changes inside subshells
  git p4: fix sync --branch when no master branch
  git p4: fail gracefully on sync with no master branch
  git p4: rearrange self.initialParent use
  git p4: allow short ref names to --branch
  git p4 doc: fix branch detection example
  git p4: clone --branch should checkout master
  git p4: verify expected refs in clone --bare test
  git p4: create p4/HEAD on initial clone
  git p4: inline listExistingP4GitBranches
  git p4: add comments to p4BranchesInGit
  git p4: rearrange and simplify hasOrigin handling
  git p4: test sync/clone --branch behavior

1 files changed, 107 insertions, 45 deletions

diff --git a/git-p4.py b/git-p4.py
index 69f1452cf8..2da564995d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -553,29 +553,49 @@ def gitConfigList(key):
         _gitConfig[key] = read_pipe("git config --get-all %s" % key, ignore_error=True).strip().split(os.linesep)
     return _gitConfig[key]
 
-def p4BranchesInGit(branchesAreInRemotes = True):
+def p4BranchesInGit(branchesAreInRemotes=True):
+    """Find all the branches whose names start with "p4/", looking
+       in remotes or heads as specified by the argument.  Return
+       a dictionary of { branch: revision } for each one found.
+       The branch names are the short names, without any
+       "p4/" prefix."""
+
     branches = {}
 
     cmdline = "git rev-parse --symbolic "
     if branchesAreInRemotes:
-        cmdline += " --remotes"
+        cmdline += "--remotes"
     else:
-        cmdline += " --branches"
+        cmdline += "--branches"
 
     for line in read_pipe_lines(cmdline):
         line = line.strip()
 
-        ## only import to p4/
-        if not line.startswith('p4/') or line == "p4/HEAD":
+        # only import to p4/
+        if not line.startswith('p4/'):
+            continue
+        # special symbolic ref to p4/master
+        if line == "p4/HEAD":
             continue
-        branch = line
 
-        # strip off p4
-        branch = re.sub ("^p4/", "", line)
+        # strip off p4/ prefix
+        branch = line[len("p4/"):]
 
         branches[branch] = parseRevision(line)
+
     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
@@ -907,7 +927,8 @@ class P4Submit(Command, P4UserMap):
                 optparse.make_option("--dry-run", "-n", dest="dry_run", action="store_true"),
                 optparse.make_option("--prepare-p4-only", dest="prepare_p4_only", action="store_true"),
                 optparse.make_option("--conflict", dest="conflict_behavior",
-                                     choices=self.conflict_behavior_choices)
+                                     choices=self.conflict_behavior_choices),
+                optparse.make_option("--branch", dest="branch"),
         ]
         self.description = "Submit changes from git to the perforce depot."
         self.usage += " [name of git branch to submit into perforce depot]"
@@ -920,6 +941,7 @@ class P4Submit(Command, P4UserMap):
         self.isWindows = (platform.system() == "Windows")
         self.exportLabels = False
         self.p4HasMoveCommand = p4_has_move_command()
+        self.branch = None
 
     def check(self):
         if len(p4CmdList("opened ...")) > 0:
@@ -1656,6 +1678,8 @@ class P4Submit(Command, P4UserMap):
             print "All commits applied!"
 
             sync = P4Sync()
+            if self.branch:
+                sync.branch = self.branch
             sync.run([])
 
             rebase = P4Rebase()
@@ -2509,13 +2533,6 @@ class P4Sync(Command, P4UserMap):
                 branch = branch[len(self.projectName):]
             self.knownBranches[branch] = branch
 
-    def listExistingP4GitBranches(self):
-        # branches holds mapping from name to commit
-        branches = p4BranchesInGit(self.importIntoRemotes)
-        self.p4BranchesInGit = branches.keys()
-        for branch in branches.keys():
-            self.initialParents[self.refPrefix + branch] = branches[branch]
-
     def updateOptionDict(self, d):
         option_keys = {}
         if self.keepRepoPath:
@@ -2687,6 +2704,7 @@ class P4Sync(Command, P4UserMap):
                     files = self.extractFilesFromCommit(description)
                     self.commit(description, files, self.branch,
                                 self.initialParent)
+                    # only needed once, to connect to the previous commit
                     self.initialParent = ""
             except IOError:
                 print self.gitError.read()
@@ -2752,34 +2770,31 @@ class P4Sync(Command, P4UserMap):
     def run(self, args):
         self.depotPaths = []
         self.changeRange = ""
-        self.initialParent = ""
         self.previousDepotPaths = []
+        self.hasOrigin = False
 
         # map from branch depot path to parent branch
         self.knownBranches = {}
         self.initialParents = {}
-        self.hasOrigin = originP4BranchesExist()
-        if not self.syncWithOrigin:
-            self.hasOrigin = False
 
         if self.importIntoRemotes:
             self.refPrefix = "refs/remotes/p4/"
         else:
             self.refPrefix = "refs/heads/p4/"
 
-        if self.syncWithOrigin and self.hasOrigin:
-            if not self.silent:
-                print "Syncing with origin first by calling git fetch origin"
-            system("git fetch origin")
+        if self.syncWithOrigin:
+            self.hasOrigin = originP4BranchesExist()
+            if self.hasOrigin:
+                if not self.silent:
+                    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:
                 system("git update-ref %s refs/heads/p4" % self.branch)
-                system("git branch -D p4");
-            # create it /after/ importing, when master exists
-            if not gitBranchExists(self.refPrefix + "HEAD") and self.importIntoRemotes and gitBranchExists(self.branch):
-                system("git symbolic-ref %sHEAD %s" % (self.refPrefix, self.branch))
+                system("git branch -D p4")
 
         # accept either the command-line option, or the configuration variable
         if self.useClientSpec:
@@ -2796,12 +2811,25 @@ class P4Sync(Command, P4UserMap):
         if args == []:
             if self.hasOrigin:
                 createOrUpdateBranchesFromOrigin(self.refPrefix, self.silent)
-            self.listExistingP4GitBranches()
+
+            # branches holds mapping from branch name to sha1
+            branches = p4BranchesInGit(self.importIntoRemotes)
+
+            # restrict to just this one, disabling detect-branches
+            if branch_arg_given:
+                short = self.branch.split("/")[-1]
+                if short in branches:
+                    self.p4BranchesInGit = [ short ]
+            else:
+                self.p4BranchesInGit = branches.keys()
 
             if len(self.p4BranchesInGit) > 1:
                 if not self.silent:
                     print "Importing from/into multiple branches"
                 self.detectBranches = True
+                for branch in branches.keys():
+                    self.initialParents[self.refPrefix + branch] = \
+                        branches[branch]
 
             if self.verbose:
                 print "branches: %s" % self.p4BranchesInGit
@@ -2838,13 +2866,21 @@ class P4Sync(Command, P4UserMap):
             if p4Change > 0:
                 self.depotPaths = sorted(self.previousDepotPaths)
                 self.changeRange = "@%s,#head" % p4Change
-                if not self.detectBranches:
-                    self.initialParent = parseRevision(self.branch)
                 if not self.silent and not self.detectBranches:
                     print "Performing incremental import into %s git branch" % self.branch
 
+        # accept multiple ref name abbreviations:
+        #    refs/foo/bar/branch -> use it exactly
+        #    p4/branch -> prepend refs/remotes/ or refs/heads/
+        #    branch -> prepend refs/remotes/p4/ or refs/heads/p4/
         if not self.branch.startswith("refs/"):
-            self.branch = "refs/heads/" + self.branch
+            if self.importIntoRemotes:
+                prepend = "refs/remotes/"
+            else:
+                prepend = "refs/heads/"
+            if not self.branch.startswith("p4/"):
+                prepend += "p4/"
+            self.branch = prepend + self.branch
 
         if len(args) == 0 and self.depotPaths:
             if not self.silent:
@@ -2955,8 +2991,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)
@@ -2974,6 +3023,14 @@ class P4Sync(Command, P4UserMap):
 
                 self.updatedBranches = set()
 
+                if not self.detectBranches:
+                    if args:
+                        # start a new branch
+                        self.initialParent = ""
+                    else:
+                        # build on a previous revision
+                        self.initialParent = parseRevision(self.branch)
+
                 self.importChanges(changes)
 
                 if not self.silent:
@@ -3006,6 +3063,13 @@ class P4Sync(Command, P4UserMap):
                 read_pipe("git update-ref -d %s" % branch)
             os.rmdir(os.path.join(os.environ.get("GIT_DIR", ".git"), self.tempBranchLocation))
 
+        # Create a symbolic ref p4/HEAD pointing to p4/<branch> to allow
+        # a convenient shortcut refname "p4".
+        if self.importIntoRemotes:
+            head_ref = self.refPrefix + "HEAD"
+            if not gitBranchExists(head_ref) and gitBranchExists(self.branch):
+                system(["git", "symbolic-ref", head_ref, self.branch])
+
         return True
 
 class P4Rebase(Command):
@@ -3113,17 +3177,15 @@ class P4Clone(P4Sync):
 
         if not P4Sync.run(self, depotPaths):
             return False
-        if self.branch != "master":
-            if self.importIntoRemotes:
-                masterbranch = "refs/remotes/p4/master"
-            else:
-                masterbranch = "refs/heads/p4/master"
-            if gitBranchExists(masterbranch):
-                system("git branch master %s" % masterbranch)
-                if not self.cloneBare:
-                    system("git checkout -f")
-            else:
-                print "Could not detect main branch. No checkout/master branch created."
+
+        # create a master branch and check out a work tree
+        if gitBranchExists(self.branch):
+            system([ "git", "branch", "master", self.branch ])
+            if not self.cloneBare:
+                system([ "git", "checkout", "-f" ])
+        else:
+            print 'Not checking out any branch, use ' \
+                  '"git checkout -q -b master <branch>"'
 
         # auto-set this variable if invoked with --use-client-spec
         if self.useClientSpec_from_options: