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 <pw@padd.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Pete Wyckoff committed Nov 23, 2012 at 17:35 UTC 78189bead3f5fde22ae651d66208a0e0a375a819
2 files changed +43 -8
git-p4.py
+31 -7
@@ -2139,6 +2139,29 @@ class P4Sync(Command, P4UserMap):
2139 # handle another chunk of streaming data
2140 def streamP4FilesCb(self, marshalled):
2141
2142 + # catch p4 errors and complain
2143 + err = None
2144 + if "code" in marshalled:
2145 + if marshalled["code"] == "error":
2146 + if "data" in marshalled:
2147 + err = marshalled["data"].rstrip()
2148 + if err:
2149 + f = None
2150 + if self.stream_have_file_info:
2151 + if "depotFile" in self.stream_file:
2152 + f = self.stream_file["depotFile"]
2153 + # force a failure in fast-import, else an empty
2154 + # commit will be made
2155 + self.gitStream.write("\n")
2156 + self.gitStream.write("die-now\n")
2157 + self.gitStream.close()
2158 + # ignore errors, but make sure it exits first
2159 + self.importProcess.wait()
2160 + if f:
2161 + die("Error from p4 print for %s: %s" % (f, err))
2162 + else:
2163 + die("Error from p4 print: %s" % err)
2164 +
2165 if marshalled.has_key('depotFile') and self.stream_have_file_info:
2166 # start of a new file - output the old one first
2167 self.streamOneP4File(self.stream_file, self.stream_contents)
@@ -2900,12 +2923,13 @@ class P4Sync(Command, P4UserMap):
2923
2924 self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))
2925
2903 - importProcess = subprocess.Popen(["git", "fast-import"],
2904 - stdin=subprocess.PIPE, stdout=subprocess.PIPE,
2905 - stderr=subprocess.PIPE);
2906 - self.gitOutput = importProcess.stdout
2907 - self.gitStream = importProcess.stdin
2908 - self.gitError = importProcess.stderr
2926 + self.importProcess = subprocess.Popen(["git", "fast-import"],
2927 + stdin=subprocess.PIPE,
2928 + stdout=subprocess.PIPE,
2929 + stderr=subprocess.PIPE);
2930 + self.gitOutput = self.importProcess.stdout
2931 + self.gitStream = self.importProcess.stdin
2932 + self.gitError = self.importProcess.stderr
2933
2934 if revision:
2935 self.importHeadRevision(revision)
@@ -2965,7 +2989,7 @@ class P4Sync(Command, P4UserMap):
2989 self.importP4Labels(self.gitStream, missingP4Labels)
2990
2991 self.gitStream.close()
2968 - if importProcess.wait() != 0:
2992 + if self.importProcess.wait() != 0:
2993 die("fast-import failed: %s" % self.gitError.read())
2994 self.gitOutput.close()
2995 self.gitError.close()
t/t9800-git-p4-basic.sh
+12 -1
@@ -143,7 +143,18 @@ test_expect_success 'exit when p4 fails to produce marshaled output' '
143 ! test_i18ngrep Traceback errs
144 '
145
146 -test_expect_success 'clone bare' '
146 +# Hide a file from p4d, make sure we catch its complaint. This won't fail in
147 +# p4 changes, files, or describe; just in p4 print. If P4CLIENT is unset, the
148 +# message will include "Librarian checkout".
149 +test_expect_success 'exit gracefully for p4 server errors' '
150 + test_when_finished "mv \"$db\"/depot/file1,v,hidden \"$db\"/depot/file1,v" &&
151 + mv "$db"/depot/file1,v "$db"/depot/file1,v,hidden &&
152 + test_when_finished cleanup_git &&
153 + test_expect_code 1 git p4 clone --dest="$git" //depot@1 >out 2>err &&
154 + test_i18ngrep "Error from p4 print" err
155 +'
156 +
157 +test_expect_success 'clone --bare should make a bare repository' '
158 rm -rf "$git" &&
159 git p4 clone --dest="$git" --bare //depot &&
160 test_when_finished cleanup_git &&