got send: show failure in updating remote branches

Gonzalo hit an issue earlier this morning: `got send -f' was failing because of server-side checks on github. It's wasn't obvious what failure was because to read the error message one had to pass -vvv and read the raw remote packet line. Diff below propagates the error read from got-send-pack where it's parsed up to the got send pack callback. Now, pushing results in: % got send -f github Connecting to "github" git@github.com 39 commits colored; 6 objects found; 4 trees scanned packing 1 reference; 6 objects; deltify: 100%; uploading pack: 484B 100% Server has rejected refs/heads/main: protected branch hook declined which at least gives a clue to what's happening. ok/thoughts? diff /home/op/w/got commit - 712c26063dd1a052a8c12603c51a363a14955915 path + /home/op/w/gotd blob - c0c38ea47dfbd657ee0c14d0e8829767ad691ae6 file + got/got.c --- got/got.c +++ got/got.c @@ -8749,7 +8749,8 @@ send_progress(void *arg, int ncolored, int nfound, int static const struct got_error * send_progress(void *arg, int ncolored, int nfound, int ntrees, off_t packfile_size, int ncommits, int nobj_total, int nobj_deltify, - int nobj_written, off_t bytes_sent, const char *refname, int success) + int nobj_written, off_t bytes_sent, const char *refname, + const char *errmsg, int success) { struct got_send_progress_arg *a = arg; char scaled_packsize[FMT_SCALED_STRSIZE]; @@ -8781,6 +8782,8 @@ send_progress(void *arg, int ncolored, int nfound, int if (a->printed_something) putchar('\n'); printf("Server has %s %s", status, refname); + if (errmsg) + printf(": %s", errmsg); a->printed_something = 1; return NULL; } blob - 19e9e4f116e70c354ec4bbff8ce7e5a1615acd1f file + include/got_send.h --- include/got_send.h +++ include/got_send.h @@ -38,7 +38,7 @@ typedef const struct got_error *(*got_send_progress_cb typedef const struct got_error *(*got_send_progress_cb)(void *, int ncolored, int nfound, int ntrees, off_t packfile_size, int ncommits, int nobj_total, int nobj_deltify, int nobj_written, off_t bytes_sent, - const char *refname, int success); + const char *refname, const char *, int success); /* * Attempt to generate a pack file and sent it to a server. blob - f84d920be3d90fdc2c9b6db77113efd0d7d87c62 file + lib/got_lib_privsep.h --- lib/got_lib_privsep.h +++ lib/got_lib_privsep.h @@ -481,7 +481,9 @@ struct got_imsg_send_ref_status { struct got_imsg_send_ref_status { int success; size_t name_len; + size_t errmsg_len; /* Followed by name_len data bytes. */ + /* Followed by errmsg_len data bytes. */ } __attribute__((__packed__)); /* Structure for GOT_IMSG_IDXPACK_REQUEST data. */ @@ -702,7 +704,7 @@ const struct got_error *got_privsep_recv_send_progress struct got_pathlist_head *, struct imsgbuf *); const struct got_error *got_privsep_send_packfd(struct imsgbuf *, int); const struct got_error *got_privsep_recv_send_progress(int *, off_t *, - int *, char **, struct imsgbuf *); + int *, char **, char **, struct imsgbuf *); const struct got_error *got_privsep_get_imsg_obj(struct got_object **, struct imsg *, struct imsgbuf *); const struct got_error *got_privsep_recv_obj(struct got_object **, blob - 175c3a0c4138ebc26f9e5160af4f028ccf1f46e7 file + lib/privsep.c --- lib/privsep.c +++ lib/privsep.c @@ -964,7 +964,7 @@ got_privsep_recv_send_progress(int *done, off_t *bytes const struct got_error * got_privsep_recv_send_progress(int *done, off_t *bytes_sent, - int *success, char **refname, struct imsgbuf *ibuf) + int *success, char **refname, char **errmsg, struct imsgbuf *ibuf) { const struct got_error *err = NULL; struct imsg imsg; @@ -975,6 +975,7 @@ got_privsep_recv_send_progress(int *done, off_t *bytes *done = 0; *success = 0; *refname = NULL; + *errmsg = NULL; err = got_privsep_recv_imsg(&imsg, ibuf, 0); if (err) @@ -995,13 +996,18 @@ got_privsep_recv_send_progress(int *done, off_t *bytes break; } memcpy(&iref_status, imsg.data, sizeof(iref_status)); - if (datalen != sizeof(iref_status) + iref_status.name_len) { + if (datalen != sizeof(iref_status) + iref_status.name_len + + iref_status.errmsg_len) { err = got_error(GOT_ERR_PRIVSEP_MSG); break; } *success = iref_status.success; *refname = strndup(imsg.data + sizeof(iref_status), iref_status.name_len); + + if (iref_status.errmsg_len != 0) + *errmsg = strndup(imsg.data + sizeof(iref_status) + + iref_status.name_len, iref_status.errmsg_len); break; case GOT_IMSG_SEND_DONE: if (datalen != 0) { blob - f916422dab87523c5b073b161f9ba85ad2f970b2 file + lib/send.c --- lib/send.c +++ lib/send.c @@ -126,7 +126,7 @@ pack_progress(void *arg, int ncolored, int nfound, int err = a->progress_cb(a->progress_arg, ncolored, nfound, ntrees, packfile_size, ncommits, nobj_total, nobj_deltify, - nobj_written, 0, NULL, 0); + nobj_written, 0, NULL, NULL, 0); if (err) return err; @@ -670,13 +670,15 @@ got_send_pack(const char *remote_name, struct got_path while (!done) { int success = 0; char *refname = NULL; + char *errmsg = NULL; + if (cancel_cb) { err = (*cancel_cb)(cancel_arg); if (err) goto done; } err = got_privsep_recv_send_progress(&done, &bytes_sent, - &success, &refname, &sendibuf); + &success, &refname, &errmsg, &sendibuf); if (err) goto done; if (refname && got_ref_name_is_valid(refname) && success && @@ -700,14 +702,16 @@ got_send_pack(const char *remote_name, struct got_path ppa.nfound, ppa.ntrees, ppa.packfile_size, ppa.ncommits, ppa.nobj_total, ppa.nobj_deltify, ppa.nobj_written, bytes_sent, - refname, success); + refname, errmsg, success); if (err) { free(refname); + free(errmsg); goto done; } bytes_sent_cur = bytes_sent; } free(refname); + free(errmsg); } done: if (sendpid != -1) { blob - c773102729c2714c83c3b63b36a5ce9699aba978 file + libexec/got-send-pack/Makefile --- libexec/got-send-pack/Makefile +++ libexec/got-send-pack/Makefile @@ -5,7 +5,7 @@ SRCS= got-send-pack.c error.c inflate.c object_parse. PROG= got-send-pack SRCS= got-send-pack.c error.c inflate.c object_parse.c \ path.c privsep.c sha1.c pkt.c gitproto.c ratelimit.c \ - pollfd.c + pollfd.c reference_parse.c CPPFLAGS = -I${.CURDIR}/../../include -I${.CURDIR}/../../lib blob - 0e18f194406f1c8fe2f30bab9b5c8a3a3c0a6e5f file + libexec/got-send-pack/got-send-pack.c --- libexec/got-send-pack/got-send-pack.c +++ libexec/got-send-pack/got-send-pack.c @@ -223,10 +223,11 @@ send_ref_status(struct imsgbuf *ibuf, const char *refn struct got_pathlist_head *refs, struct got_pathlist_head *delete_refs) { struct ibuf *wbuf; - size_t len, reflen = strlen(refname); + size_t len, reflen, errmsglen = 0; struct got_pathlist_entry *pe; int ref_valid = 0; - char *eol; + char *eol, *sp; + const char *errmsg = ""; eol = strchr(refname, '\n'); if (eol == NULL) { @@ -235,6 +236,19 @@ send_ref_status(struct imsgbuf *ibuf, const char *refn } *eol = '\0'; + sp = strchr(refname, ' '); + if (sp != NULL) { + *sp++ = '\0'; + errmsg = sp; + errmsglen = strlen(errmsg); + } + + reflen = strlen(refname); + if (!got_ref_name_is_valid(refname)) { + return got_error_msg(GOT_ERR_BAD_PACKET, + "unexpected message from server"); + } + TAILQ_FOREACH(pe, refs, entry) { if (strcmp(refname, pe->path) == 0) { ref_valid = 1; @@ -254,7 +268,7 @@ send_ref_status(struct imsgbuf *ibuf, const char *refn "unexpected message from server"); } - len = sizeof(struct got_imsg_send_ref_status) + reflen; + len = sizeof(struct got_imsg_send_ref_status) + reflen + errmsglen; if (len >= MAX_IMSGSIZE - IMSG_HEADER_SIZE) return got_error(GOT_ERR_NO_SPACE); @@ -268,8 +282,12 @@ send_ref_status(struct imsgbuf *ibuf, const char *refn return got_error_from_errno("imsg_add SEND_REF_STATUS"); if (imsg_add(wbuf, &reflen, sizeof(reflen)) == -1) return got_error_from_errno("imsg_add SEND_REF_STATUS"); + if (imsg_add(wbuf, &errmsglen, sizeof(errmsglen)) == -1) + return got_error_from_errno("imsg_add SEND_REF_STATUS"); if (imsg_add(wbuf, refname, reflen) == -1) return got_error_from_errno("imsg_add SEND_REF_STATUS"); + if (imsg_add(wbuf, errmsg, errmsglen) == -1) + return got_error_from_errno("imsg_add SEND_REF_STATUS"); wbuf->fd = -1; imsg_close(ibuf, wbuf); blob - f989b0dbd6a3ca710184d881f574e55a5eb83e78 file + regress/cmdline/send.sh --- regress/cmdline/send.sh +++ regress/cmdline/send.sh @@ -1513,6 +1513,69 @@ test_parseargs "$@" test_done "$testroot" "$ret" } +test_send_rejected() { + local testroot=`test_init send_rejected` + local testurl=ssh://127.0.0.1/$testroot + local commit_id=`git_show_head $testroot/repo` + + if ! got clone -q "$testurl/repo" "$testroot/repo-clone"; then + echo "got clone command failed unexpectedly" >&2 + test_done "$testroot" 1 + return 1 + fi + + mkdir "$testroot/repo-clone/hooks" + cat <<'EOF' >$testroot/repo-clone/hooks/update +case "$1" in + *master*) + echo "rejecting push on master branch" + exit 1 + ;; +esac +exit 0 +EOF + chmod +x "$testroot/repo-clone/hooks/update" + + cat > $testroot/repo/.git/got.conf <<EOF +remote "origin" { + protocol ssh + server 127.0.0.1 + repository "$testroot/repo-clone" +} +EOF + + echo "modified alpha" >$testroot/repo/alpha + git_commit "$testroot/repo" -m "modified alpha" + + got send -q -r "$testroot/repo" >$testroot/stdout 2>$testroot/stderr + ret=$? + if [ $ret -ne 0 ]; then + echo "got send command failed unexpectedly" >&2 + test_done "$testroot" $ret + return 1 + fi + + touch "$testroot/stdout.expected" + if ! cmp -s "$testroot/stdout.expected" "$testroot/stdout"; then + diff -u "$testroot/stdout.expected" "$testroot/stdout" + test_done "$testroot" 1 + return 1 + fi + + cat <<EOF >$testroot/stderr.expected +rejecting push on master branch +error: hook declined to update refs/heads/master +EOF + + if ! cmp -s "$testroot/stderr.expected" "$testroot/stderr"; then + diff -u "$testroot/stderr.expected" "$testroot/stderr" + test_done "$testroot" 1 + return 1 + fi + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_send_basic run_test test_send_rebase_required @@ -1526,3 +1589,4 @@ run_test test_send_config run_test test_send_to_empty_repo run_test test_send_and_fetch_config run_test test_send_config +run_test test_send_rejected

2022-11-14 16:05 Omar Polo:
got send: show failure in updating remote branches
- 2022-11-14 16:54 Omar Polo:
  got send: show failure in updating remote branches
- - 2022-11-14 20:14 Stefan Sperling:
    got send: show failure in updating remote branches