git-p4: cleanup better on error exit
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Luke Diamand committed
Jan 29, 2020 at 11:12 UTC
6026aff5bbe7389fa276188237b58afbef1b07ff
1 file changed
+28
-15
git-p4.py
+28
-15
@@ -169,6 +169,9 @@ def calcDiskFree():
169
return st.f_bavail * st.f_frsize
170
171
def die(msg):
172
+ """ Terminate execution. Make sure that any running child processes have been wait()ed for before
173
+ calling this.
174
+ """
175
if verbose:
176
raise Exception(msg)
177
else:
@@ -3574,7 +3577,7 @@ class P4Sync(Command, P4UserMap):
3577
# does not have any existing p4 branches
3578
if len(args) == 0:
3579
if not self.p4BranchesInGit:
3577
- die("No remote p4 branches. Perhaps you never did \"git p4 clone\" in here.")
3580
+ raise P4CommandException("No remote p4 branches. Perhaps you never did \"git p4 clone\" in here.")
3581
3582
# The default branch is master, unless --branch is used to
3583
# specify something else. Make sure it exists, or complain
@@ -3582,9 +3585,9 @@ class P4Sync(Command, P4UserMap):
3585
if not self.detectBranches:
3586
if not branch_exists(self.branch):
3587
if branch_arg_given:
3585
- die("Error: branch %s does not exist." % self.branch)
3588
+ raise P4CommandException("Error: branch %s does not exist." % self.branch)
3589
else:
3587
- die("Error: no branch %s; perhaps specify one with --branch." %
3590
+ raise P4CommandException("Error: no branch %s; perhaps specify one with --branch." %
3591
self.branch)
3592
3593
if self.verbose:
@@ -3825,22 +3828,32 @@ class P4Sync(Command, P4UserMap):
3828
3829
self.openStreams()
3830
3828
- if revision:
3829
- self.importHeadRevision(revision)
3830
- else:
3831
- self.importRevisions(args, branch_arg_given)
3831
+ err = None
3832
3833
- if gitConfigBool("git-p4.importLabels"):
3834
- self.importLabels = True
3833
+ try:
3834
+ if revision:
3835
+ self.importHeadRevision(revision)
3836
+ else:
3837
+ self.importRevisions(args, branch_arg_given)
3838
3836
- if self.importLabels:
3837
- p4Labels = getP4Labels(self.depotPaths)
3838
- gitTags = getGitTags()
3839
+ if gitConfigBool("git-p4.importLabels"):
3840
+ self.importLabels = True
3841
3840
- missingP4Labels = p4Labels - gitTags
3841
- self.importP4Labels(self.gitStream, missingP4Labels)
3842
+ if self.importLabels:
3843
+ p4Labels = getP4Labels(self.depotPaths)
3844
+ gitTags = getGitTags()
3845
3843
- self.closeStreams()
3846
+ missingP4Labels = p4Labels - gitTags
3847
+ self.importP4Labels(self.gitStream, missingP4Labels)
3848
+
3849
+ except P4CommandException as e:
3850
+ err = e
3851
+
3852
+ finally:
3853
+ self.closeStreams()
3854
+
3855
+ if err:
3856
+ die(str(err))
3857
3858
# Cleanup temporary branches created during import
3859
if self.tempBranches != []: