Permalink
Browse files

Merge branch 'pw/p4-various-fixes' into next

* pw/p4-various-fixes:
  git p4: remove unneeded cmd initialization
  git p4: fix labelDetails typo in exception
  git p4 test: display unresolvable host error
  git p4: catch p4 errors when streaming file contents
  git p4: handle servers without move support
  git p4: catch p4 describe errors
  • Loading branch information...
gitster committed Nov 28, 2012
2 parents 5528439 + 73350fb commit 1a6f9a9e64f9c2e2d73006b9befaf1cc228a4839
Showing with 137 additions and 20 deletions.
  1. +78 −19 git-p4.py
  2. +24 −1 t/t9800-git-p4-basic.sh
  3. +35 −0 t/t9814-git-p4-rename.sh
View
@@ -129,6 +129,25 @@ def p4_has_command(cmd):
p.communicate()
return p.returncode == 0
+def p4_has_move_command():
+ """See if the move command exists, that it supports -k, and that
+ it has not been administratively disabled. The arguments
+ must be correct, but the filenames do not have to exist. Use
+ ones with wildcards so even if they exist, it will fail."""
+
+ if not p4_has_command("move"):
+ return False
+ cmd = p4_build_cmd(["move", "-k", "@from", "@to"])
+ p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ (out, err) = p.communicate()
+ # return code will be 1 in either case
+ if err.find("Invalid option") >= 0:
+ return False
+ if err.find("disabled") >= 0:
+ return False
+ # assume it failed because @... was invalid changelist
+ return True
+
def system(cmd):
expand = isinstance(cmd,basestring)
if verbose:
@@ -169,6 +188,29 @@ def p4_reopen(type, f):
def p4_move(src, dest):
p4_system(["move", "-k", wildcard_encode(src), wildcard_encode(dest)])
+def p4_describe(change):
+ """Make sure it returns a valid result by checking for
+ the presence of field "time". Return a dict of the
+ results."""
+
+ ds = p4CmdList(["describe", "-s", str(change)])
+ if len(ds) != 1:
+ die("p4 describe -s %d did not return 1 result: %s" % (change, str(ds)))
+
+ d = ds[0]
+
+ if "p4ExitCode" in d:
+ die("p4 describe -s %d exited with %d: %s" % (change, d["p4ExitCode"],
+ str(d)))
+ if "code" in d:
+ if d["code"] == "error":
+ die("p4 describe -s %d returned error code: %s" % (change, str(d)))
+
+ if "time" not in d:
+ die("p4 describe -s %d returned no \"time\": %s" % (change, str(d)))
+
+ return d
+
#
# Canonicalize the p4 type and return a tuple of the
# base type, plus any modifiers. See "p4 help filetypes"
@@ -871,7 +913,7 @@ def __init__(self):
self.conflict_behavior = None
self.isWindows = (platform.system() == "Windows")
self.exportLabels = False
- self.p4HasMoveCommand = p4_has_command("move")
+ self.p4HasMoveCommand = p4_has_move_command()
def check(self):
if len(p4CmdList("opened ...")) > 0:
@@ -2097,6 +2139,29 @@ def streamOneP4Deletion(self, file):
# 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)
@@ -2341,7 +2406,7 @@ def importP4Labels(self, stream, p4Labels):
try:
tmwhen = time.strptime(labelDetails['Update'], "%Y/%m/%d %H:%M:%S")
except ValueError:
- print "Could not convert label time %s" % labelDetail['Update']
+ print "Could not convert label time %s" % labelDetails['Update']
tmwhen = 1
when = int(time.mktime(tmwhen))
@@ -2543,7 +2608,7 @@ def searchParent(self, parent, branch, target):
def importChanges(self, changes):
cnt = 1
for change in changes:
- description = p4Cmd(["describe", str(change)])
+ description = p4_describe(change)
self.updateOptionDict(description)
if not self.silent:
@@ -2667,14 +2732,8 @@ def importHeadRevision(self, revision):
# Use time from top-most change so that all git p4 clones of
# the same p4 repo have the same commit SHA1s.
- res = p4CmdList("describe -s %d" % newestRevision)
- newestTime = None
- for r in res:
- if r.has_key('time'):
- newestTime = int(r['time'])
- if newestTime is None:
- die("\"describe -s\" on newest change %d did not give a time")
- details["time"] = newestTime
+ res = p4_describe(newestRevision)
+ details["time"] = res["time"]
self.updateOptionDict(details)
try:
@@ -2864,12 +2923,13 @@ def run(self, args):
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)
@@ -2929,7 +2989,7 @@ def run(self, args):
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()
@@ -3128,7 +3188,6 @@ def main():
printUsage(commands.keys())
sys.exit(2)
- cmd = ""
cmdName = sys.argv[1]
try:
klass = commands[cmdName]
View
@@ -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 &&
@@ -172,6 +183,18 @@ test_expect_success 'initial import time from top change time' '
)
'
+test_expect_success 'unresolvable host in P4PORT should display error' '
+ test_when_finished cleanup_git &&
+ git p4 clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ P4PORT=nosuchhost:65537 &&
+ export P4PORT &&
+ test_expect_code 1 git p4 sync >out 2>err &&
+ grep "connect to nosuchhost" err
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
View
@@ -199,6 +199,41 @@ test_expect_success 'detect copies' '
)
'
+# See if configurables can be set, and in particular if the run.move.allow
+# variable exists, which allows admins to disable the "p4 move" command.
+test_expect_success 'p4 configure command and run.move.allow are available' '
+ p4 configure show run.move.allow >out ; retval=$? &&
+ test $retval = 0 &&
+ {
+ egrep ^run.move.allow: out &&
+ test_set_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW ||
+ true
+ } || true
+'
+
+# If move can be disabled, turn it off and test p4 move handling
+test_expect_success P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW \
+ 'do not use p4 move when administratively disabled' '
+ test_when_finished "p4 configure set run.move.allow=1" &&
+ p4 configure set run.move.allow=0 &&
+ (
+ cd "$cli" &&
+ echo move-disallow-file >move-disallow-file &&
+ p4 add move-disallow-file &&
+ p4 submit -d "add move-disallow-file"
+ ) &&
+ test_when_finished cleanup_git &&
+ git p4 clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.detectRenames true &&
+ git mv move-disallow-file move-disallow-file-moved &&
+ git commit -m "move move-disallow-file" &&
+ git p4 submit
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'

0 comments on commit 1a6f9a9

Please sign in to comment.