[PATCHv2] git-p4: Handle p4 submit failure

Luke Diamand· Nov 24, 2015, 07:43 UTC · lore

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

Luke Diamand· Nov 24, 2015, 07:43 UTC · re: Luke Diamand · lore

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

From: GIRARD Etienne <egirard@murex.com>

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

Jeff King· Nov 24, 2015, 21:18 UTC · re: Luke Diamand · lore

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

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