send-pack: fix inconsistent porcelain output
The porcelain output of a failed `git-push` command is inconsistent for different protocols. For example, the following `git-push` command may fail due to the failure of the `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. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiang Xin committed
Apr 17, 2020 at 05:45 UTC
7dcbeaa0df01661f31a0d93e5d5787a5d3767358
5 files changed
+294
-5
send-pack.c
+3
-4
@@ -190,10 +190,8 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
190
191
if (reader->line[0] == 'o' && reader->line[1] == 'k')
192
hint->status = REF_STATUS_OK;
193
- else {
193
+ else
194
hint->status = REF_STATUS_REMOTE_REJECT;
195
- ret = -1;
196
- }
195
hint->remote_status = xstrdup_or_null(msg);
196
/* start our next search from the next ref */
197
hint = hint->next;
@@ -489,7 +487,8 @@ int send_pack(struct send_pack_args *args,
487
if (use_atomic) {
488
strbuf_release(&req_buf);
489
strbuf_release(&cap_buf);
492
- return atomic_push_failure(args, remote_refs, ref);
490
+ atomic_push_failure(args, remote_refs, ref);
491
+ return args->porcelain ? 0 : -1;
492
}
493
/* else fallthrough */
494
default:
t/t5504-fetch-receive-strict.sh
+1
@@ -65,6 +65,7 @@ test_expect_success 'fetch with transfer.fsckobjects' '
65
cat >exp <<EOF
66
To dst
67
! refs/heads/master:refs/heads/test [remote rejected] (missing necessary objects)
68
+Done
69
EOF
70
71
test_expect_success 'push without strict' '
t/t5516-fetch-push.sh
+1
@@ -1066,6 +1066,7 @@ test_expect_success 'push --porcelain rejected' '
1066
1067
echo >.git/foo "To testrepo" &&
1068
echo >>.git/foo "! refs/heads/master:refs/heads/master [remote rejected] (branch is currently checked out)" &&
1069
+ echo >>.git/foo "Done" &&
1070
1071
test_must_fail git push >.git/bar --porcelain testrepo refs/heads/master:refs/heads/master &&
1072
test_cmp .git/foo .git/bar
t/t5548-push-porcelain.sh
new
+280
@@ -0,0 +1,280 @@
1
+#!/bin/sh
2
+#
3
+# Copyright (c) 2020 Jiang Xin
4
+#
5
+test_description='Test git push porcelain output'
6
+
7
+. ./test-lib.sh
8
+
9
+# Create commits in <repo> and assign each commit's oid to shell variables
10
+# given in the arguments (A, B, and C). E.g.:
11
+#
12
+# create_commits_in <repo> A B C
13
+#
14
+# NOTE: Never calling this function from a subshell since variable
15
+# assignments will disappear when subshell exits.
16
+create_commits_in () {
17
+ repo="$1" &&
18
+ if ! parent=$(git -C "$repo" rev-parse HEAD^{} --)
19
+ then
20
+ parent=
21
+ fi &&
22
+ T=$(git -C "$repo" write-tree) &&
23
+ shift &&
24
+ while test $# -gt 0
25
+ do
26
+ name=$1 &&
27
+ test_tick &&
28
+ if test -z "$parent"
29
+ then
30
+ oid=$(echo $name | git -C "$repo" commit-tree $T)
31
+ else
32
+ oid=$(echo $name | git -C "$repo" commit-tree -p $parent $T)
33
+ fi &&
34
+ eval $name=$oid &&
35
+ parent=$oid &&
36
+ shift ||
37
+ return 1
38
+ done &&
39
+ git -C "$repo" update-ref refs/heads/master $oid
40
+}
41
+
42
+# Format the output of git-push, git-show-ref and other commands to make a
43
+# user-friendly and stable text. We can easily prepare the expect text
44
+# without having to worry about future changes of the commit ID and spaces
45
+# of the output.
46
+make_user_friendly_and_stable_output () {
47
+ sed \
48
+ -e "s/ *\$//" \
49
+ -e "s/ */ /g" \
50
+ -e "s/ / /g" \
51
+ -e "s/$A/<COMMIT-A>/g" \
52
+ -e "s/$B/<COMMIT-B>/g" \
53
+ -e "s/$ZERO_OID/<ZERO-OID>/g" \
54
+ -e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<OID-A>/g" \
55
+ -e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g" \
56
+ -e "s#To $URL_PREFIX/upstream.git#To <URL/of/upstream.git>#"
57
+}
58
+
59
+setup_upstream_and_workbench () {
60
+ # Upstream after setup : master(B) foo(A) bar(A) baz(A)
61
+ # Workbench after setup : master(A)
62
+ test_expect_success "setup upstream repository and workbench" '
63
+ rm -rf upstream.git workbench &&
64
+ git init --bare upstream.git &&
65
+ git init workbench &&
66
+ create_commits_in workbench A B &&
67
+ (
68
+ cd workbench &&
69
+ # Try to make a stable fixed width for abbreviated commit ID,
70
+ # this fixed-width oid will be replaced with "<OID>".
71
+ git config core.abbrev 7 &&
72
+ git remote add origin ../upstream.git &&
73
+ git update-ref refs/heads/master $A &&
74
+ git push origin \
75
+ $B:refs/heads/master \
76
+ $A:refs/heads/foo \
77
+ $A:refs/heads/bar \
78
+ $A:refs/heads/baz
79
+ ) &&
80
+ git -C "workbench" config advice.pushUpdateRejected false &&
81
+ upstream=upstream.git
82
+ '
83
+}
84
+
85
+run_git_push_porcelain_output_test() {
86
+ case $1 in
87
+ http)
88
+ PROTOCOL="HTTP protocol"
89
+ URL_PREFIX="http://.*"
90
+ ;;
91
+ file)
92
+ PROTOCOL="builtin protocol"
93
+ URL_PREFIX="\.\."
94
+ ;;
95
+ esac
96
+
97
+ # Refs of upstream : master(B) foo(A) bar(A) baz(A)
98
+ # Refs of workbench: master(A) baz(A) next(A)
99
+ # git-push : master(A) NULL (B) baz(A) next(A)
100
+ test_expect_success "porcelain output of successful git-push ($PROTOCOL)" '
101
+ (
102
+ cd workbench &&
103
+ git update-ref refs/heads/master $A &&
104
+ git update-ref refs/heads/baz $A &&
105
+ git update-ref refs/heads/next $A &&
106
+ git push --porcelain --force origin \
107
+ master \
108
+ :refs/heads/foo \
109
+ $B:bar \
110
+ baz \
111
+ next
112
+ ) >out &&
113
+ make_user_friendly_and_stable_output <out >actual &&
114
+ cat >expect <<-EOF &&
115
+ To <URL/of/upstream.git>
116
+ = refs/heads/baz:refs/heads/baz [up to date]
117
+ <COMMIT-B>:refs/heads/bar <OID-A>..<OID-B>
118
+ - :refs/heads/foo [deleted]
119
+ + refs/heads/master:refs/heads/master <OID-B>...<OID-A> (forced update)
120
+ * refs/heads/next:refs/heads/next [new branch]
121
+ Done
122
+ EOF
123
+ test_cmp expect actual &&
124
+
125
+ git -C "$upstream" show-ref >out &&
126
+ make_user_friendly_and_stable_output <out >actual &&
127
+ cat >expect <<-EOF &&
128
+ <COMMIT-B> refs/heads/bar
129
+ <COMMIT-A> refs/heads/baz
130
+ <COMMIT-A> refs/heads/master
131
+ <COMMIT-A> refs/heads/next
132
+ EOF
133
+ test_cmp expect actual
134
+ '
135
+
136
+ # Refs of upstream : master(A) bar(B) baz(A) next(A)
137
+ # Refs of workbench: master(B) bar(A) baz(A) next(A)
138
+ # git-push : master(B) bar(A) NULL next(A)
139
+ test_expect_success "atomic push failed ($PROTOCOL)" '
140
+ (
141
+ cd workbench &&
142
+ git update-ref refs/heads/master $B &&
143
+ git update-ref refs/heads/bar $A &&
144
+ test_must_fail git push --atomic --porcelain origin \
145
+ master \
146
+ bar \
147
+ :baz \
148
+ next
149
+ ) >out &&
150
+ make_user_friendly_and_stable_output <out >actual &&
151
+ cat >expect <<-EOF &&
152
+ To <URL/of/upstream.git>
153
+ ! refs/heads/bar:refs/heads/bar [rejected] (non-fast-forward)
154
+ ! (delete):refs/heads/baz [rejected] (atomic push failed)
155
+ ! refs/heads/master:refs/heads/master [rejected] (atomic push failed)
156
+ ! refs/heads/next:refs/heads/next [rejected] (atomic push failed)
157
+ Done
158
+ EOF
159
+ test_cmp expect actual &&
160
+
161
+ git -C "$upstream" show-ref >out &&
162
+ make_user_friendly_and_stable_output <out >actual &&
163
+ cat >expect <<-EOF &&
164
+ <COMMIT-B> refs/heads/bar
165
+ <COMMIT-A> refs/heads/baz
166
+ <COMMIT-A> refs/heads/master
167
+ <COMMIT-A> refs/heads/next
168
+ EOF
169
+ test_cmp expect actual
170
+ '
171
+
172
+ test_expect_success "prepare pre-receive hook ($PROTOCOL)" '
173
+ write_script "$upstream/hooks/pre-receive" <<-EOF
174
+ exit 1
175
+ EOF
176
+ '
177
+
178
+ # Refs of upstream : master(A) bar(B) baz(A) next(A)
179
+ # Refs of workbench: master(B) bar(A) baz(A) next(A)
180
+ # git-push : master(B) bar(A) NULL next(A)
181
+ test_expect_success "pre-receive hook declined ($PROTOCOL)" '
182
+ (
183
+ cd workbench &&
184
+ git update-ref refs/heads/master $B &&
185
+ git update-ref refs/heads/bar $A &&
186
+ test_must_fail git push --porcelain --force origin \
187
+ master \
188
+ bar \
189
+ :baz \
190
+ next
191
+ ) >out &&
192
+ make_user_friendly_and_stable_output <out >actual &&
193
+ cat >expect <<-EOF &&
194
+ To <URL/of/upstream.git>
195
+ = refs/heads/next:refs/heads/next [up to date]
196
+ ! refs/heads/bar:refs/heads/bar [remote rejected] (pre-receive hook declined)
197
+ ! :refs/heads/baz [remote rejected] (pre-receive hook declined)
198
+ ! refs/heads/master:refs/heads/master [remote rejected] (pre-receive hook declined)
199
+ Done
200
+ EOF
201
+ test_cmp expect actual &&
202
+
203
+ git -C "$upstream" show-ref >out &&
204
+ make_user_friendly_and_stable_output <out >actual &&
205
+ cat >expect <<-EOF &&
206
+ <COMMIT-B> refs/heads/bar
207
+ <COMMIT-A> refs/heads/baz
208
+ <COMMIT-A> refs/heads/master
209
+ <COMMIT-A> refs/heads/next
210
+ EOF
211
+ test_cmp expect actual
212
+ '
213
+
214
+ test_expect_success "remove pre-receive hook ($PROTOCOL)" '
215
+ rm "$upstream/hooks/pre-receive"
216
+ '
217
+
218
+ # Refs of upstream : master(A) bar(B) baz(A) next(A)
219
+ # Refs of workbench: master(B) bar(A) baz(A) next(A)
220
+ # git-push : master(B) bar(A) NULL next(A)
221
+ test_expect_success "non-fastforward push ($PROTOCOL)" '
222
+ (
223
+ cd workbench &&
224
+ test_must_fail git push --porcelain origin \
225
+ master \
226
+ bar \
227
+ :baz \
228
+ next
229
+ ) >out &&
230
+ make_user_friendly_and_stable_output <out >actual &&
231
+ cat >expect <<-EOF &&
232
+ To <URL/of/upstream.git>
233
+ = refs/heads/next:refs/heads/next [up to date]
234
+ - :refs/heads/baz [deleted]
235
+ refs/heads/master:refs/heads/master <OID-A>..<OID-B>
236
+ ! refs/heads/bar:refs/heads/bar [rejected] (non-fast-forward)
237
+ Done
238
+ EOF
239
+ test_cmp expect actual &&
240
+
241
+ git -C "$upstream" show-ref >out &&
242
+ make_user_friendly_and_stable_output <out >actual &&
243
+ cat >expect <<-EOF &&
244
+ <COMMIT-B> refs/heads/bar
245
+ <COMMIT-B> refs/heads/master
246
+ <COMMIT-A> refs/heads/next
247
+ EOF
248
+ test_cmp expect actual
249
+ '
250
+}
251
+
252
+# Initialize the upstream repository and local workbench.
253
+setup_upstream_and_workbench
254
+
255
+# Run git-push porcelain test on builtin protocol
256
+run_git_push_porcelain_output_test file
257
+
258
+ROOT_PATH="$PWD"
259
+. "$TEST_DIRECTORY"/lib-gpg.sh
260
+. "$TEST_DIRECTORY"/lib-httpd.sh
261
+. "$TEST_DIRECTORY"/lib-terminal.sh
262
+start_httpd
263
+
264
+# Re-initialize the upstream repository and local workbench.
265
+setup_upstream_and_workbench
266
+
267
+test_expect_success "setup for http" '
268
+ git -C upstream.git config http.receivepack true &&
269
+ upstream="$HTTPD_DOCUMENT_ROOT_PATH/upstream.git" &&
270
+ mv upstream.git "$upstream" &&
271
+
272
+ git -C workbench remote set-url origin $HTTPD_URL/smart/upstream.git
273
+'
274
+
275
+setup_askpass_helper
276
+
277
+# Run git-push porcelain test on HTTP protocol
278
+run_git_push_porcelain_output_test http
279
+
280
+test_done
transport.c
+9
-1
@@ -715,7 +715,15 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
715
716
close(data->fd[1]);
717
close(data->fd[0]);
718
- ret |= finish_connect(data->conn);
718
+ /*
719
+ * Atomic push may abort the connection early and close the pipe,
720
+ * which may cause an error for `finish_connect()`. Ignore this error
721
+ * for atomic git-push.
722
+ */
723
+ if (ret || args.atomic)
724
+ finish_connect(data->conn);
725
+ else
726
+ ret = finish_connect(data->conn);
727
data->conn = NULL;
728
data->got_remote_heads = 0;
729