From 78189bead3f5fde22ae651d66208a0e0a375a819 Mon Sep 17 00:00:00 2001 From: Pete Wyckoff Date: Fri, 23 Nov 2012 17:35:36 -0500 Subject: git p4: catch p4 errors when streaming file contents Error messages that arise during the "p4 print" phase of generating commits were silently ignored. Catch them, abort the fast-import, and exit. Without this fix, the sync/clone appears to work, but files that are inaccessible by the p4d server will still be imported to git, although without the proper contents. Instead the errant files will contain a p4 error message, such as "Librarian checkout //depot/path failed". Signed-off-by: Pete Wyckoff Signed-off-by: Junio C Hamano --- git-p4.py | 38 +++++++++++++++++++++++++++++++------- t/t9800-git-p4-basic.sh | 13 ++++++++++++- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/git-p4.py b/git-p4.py index 9644c9f87..cb1ec8d66 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2139,6 +2139,29 @@ class P4Sync(Command, P4UserMap): # handle another chunk of streaming data def streamP4FilesCb(self, marshalled): + # catch p4 errors and complain + err = None + if "code" in marshalled: + if marshalled["code"] == "error": + if "data" in marshalled: + err = marshalled["data"].rstrip() + if err: + f = None + if self.stream_have_file_info: + if "depotFile" in self.stream_file: + f = self.stream_file["depotFile"] + # force a failure in fast-import, else an empty + # commit will be made + self.gitStream.write("\n") + self.gitStream.write("die-now\n") + self.gitStream.close() + # ignore errors, but make sure it exits first + self.importProcess.wait() + if f: + die("Error from p4 print for %s: %s" % (f, err)) + else: + die("Error from p4 print: %s" % err) + if marshalled.has_key('depotFile') and self.stream_have_file_info: # start of a new file - output the old one first self.streamOneP4File(self.stream_file, self.stream_contents) @@ -2900,12 +2923,13 @@ class P4Sync(Command, P4UserMap): self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) - importProcess = subprocess.Popen(["git", "fast-import"], - stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE); - self.gitOutput = importProcess.stdout - self.gitStream = importProcess.stdin - self.gitError = importProcess.stderr + self.importProcess = subprocess.Popen(["git", "fast-import"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE); + self.gitOutput = self.importProcess.stdout + self.gitStream = self.importProcess.stdin + self.gitError = self.importProcess.stderr if revision: self.importHeadRevision(revision) @@ -2965,7 +2989,7 @@ class P4Sync(Command, P4UserMap): self.importP4Labels(self.gitStream, missingP4Labels) self.gitStream.close() - if importProcess.wait() != 0: + if self.importProcess.wait() != 0: die("fast-import failed: %s" % self.gitError.read()) self.gitOutput.close() self.gitError.close() diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh index b7ad716b0..05797c31d 100755 --- a/t/t9800-git-p4-basic.sh +++ b/t/t9800-git-p4-basic.sh @@ -143,7 +143,18 @@ test_expect_success 'exit when p4 fails to produce marshaled output' ' ! test_i18ngrep Traceback errs ' -test_expect_success 'clone bare' ' +# Hide a file from p4d, make sure we catch its complaint. This won't fail in +# p4 changes, files, or describe; just in p4 print. If P4CLIENT is unset, the +# message will include "Librarian checkout". +test_expect_success 'exit gracefully for p4 server errors' ' + test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" && + mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden && + test_when_finished cleanup_git && + test_expect_code 1 git p4 clone --dest="$git" //depot@1 >out 2>err && + test_i18ngrep "Error from p4 print" err +' + +test_expect_success 'clone --bare should make a bare repository' ' rm -rf "$git" && git p4 clone --dest="$git" --bare //depot && test_when_finished cleanup_git && -- cgit v1.2.1