[PATCHv1 0/1] git-p4: better error reporting
From: Luke Diamand <hidden>
Date: 2018-06-05 08:55:37
Subsystems: the rest · Maintainer: Linus Torvalds
Series: 1 patch touching 2 files, +133/-0
2 messages, 1 author, 2018-06-05 · open the first message on its own page
From: Luke Diamand <hidden>
Date: 2018-06-05 08:55:37
If git-p4 cannot talk to the Perforce server, it will either give a confusing error message, or just crash. Even I get tripped up by this. This change just checks that the client can talk to the server, and in particular that the user is logged in (the default login timeout is 12 hours). It would be nice to also check for ticket expiration during long Perforce operations, but this change does not attempt to do that. Luke Diamand (1): git-p4: better error reporting when p4 fails git-p4.py | 55 +++++++++++++++++++++++++++++++++ t/t9833-errors.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100755 t/t9833-errors.sh -- 2.17.0.392.gdeb1a6e9b7
From: Luke Diamand <hidden>
Date: 2018-06-05 08:55:40
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 <redacted> --- 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 b9e79f1d8b..66652f8280 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 + 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