git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCHv1 5/6] git-p4: cleanup better on error exit

From
Luke Diamand <luke@diamand.org>
Date
Jan 29, 2020, 11:12 UTC
Message-ID
<20200129111246.12196-6-luke@diamand.org>
In-Reply-To
<20200129111246.12196-5-luke@diamand.org>

After an error, git-p4 calls die(). This just exits, and leaves child processes still running.

Instead of calling die(), raise an exception and catch it where the child process(es) (git-fastimport) are created.

This was analyzed in detail here:
    https://public-inbox.org/git/20190227094926.GE19739@szeder.dev/

This change does not address the particular issue of p4CmdList() invoking a subchild and not waiting for it on error.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index f90b43fe5e..a69a24bf4c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -169,6 +169,9 @@ def calcDiskFree():
         return st.f_bavail * st.f_frsize
 
 def die(msg):
+    """ Terminate execution. Make sure that any running child processes have been wait()ed for before
+        calling this.
+    """
     if verbose:
         raise Exception(msg)
     else:
@@ -3574,7 +3577,7 @@ def importRevisions(self, args, branch_arg_given):
             # does not have any existing p4 branches
             if len(args) == 0:
                 if not self.p4BranchesInGit:
-                    die("No remote p4 branches.  Perhaps you never did \"git p4 clone\" in here.")
+                    raise P4CommandException("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
@@ -3582,9 +3585,9 @@ def importRevisions(self, args, branch_arg_given):
                 if not self.detectBranches:
                     if not branch_exists(self.branch):
                         if branch_arg_given:
-                            die("Error: branch %s does not exist." % self.branch)
+                            raise P4CommandException("Error: branch %s does not exist." % self.branch)
                         else:
-                            die("Error: no branch %s; perhaps specify one with --branch." %
+                            raise P4CommandException("Error: no branch %s; perhaps specify one with --branch." %
                                 self.branch)
 
             if self.verbose:
@@ -3825,22 +3828,32 @@ def run(self, args):
 
         self.openStreams()
 
-        if revision:
-            self.importHeadRevision(revision)
-        else:
-            self.importRevisions(args, branch_arg_given)
+        err = None
 
-        if gitConfigBool("git-p4.importLabels"):
-            self.importLabels = True
+        try:
+            if revision:
+                self.importHeadRevision(revision)
+            else:
+                self.importRevisions(args, branch_arg_given)
 
-        if self.importLabels:
-            p4Labels = getP4Labels(self.depotPaths)
-            gitTags = getGitTags()
+            if gitConfigBool("git-p4.importLabels"):
+                self.importLabels = True
 
-            missingP4Labels = p4Labels - gitTags
-            self.importP4Labels(self.gitStream, missingP4Labels)
+            if self.importLabels:
+                p4Labels = getP4Labels(self.depotPaths)
+                gitTags = getGitTags()
 
-        self.closeStreams()
+                missingP4Labels = p4Labels - gitTags
+                self.importP4Labels(self.gitStream, missingP4Labels)
+
+        except P4CommandException as e:
+            err = e
+
+        finally:
+            self.closeStreams()
+
+        if err:
+            die(str(err))
 
         # Cleanup temporary branches created during import
         if self.tempBranches != []:
-- 
2.20.1.390.gb5101f9297
Previous: Luke DiamandNext: Luke Diamand
Message 6 of 13 in “[PATCHv1 0/6] git-p4: wait() for child processes better”
  1. Luke DiamandJan 29, 2020
  2. 1/6 git-p4: make closeStreams() idempotentLuke Diamand, Jan 29, 2020
  3. 2/6 git-p4: add P4CommandException to report errors talking to PerforceLuke Diamand, Jan 29, 2020
  4. 3/6 git-p4: disable some pylint warnings, to get pylint output to something manageableLuke Diamand, Jan 29, 2020
  5. 4/6 git-p4: create helper function importRevisions()Luke Diamand, Jan 29, 2020
  6. 5/6 git-p4: cleanup better on error exitLuke Diamand, Jan 29, 2020
  7. 6/6 git-p4: check for access to remote host earlierLuke Diamand, Jan 29, 2020
  8. Eric SunshineJan 29, 2020
  9. Luke DiamandJan 29, 2020
  10. Junio C HamanoJan 30, 2020
  11. Luke DiamandJan 31, 2020
  12. Johannes SchindelinJan 30, 2020
  13. Luke DiamandJan 30, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.