diff --git a/send-pack.c b/send-pack.c index da4741ce4a..d1b7edc995 100644 --- a/send-pack.c +++ b/send-pack.c @@ -190,10 +190,8 @@ static int receive_status(struct packet_reader *reader, struct ref *refs) if (reader->line[0] == 'o' && reader->line[1] == 'k') hint->status = REF_STATUS_OK; - else { + else hint->status = REF_STATUS_REMOTE_REJECT; - ret = -1; - } hint->remote_status = xstrdup_or_null(msg); /* start our next search from the next ref */ hint = hint->next; @@ -322,29 +320,6 @@ static int generate_push_cert(struct strbuf *req_buf, return update_seen; } - -static int atomic_push_failure(struct send_pack_args *args, - struct ref *remote_refs, - struct ref *failing_ref) -{ - struct ref *ref; - /* Mark other refs as failed */ - for (ref = remote_refs; ref; ref = ref->next) { - if (!ref->peer_ref && !args->send_mirror) - continue; - - switch (ref->status) { - case REF_STATUS_EXPECTING_REPORT: - ref->status = REF_STATUS_ATOMIC_PUSH_FAILED; - continue; - default: - break; /* do nothing */ - } - } - return error("atomic push failed for ref %s. status: %d\n", - failing_ref->name, failing_ref->status); -} - #define NONCE_LEN_LIMIT 256 static void reject_invalid_nonce(const char *nonce, int len) @@ -489,7 +464,10 @@ int send_pack(struct send_pack_args *args, if (use_atomic) { strbuf_release(&req_buf); strbuf_release(&cap_buf); - return atomic_push_failure(args, remote_refs, ref); + reject_atomic_push(remote_refs, args->send_mirror); + error("atomic push failed for ref %s. status: %d\n", + ref->name, ref->status); + return args->porcelain ? 0 : -1; } /* else fallthrough */ default: diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 645b4c78d3..a32efe2b6c 100755 --- a/t/t5504-fetch-receive-strict.sh +++ b/t/t5504-fetch-receive-strict.sh @@ -65,6 +65,7 @@ test_expect_success 'fetch with transfer.fsckobjects' ' cat >exp <.git/foo "To testrepo" && echo >>.git/foo "! refs/heads/master:refs/heads/master [remote rejected] (branch is currently checked out)" && + echo >>.git/foo "Done" && test_must_fail git push >.git/bar --porcelain testrepo refs/heads/master:refs/heads/master && test_cmp .git/foo .git/bar diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index 23be8ce92d..afc680d5e3 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -177,6 +177,9 @@ test_expect_success 'push (chunked)' ' test $HEAD = $(git rev-parse --verify HEAD)) ' +## References of remote: atomic1(1) master(2) collateral(2) other(2) +## References of local : atomic2(2) master(1) collateral(3) other(2) collateral1(3) atomic(1) +## Atomic push : master(1) collateral(3) atomic(1) test_expect_success 'push --atomic also prevents branch creation, reports collateral' ' # Setup upstream repo - empty for now d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git && @@ -189,7 +192,8 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat test_commit atomic2 && git branch collateral && git branch other && - git push "$up" master collateral other && + git push "$up" atomic1 master collateral other && + git tag -d atomic1 && # collateral is a valid push, but should be failed by atomic push git checkout collateral && @@ -224,7 +228,11 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat # the collateral failure refs should be indicated to the user grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output && - grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output + grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output && + + # never report what we do not push + ! grep "^ ! .*rejected.* atomic1 " output && + ! grep "^ ! .*rejected.* other " output ' test_expect_success 'push --atomic fails on server-side errors' ' diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh index 7079bcf9a0..620c30d58f 100755 --- a/t/t5543-atomic-push.sh +++ b/t/t5543-atomic-push.sh @@ -27,6 +27,12 @@ test_refs () { test_cmp expect actual } +fmt_status_report () { + sed -n \ + -e "/^To / { s/ */ /g; p; }" \ + -e "/^ ! / { s/ */ /g; p; }" +} + test_expect_success 'atomic push works for a single branch' ' mk_repo_pair && ( @@ -191,4 +197,87 @@ test_expect_success 'atomic push is not advertised if configured' ' test_refs master HEAD@{1} ' +# References in upstream : master(1) one(1) foo(1) +# References in workbench: master(2) foo(1) two(2) bar(2) +# Atomic push : master(2) two(2) bar(2) +test_expect_success 'atomic push reports (reject by update hook)' ' + mk_repo_pair && + ( + cd workbench && + test_commit one && + git branch foo && + git push up master one foo && + git tag -d one + ) && + ( + mkdir -p upstream/.git/hooks && + cat >upstream/.git/hooks/update <<-EOF && + #!/bin/sh + + if test "\$1" = "refs/heads/bar" + then + echo >&2 "Pusing to branch bar is prohibited" + exit 1 + fi + EOF + chmod a+x upstream/.git/hooks/update + ) && + ( + cd workbench && + test_commit two && + git branch bar + ) && + test_must_fail git -C workbench \ + push --atomic up master two bar >out 2>&1 && + fmt_status_report actual && + cat >expect <<-EOF && + To ../upstream + ! [remote rejected] master -> master (atomic push failure) + ! [remote rejected] two -> two (atomic push failure) + ! [remote rejected] bar -> bar (hook declined) + EOF + test_cmp expect actual +' + +# References in upstream : master(1) one(1) foo(1) +# References in workbench: master(2) foo(1) two(2) bar(2) +test_expect_success 'atomic push reports (mirror, but reject by update hook)' ' + ( + cd workbench && + git remote remove up && + git remote add up ../upstream + ) && + test_must_fail git -C workbench \ + push --atomic --mirror up >out 2>&1 && + fmt_status_report actual && + cat >expect <<-EOF && + To ../upstream + ! [remote rejected] master -> master (atomic push failure) + ! [remote rejected] one (atomic push failure) + ! [remote rejected] bar -> bar (hook declined) + ! [remote rejected] two -> two (atomic push failure) + EOF + test_cmp expect actual +' + +# References in upstream : master(2) one(1) foo(1) +# References in workbench: master(1) foo(1) two(2) bar(2) +test_expect_success 'atomic push reports (reject by non-ff)' ' + rm upstream/.git/hooks/update && + ( + cd workbench && + git push up master && + git reset --hard HEAD^ + ) && + test_must_fail git -C workbench \ + push --atomic up master foo bar >out 2>&1 && + fmt_status_report actual && + cat >expect <<-EOF && + To ../upstream + ! [rejected] master -> master (non-fast-forward) + ! [rejected] bar -> bar (atomic push failed) + EOF + test_cmp expect actual +' + test_done diff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh new file mode 100755 index 0000000000..1b19b3ef55 --- /dev/null +++ b/t/t5548-push-porcelain.sh @@ -0,0 +1,279 @@ +#!/bin/sh +# +# Copyright (c) 2020 Jiang Xin +# +test_description='Test git push porcelain output' + +. ./test-lib.sh + +# Create commits in and assign each commit's oid to shell variables +# given in the arguments (A, B, and C). E.g.: +# +# create_commits_in A B C +# +# NOTE: Never calling this function from a subshell since variable +# assignments will disappear when subshell exits. +create_commits_in () { + repo="$1" && + if ! parent=$(git -C "$repo" rev-parse HEAD^{} --) + then + parent= + fi && + T=$(git -C "$repo" write-tree) && + shift && + while test $# -gt 0 + do + name=$1 && + test_tick && + if test -z "$parent" + then + oid=$(echo $name | git -C "$repo" commit-tree $T) + else + oid=$(echo $name | git -C "$repo" commit-tree -p $parent $T) + fi && + eval $name=$oid && + parent=$oid && + shift || + return 1 + done && + git -C "$repo" update-ref refs/heads/master $oid +} + +# Format the output of git-push, git-show-ref and other commands to make a +# user-friendly and stable text. We can easily prepare the expect text +# without having to worry about future changes of the commit ID and spaces +# of the output. +make_user_friendly_and_stable_output () { + sed \ + -e "s/ *\$//" \ + -e "s/ */ /g" \ + -e "s/ / /g" \ + -e "s/$A//g" \ + -e "s/$B//g" \ + -e "s/$ZERO_OID//g" \ + -e "s/$(echo $A | cut -c1-7)[0-9a-f]*//g" \ + -e "s/$(echo $B | cut -c1-7)[0-9a-f]*//g" \ + -e "s#To $URL_PREFIX/upstream.git#To #" +} + +setup_upstream_and_workbench () { + # Upstream after setup : master(B) foo(A) bar(A) baz(A) + # Workbench after setup : master(A) + test_expect_success "setup upstream repository and workbench" ' + rm -rf upstream.git workbench && + git init --bare upstream.git && + git init workbench && + create_commits_in workbench A B && + ( + cd workbench && + # Try to make a stable fixed width for abbreviated commit ID, + # this fixed-width oid will be replaced with "". + git config core.abbrev 7 && + git remote add origin ../upstream.git && + git update-ref refs/heads/master $A && + git push origin \ + $B:refs/heads/master \ + $A:refs/heads/foo \ + $A:refs/heads/bar \ + $A:refs/heads/baz + ) && + git -C "workbench" config advice.pushUpdateRejected false && + upstream=upstream.git + ' +} + +run_git_push_porcelain_output_test() { + case $1 in + http) + PROTOCOL="HTTP protocol" + URL_PREFIX="http://.*" + ;; + file) + PROTOCOL="builtin protocol" + URL_PREFIX="\.\." + ;; + esac + + # Refs of upstream : master(B) foo(A) bar(A) baz(A) + # Refs of workbench: master(A) baz(A) next(A) + # git-push : master(A) NULL (B) baz(A) next(A) + test_expect_success "porcelain output of successful git-push ($PROTOCOL)" ' + ( + cd workbench && + git update-ref refs/heads/master $A && + git update-ref refs/heads/baz $A && + git update-ref refs/heads/next $A && + git push --porcelain --force origin \ + master \ + :refs/heads/foo \ + $B:bar \ + baz \ + next + ) >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + To + = refs/heads/baz:refs/heads/baz [up to date] + :refs/heads/bar .. + - :refs/heads/foo [deleted] + + refs/heads/master:refs/heads/master ... (forced update) + * refs/heads/next:refs/heads/next [new branch] + Done + EOF + test_cmp expect actual && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/bar + refs/heads/baz + refs/heads/master + refs/heads/next + EOF + test_cmp expect actual + ' + + # Refs of upstream : master(A) bar(B) baz(A) next(A) + # Refs of workbench: master(B) bar(A) baz(A) next(A) + # git-push : master(B) bar(A) NULL next(A) + test_expect_success "atomic push failed ($PROTOCOL)" ' + ( + cd workbench && + git update-ref refs/heads/master $B && + git update-ref refs/heads/bar $A && + test_must_fail git push --atomic --porcelain origin \ + master \ + bar \ + :baz \ + next + ) >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + To + = refs/heads/next:refs/heads/next [up to date] + ! refs/heads/bar:refs/heads/bar [rejected] (non-fast-forward) + ! (delete):refs/heads/baz [rejected] (atomic push failed) + ! refs/heads/master:refs/heads/master [rejected] (atomic push failed) + Done + EOF + test_cmp expect actual && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/bar + refs/heads/baz + refs/heads/master + refs/heads/next + EOF + test_cmp expect actual + ' + test_expect_success "prepare pre-receive hook ($PROTOCOL)" ' + write_script "$upstream/hooks/pre-receive" <<-EOF + exit 1 + EOF + ' + + # Refs of upstream : master(A) bar(B) baz(A) next(A) + # Refs of workbench: master(B) bar(A) baz(A) next(A) + # git-push : master(B) bar(A) NULL next(A) + test_expect_success "pre-receive hook declined ($PROTOCOL)" ' + ( + cd workbench && + git update-ref refs/heads/master $B && + git update-ref refs/heads/bar $A && + test_must_fail git push --porcelain --force origin \ + master \ + bar \ + :baz \ + next + ) >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + To + = refs/heads/next:refs/heads/next [up to date] + ! refs/heads/bar:refs/heads/bar [remote rejected] (pre-receive hook declined) + ! :refs/heads/baz [remote rejected] (pre-receive hook declined) + ! refs/heads/master:refs/heads/master [remote rejected] (pre-receive hook declined) + Done + EOF + test_cmp expect actual && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/bar + refs/heads/baz + refs/heads/master + refs/heads/next + EOF + test_cmp expect actual + ' + + test_expect_success "remove pre-receive hook ($PROTOCOL)" ' + rm "$upstream/hooks/pre-receive" + ' + + # Refs of upstream : master(A) bar(B) baz(A) next(A) + # Refs of workbench: master(B) bar(A) baz(A) next(A) + # git-push : master(B) bar(A) NULL next(A) + test_expect_success "non-fastforward push ($PROTOCOL)" ' + ( + cd workbench && + test_must_fail git push --porcelain origin \ + master \ + bar \ + :baz \ + next + ) >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + To + = refs/heads/next:refs/heads/next [up to date] + - :refs/heads/baz [deleted] + refs/heads/master:refs/heads/master .. + ! refs/heads/bar:refs/heads/bar [rejected] (non-fast-forward) + Done + EOF + test_cmp expect actual && + + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output actual && + cat >expect <<-EOF && + refs/heads/bar + refs/heads/master + refs/heads/next + EOF + test_cmp expect actual + ' +} + +# Initialize the upstream repository and local workbench. +setup_upstream_and_workbench + +# Run git-push porcelain test on builtin protocol +run_git_push_porcelain_output_test file + +ROOT_PATH="$PWD" +. "$TEST_DIRECTORY"/lib-gpg.sh +. "$TEST_DIRECTORY"/lib-httpd.sh +. "$TEST_DIRECTORY"/lib-terminal.sh +start_httpd + +# Re-initialize the upstream repository and local workbench. +setup_upstream_and_workbench + +test_expect_success "setup for http" ' + git -C upstream.git config http.receivepack true && + upstream="$HTTPD_DOCUMENT_ROOT_PATH/upstream.git" && + mv upstream.git "$upstream" && + + git -C workbench remote set-url origin $HTTPD_URL/smart/upstream.git +' + +setup_askpass_helper + +# Run git-push porcelain test on HTTP protocol +run_git_push_porcelain_output_test http + +test_done diff --git a/transport-helper.c b/transport-helper.c index 20a7185ec4..a46afcb69d 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -894,6 +894,7 @@ static int push_refs_with_push(struct transport *transport, case REF_STATUS_REJECT_STALE: case REF_STATUS_REJECT_ALREADY_EXISTS: if (atomic) { + reject_atomic_push(remote_refs, mirror); string_list_clear(&cas_options, 0); return 0; } else @@ -1488,3 +1489,25 @@ int bidirectional_transfer_loop(int input, int output) return tloop_spawnwait_tasks(&state); } + +void reject_atomic_push(struct ref *remote_refs, int mirror_mode) +{ + struct ref *ref; + + /* Mark other refs as failed */ + for (ref = remote_refs; ref; ref = ref->next) { + if (!ref->peer_ref && !mirror_mode) + continue; + + switch (ref->status) { + case REF_STATUS_NONE: + case REF_STATUS_OK: + case REF_STATUS_EXPECTING_REPORT: + ref->status = REF_STATUS_ATOMIC_PUSH_FAILED; + continue; + default: + break; /* do nothing */ + } + } + return; +} diff --git a/transport.c b/transport.c index 471c5bd339..15f5ba4e8f 100644 --- a/transport.c +++ b/transport.c @@ -715,7 +715,15 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re close(data->fd[1]); close(data->fd[0]); - ret |= finish_connect(data->conn); + /* + * Atomic push may abort the connection early and close the pipe, + * which may cause an error for `finish_connect()`. Ignore this error + * for atomic git-push. + */ + if (ret || args.atomic) + finish_connect(data->conn); + else + ret = finish_connect(data->conn); data->conn = NULL; data->got_remote_heads = 0; @@ -1240,20 +1248,6 @@ int transport_push(struct repository *r, err = push_had_errors(remote_refs); ret = push_ret | err; - if ((flags & TRANSPORT_PUSH_ATOMIC) && err) { - struct ref *it; - for (it = remote_refs; it; it = it->next) - switch (it->status) { - case REF_STATUS_NONE: - case REF_STATUS_UPTODATE: - case REF_STATUS_OK: - it->status = REF_STATUS_ATOMIC_PUSH_FAILED; - break; - default: - break; - } - } - if (!quiet || err) transport_print_push_status(transport->url, remote_refs, verbose | porcelain, porcelain, diff --git a/transport.h b/transport.h index e0131daab9..4298c855be 100644 --- a/transport.h +++ b/transport.h @@ -265,4 +265,7 @@ int transport_refs_pushed(struct ref *ref); void transport_print_push_status(const char *dest, struct ref *refs, int verbose, int porcelain, unsigned int *reject_reasons); +/* common method used by transport-helper.c and send-pack.c */ +void reject_atomic_push(struct ref *refs, int mirror_mode); + #endif