From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
Find a new issue on porcelain output for git-push, add a new patch 1/5
to fix this issue. This fix overlaps with and the topic `jx/atomic-push`,
so merge them together.
I find this issue when adding test cases of porcelain output for topic
`jx/proc-receive-hook`, which depends on this fix.
## Details of this new issue
The porcelain output of a failed `git-push` command is inconsistent for
different protocols. For example, the following `git-push` command
failed because of the failure of `pre-receive` hook.
git push --porcelain origin HEAD:refs/heads/master
For SSH protocol, the porcelain output does not end with a "Done"
message:
To <URL/of/upstream.git>
! HEAD:refs/heads/master [remote rejected] (pre-receive hook declined)
While for HTTP protocol, the porcelain output does end with a "Done"
message;
To <URL/of/upstream.git>
! HEAD:refs/heads/master [remote rejected] (pre-receive hook declined)
Done
The following code at the end of function `send_pack()` indicates that
`send_pack()` should not return an error if some references are rejected
in porcelain mode.
int send_pack(...)
... ...
if (args->porcelain)
return 0;
for (ref = remote_refs; ref; ref = ref->next) {
switch (ref->status) {
case REF_STATUS_NONE:
case REF_STATUS_UPTODATE:
case REF_STATUS_OK:
break;
default:
return -1;
}
}
return 0;
}
So if atomic push failed, must check the porcelain mode before return
an error. And `receive_status()` should not return an error for a
failed updated reference, because `send_pack()` will check them instead.
## Range-diff v2...v3
-: ---------- > 1: d9ea3c35a3 send-pack: fix inconsistent porcelain output
1: 7a0579ba13 = 2: bb07d5c330 t5543: never report what we do not push
2: 9b4bca8f4c ! 3: 1aa917b097 send-pack: mark failure of atomic push properly
@@ t/t5543-atomic-push.sh: test_expect_failure 'atomic push reports (mirror, but re
(
cd workbench &&
+ ## t/t5548-push-porcelain.sh ##
+@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
+ # 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)" '
++ test_expect_failure "atomic push failed ($PROTOCOL)" '
+ (
+ cd workbench &&
+ git update-ref refs/heads/master $B &&
+
## transport.c ##
@@ transport.c: int transport_push(struct repository *r,
err = push_had_errors(remote_refs);
3: a7e8d7c893 ! 4: 2848088852 transport-helper: mark failure for atomic push
@@ t/t5541-http-push-smart.sh: test_expect_failure 'push --atomic also prevents bra
test_expect_success 'push --atomic fails on server-side errors' '
+ ## t/t5548-push-porcelain.sh ##
+@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
+ # 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_failure "atomic push failed ($PROTOCOL)" '
++ test_expect_success "atomic push failed ($PROTOCOL)" '
+ (
+ cd workbench &&
+ git update-ref refs/heads/master $B &&
+@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
+ make_user_friendly_and_stable_output <out >actual &&
+ cat >expect <<-EOF &&
+ To <URL/of/upstream.git>
++ = 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)
+- ! refs/heads/next:refs/heads/next [rejected] (atomic push failed)
+ Done
+ EOF
+ test_cmp expect actual &&
+@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
+ EOF
+ test_cmp expect actual
+ '
+-
+ test_expect_success "prepare pre-receive hook ($PROTOCOL)" '
+ write_script "$upstream/hooks/pre-receive" <<-EOF
+ exit 1
+
## transport-helper.c ##
@@ transport-helper.c: static int push_refs_with_push(struct transport *transport,
case REF_STATUS_REJECT_STALE:
4: 94b13f5dcd ! 5: d2f0b50395 transport-helper: new method reject_atomic_push()
@@ send-pack.c: 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);
+- atomic_push_failure(args, remote_refs, ref);
+ reject_atomic_push(remote_refs, args->send_mirror);
-+ return error("atomic push failed for ref %s. status: %d\n",
-+ ref->name, ref->status);
++ error("atomic push failed for ref %s. status: %d\n",
++ ref->name, ref->status);
+ return args->porcelain ? 0 : -1;
}
/* else fallthrough */
- default:
## transport-helper.c ##
@@ transport-helper.c: static int push_refs_with_push(struct transport *transport,
---
Jiang Xin (5):
send-pack: fix inconsistent porcelain output
t5543: never report what we do not push
send-pack: mark failure of atomic push properly
transport-helper: mark failure for atomic push
transport-helper: new method reject_atomic_push()
send-pack.c | 32 +---
t/t5541-http-push-smart.sh | 12 +-
t/t5543-atomic-push.sh | 89 +++++++++++
t/t5548-push-porcelain.sh | 300 +++++++++++++++++++++++++++++++++++++
transport-helper.c | 23 +++
transport.c | 24 ++-
transport.h | 3 +
7 files changed, 439 insertions(+), 44 deletions(-)
create mode 100755 t/t5548-push-porcelain.sh
--
2.24.1.15.g448c31058d.agit.4.5
