[PATCH v2 0/2] git-p4: allow submission from detached head

[PATCH v2 0/2] git-p4: allow submission from detached head

[Luke Diamand]

This is a reroll of my earlier patch to teach git-p4
about detached heads.

It uses Junio's suggestion of calling "git symbolic-ref"
to determine if we're on a detached head, rather than
parsing text strings.

Luke Diamand (3):
  git-p4: add failing test for submit from detached head
  git-p4: add option to system() to return subshell status
  git-p4: work with a detached head

 git-p4.py               | 29 ++++++++++++++++++++---------
 t/t9800-git-p4-basic.sh | 16 ++++++++++++++++
 2 files changed, 36 insertions(+), 9 deletions(-)

-- 
2.6.0.rc3.238.gc07a1e8

[PATCH v2 1/3] git-p4: add failing test for submit from detached head

[Luke Diamand]

git-p4 can't submit from a detached head. This test case
demonstrates the problem.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9800-git-p4-basic.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 90d41ed..114b19f 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,6 +241,22 @@ test_expect_success 'unresolvable host in P4PORT should display error' '
 	)
 '
 
+test_expect_failure 'submit from detached head' '
+	test_when_finished cleanup_git &&
+	git p4 clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		git checkout p4/master &&
+		>detached_head_test &&
+		git add detached_head_test &&
+		git commit -m "add detached_head" &&
+		git config git-p4.skipSubmitEdit true &&
+		git p4 submit &&
+		git p4 rebase &&
+		git log p4/master | grep detached_head
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
2.6.0.rc3.238.gc07a1e8

[PATCH v2 2/3] git-p4: add option to system() to return subshell status

[Luke Diamand]

Add an optional parameter ignore_error to the git-p4 system()
function. If used, it will return the subshell exit status
rather than throwing an exception.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 0093fa3..9d55f9c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -192,14 +192,16 @@ def p4_has_move_command():
     # assume it failed because @... was invalid changelist
     return True
 
-def system(cmd):
+def system(cmd, ignore_error=False):
     expand = isinstance(cmd,basestring)
     if verbose:
         sys.stderr.write("executing %s\n" % str(cmd))
     retcode = subprocess.call(cmd, shell=expand)
-    if retcode:
+    if retcode and not ignore_error:
         raise CalledProcessError(retcode, cmd)
 
+    return retcode
+
 def p4_system(cmd):
     """Specifically invoke p4 as the system command. """
     real_cmd = p4_build_cmd(cmd)
-- 
2.6.0.rc3.238.gc07a1e8

[PATCH v2 3/3] git-p4: work with a detached head

[Luke Diamand]

When submitting, git-p4 finds the current branch in
order to know if it is allowed to submit (configuration
"git-p4.allowSubmit").

On a detached head, detecting the branch would fail, and
git-p4 would report a cryptic error.

This change teaches git-p4 to recognise a detached head and
submit successfully.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py               | 23 ++++++++++++++++-------
 t/t9800-git-p4-basic.sh |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 9d55f9c..0cfc866 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -544,7 +544,12 @@ def p4Where(depotPath):
     return clientPath
 
 def currentGitBranch():
-    return read_pipe("git name-rev HEAD").split(" ")[1].strip()
+    retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
+    if retcode != 0:
+        # on a detached head
+        return None
+    else:
+        return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
 
 def isValidGitDir(path):
     if (os.path.exists(path + "/HEAD")
@@ -1653,8 +1658,6 @@ class P4Submit(Command, P4UserMap):
     def run(self, args):
         if len(args) == 0:
             self.master = currentGitBranch()
-            if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % self.master):
-                die("Detecting current git branch failed!")
         elif len(args) == 1:
             self.master = args[0]
             if not branchExists(self.master):
@@ -1662,9 +1665,10 @@ class P4Submit(Command, P4UserMap):
         else:
             return False
 
-        allowSubmit = gitConfig("git-p4.allowSubmit")
-        if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
-            die("%s is not in git-p4.allowSubmit" % self.master)
+        if self.master:
+            allowSubmit = gitConfig("git-p4.allowSubmit")
+            if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
+                die("%s is not in git-p4.allowSubmit" % self.master)
 
         [upstream, settings] = findUpstreamBranchPoint()
         self.depotPath = settings['depot-paths'][0]
@@ -1732,7 +1736,12 @@ class P4Submit(Command, P4UserMap):
         self.check()
 
         commits = []
-        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, self.master)]):
+        if self.master:
+            commitish = self.master
+        else:
+            commitish = 'HEAD'
+
+        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
             commits.append(line.strip())
         commits.reverse()
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 114b19f..0730f18 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,7 +241,7 @@ test_expect_success 'unresolvable host in P4PORT should display error' '
 	)
 '
 
-test_expect_failure 'submit from detached head' '
+test_expect_success 'submit from detached head' '
 	test_when_finished cleanup_git &&
 	git p4 clone --dest="$git" //depot &&
 	(
-- 
2.6.0.rc3.238.gc07a1e8

[PATCH v2 3/3] git-p4: work with a detached head

[Luke Diamand]

When submitting, git-p4 finds the current branch in
order to know if it is allowed to submit (configuration
"git-p4.allowSubmit").

On a detached head, detecting the branch would fail, and
git-p4 would report a cryptic error.

This change teaches git-p4 to recognise a detached head and
submit successfully.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py               | 23 ++++++++++++++++-------
 t/t9800-git-p4-basic.sh |  2 +-
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 9d55f9c..0cfc866 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -544,7 +544,12 @@ def p4Where(depotPath):
     return clientPath
 
 def currentGitBranch():
-    return read_pipe("git name-rev HEAD").split(" ")[1].strip()
+    retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
+    if retcode != 0:
+        # on a detached head
+        return None
+    else:
+        return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
 
 def isValidGitDir(path):
     if (os.path.exists(path + "/HEAD")
@@ -1653,8 +1658,6 @@ class P4Submit(Command, P4UserMap):
     def run(self, args):
         if len(args) == 0:
             self.master = currentGitBranch()
-            if len(self.master) == 0 or not gitBranchExists("refs/heads/%s" % self.master):
-                die("Detecting current git branch failed!")
         elif len(args) == 1:
             self.master = args[0]
             if not branchExists(self.master):
@@ -1662,9 +1665,10 @@ class P4Submit(Command, P4UserMap):
         else:
             return False
 
-        allowSubmit = gitConfig("git-p4.allowSubmit")
-        if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
-            die("%s is not in git-p4.allowSubmit" % self.master)
+        if self.master:
+            allowSubmit = gitConfig("git-p4.allowSubmit")
+            if len(allowSubmit) > 0 and not self.master in allowSubmit.split(","):
+                die("%s is not in git-p4.allowSubmit" % self.master)
 
         [upstream, settings] = findUpstreamBranchPoint()
         self.depotPath = settings['depot-paths'][0]
@@ -1732,7 +1736,12 @@ class P4Submit(Command, P4UserMap):
         self.check()
 
         commits = []
-        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, self.master)]):
+        if self.master:
+            commitish = self.master
+        else:
+            commitish = 'HEAD'
+
+        for line in read_pipe_lines(["git", "rev-list", "--no-merges", "%s..%s" % (self.origin, commitish)]):
             commits.append(line.strip())
         commits.reverse()
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 114b19f..0730f18 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -241,7 +241,7 @@ test_expect_success 'unresolvable host in P4PORT should display error' '
 	)
 '
 
-test_expect_failure 'submit from detached head' '
+test_expect_success 'submit from detached head' '
 	test_when_finished cleanup_git &&
 	git p4 clone --dest="$git" //depot &&
 	(
-- 
2.6.0.rc3.238.gc07a1e8