[PATCHv2] git-p4: Handle p4 submit failure

[PATCHv2] git-p4: Handle p4 submit failure

[Luke Diamand]

This is a small reroll of Etienne's earlier patch to clean up
the p4 repo on submit failure.

The only difference is to fix the t9807- test. This test
was trying to remove a file which had already been cleaned
up as a result of this change.

GIRARD Etienne (1):
  git-p4: clean up after p4 submit failure

 git-p4.py                | 71 +++++++++++++++++++++++++-----------------------
 t/t9807-git-p4-submit.sh |  2 +-
 2 files changed, 38 insertions(+), 35 deletions(-)

-- 
2.6.3.492.g06488d6

[PATCHv2] git-p4: clean up after p4 submit failure

[Luke Diamand]

From: GIRARD Etienne <egirard@murex.com>

When "p4 submit" command fails in P4Submit.applyCommit, the
workspace is left with the changes.  We already have code to revert
the changes to the workspace when the user decides to cancel
submission by aborting the editor that edits the change description,
and we should treat the "p4 submit" failure the same way.

Clean the workspace if p4_write_pipe raised SystemExit, so that the
user don't have to do it themselves.

Signed-off-by: GIRARD Etienne <egirard@murex.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py                | 71 +++++++++++++++++++++++++-----------------------
 t/t9807-git-p4-submit.sh |  2 +-
 2 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/git-p4.py b/git-p4.py
index 0093fa3..d535904 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1538,44 +1538,47 @@ class P4Submit(Command, P4UserMap):
         #
         # Let the user edit the change description, then submit it.
         #
-        if self.edit_template(fileName):
-            # read the edited message and submit
-            ret = True
-            tmpFile = open(fileName, "rb")
-            message = tmpFile.read()
-            tmpFile.close()
-            if self.isWindows:
-                message = message.replace("\r\n", "\n")
-            submitTemplate = message[:message.index(separatorLine)]
-            p4_write_pipe(['submit', '-i'], submitTemplate)
-
-            if self.preserveUser:
-                if p4User:
-                    # Get last changelist number. Cannot easily get it from
-                    # the submit command output as the output is
-                    # unmarshalled.
-                    changelist = self.lastP4Changelist()
-                    self.modifyChangelistUser(changelist, p4User)
-
-            # The rename/copy happened by applying a patch that created a
-            # new file.  This leaves it writable, which confuses p4.
-            for f in pureRenameCopy:
-                p4_sync(f, "-f")
+        submitted = False
 
-        else:
+        try:
+            if self.edit_template(fileName):
+                # read the edited message and submit
+                tmpFile = open(fileName, "rb")
+                message = tmpFile.read()
+                tmpFile.close()
+                if self.isWindows:
+                    message = message.replace("\r\n", "\n")
+                submitTemplate = message[:message.index(separatorLine)]
+                p4_write_pipe(['submit', '-i'], submitTemplate)
+
+                if self.preserveUser:
+                    if p4User:
+                        # Get last changelist number. Cannot easily get it from
+                        # the submit command output as the output is
+                        # unmarshalled.
+                        changelist = self.lastP4Changelist()
+                        self.modifyChangelistUser(changelist, p4User)
+
+                # The rename/copy happened by applying a patch that created a
+                # new file.  This leaves it writable, which confuses p4.
+                for f in pureRenameCopy:
+                    p4_sync(f, "-f")
+                submitted = True
+
+        finally:
             # skip this patch
-            ret = False
-            print "Submission cancelled, undoing p4 changes."
-            for f in editedFiles:
-                p4_revert(f)
-            for f in filesToAdd:
-                p4_revert(f)
-                os.remove(f)
-            for f in filesToDelete:
-                p4_revert(f)
+            if not submitted:
+                print "Submission cancelled, undoing p4 changes."
+                for f in editedFiles:
+                    p4_revert(f)
+                for f in filesToAdd:
+                    p4_revert(f)
+                    os.remove(f)
+                for f in filesToDelete:
+                    p4_revert(f)
 
         os.remove(fileName)
-        return ret
+        return submitted
 
     # Export git tags as p4 labels. Create a p4 label and then tag
     # with that.
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 1f74a88..5931528 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -389,7 +389,7 @@ test_expect_success 'description with Jobs section and bogus following text' '
 	(
 		cd "$cli" &&
 		p4 revert desc6 &&
-		rm desc6
+		rm -f desc6
 	)
 '
 
-- 
2.6.3.492.g06488d6

Re: [PATCHv2] git-p4: clean up after p4 submit failure

[Jeff King]

On Tue, Nov 24, 2015 at 07:43:59AM +0000, Luke Diamand wrote:

> From: GIRARD Etienne <egirard@murex.com>
> 
> When "p4 submit" command fails in P4Submit.applyCommit, the
> workspace is left with the changes.  We already have code to revert
> the changes to the workspace when the user decides to cancel
> submission by aborting the editor that edits the change description,
> and we should treat the "p4 submit" failure the same way.
> 
> Clean the workspace if p4_write_pipe raised SystemExit, so that the
> user don't have to do it themselves.
> 
> Signed-off-by: GIRARD Etienne <egirard@murex.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---

Thanks, I think this should be ready for 'next', then.

-Peff