[PATCHv2 3/6] git-p4: better error reporting when p4 fails

[Luke Diamand <luke@diamand.org>]

Currently when p4 fails to run, git-p4 just crashes with an obscure
error message.

For example, if the P4 ticket has expired, you get:

  Error: Cannot locate perforce checkout of <path> in client view

This change checks whether git-p4 can talk to the Perforce server when
the first P4 operation is attempted, and tries to print a meaningful
error message if it fails.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py         | 55 +++++++++++++++++++++++++++++++++
 t/t9833-errors.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+)
 create mode 100755 t/t9833-errors.sh

diff --git a/git-p4.py b/git-p4.py
index b61f47cc61..3de12a4a0a 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -50,6 +50,8 @@ def __str__(self):
 # Grab changes in blocks of this many revisions, unless otherwise requested
 defaultBlockSize = 512
 
+p4_access_checked = False
+
 def p4_build_cmd(cmd):
     """Build a suitable p4 command line.
 
@@ -91,6 +93,13 @@ def p4_build_cmd(cmd):
         real_cmd = ' '.join(real_cmd) + ' ' + cmd
     else:
         real_cmd += cmd
+
+    # now check that we can actually talk to the server
+    global p4_access_checked
+    if not p4_access_checked:
+        p4_access_checked = True    # suppress access checks in p4_check_access itself
+        p4_check_access()
+
     return real_cmd
 
 def git_dir(path):
@@ -264,6 +273,52 @@ def p4_system(cmd):
     if retcode:
         raise CalledProcessError(retcode, real_cmd)
 
+def die_bad_access(s):
+    die("failure accessing depot: {0}".format(s.rstrip()))
+
+def p4_check_access(min_expiration=1):
+    """ Check if we can access Perforce - account still logged in
+    """
+    results = p4CmdList(["login", "-s"])
+
+    if len(results) == 0:
+        # should never get here: always get either some results, or a p4ExitCode
+        assert("could not parse response from perforce")
+
+    result = results[0]
+
+    if 'p4ExitCode' in result:
+        # p4 returned non-zero status, e.g. P4PORT invalid, or p4 not in path
+        die_bad_access("could not run p4")
+
+    code = result.get("code")
+    if not code:
+        # we get here if we couldn't connect and there was nothing to unmarshal
+        die_bad_access("could not connect")
+
+    elif code == "stat":
+        expiry = result.get("TicketExpiration")
+        if expiry:
+            expiry = int(expiry)
+            if expiry > min_expiration:
+                # ok to carry on
+                return
+            else:
+                die_bad_access("perforce ticket expires in {0} seconds".format(expiry))
+
+        else:
+            # account without a timeout - all ok
+            return
+
+    elif code == "error":
+        data = result.get("data")
+        if data:
+            die_bad_access("p4 error: {0}".format(data))
+        else:
+            die_bad_access("unknown error")
+    else:
+        die_bad_access("unknown error code {0}".format(code))
+
 _p4_version_string = None
 def p4_version_string():
     """Read the version string, showing just the last line, which
diff --git a/t/t9833-errors.sh b/t/t9833-errors.sh
new file mode 100755
index 0000000000..9ba892de7a
--- /dev/null
+++ b/t/t9833-errors.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='git p4 errors'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'add p4 files' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "file1"
+	)
+'
+
+# after this test, the default user requires a password
+test_expect_success 'error handling' '
+	git p4 clone --dest="$git" //depot@all &&
+	(
+		cd "$git" &&
+		P4PORT=: test_must_fail git p4 submit 2>errmsg
+	) &&
+	p4 passwd -P newpassword &&
+	(
+		P4PASSWD=badpassword test_must_fail git p4 clone //depot/foo 2>errmsg &&
+		grep -q "failure accessing depot.*P4PASSWD" errmsg
+	)
+'
+
+test_expect_success 'ticket logged out' '
+	P4TICKETS="$cli/tickets" &&
+	echo "newpassword" | p4 login &&
+	(
+		cd "$git" &&
+		test_commit "ticket-auth-check" &&
+		p4 logout &&
+		test_must_fail git p4 submit 2>errmsg &&
+		grep -q "failure accessing depot" errmsg
+	)
+'
+
+test_expect_success 'create group with short ticket expiry' '
+	P4TICKETS="$cli/tickets" &&
+	echo "newpassword" | p4 login &&
+	p4_add_user short_expiry_user &&
+	p4 -u short_expiry_user passwd -P password &&
+	p4 group -i <<-EOF &&
+	Group: testgroup
+	Timeout: 3
+	Users: short_expiry_user
+	EOF
+
+	p4 users | grep short_expiry_user
+'
+
+test_expect_success 'git operation with expired ticket' '
+	P4TICKETS="$cli/tickets" &&
+	P4USER=short_expiry_user &&
+	echo "password" | p4 login &&
+	(
+		cd "$git" &&
+		git p4 sync &&
+		sleep 5 &&
+		test_must_fail git p4 sync 2>errmsg &&
+		grep "failure accessing depot" errmsg
+	)
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+
+test_done
-- 
2.17.0.392.gdeb1a6e9b7