transport-helper: enforce atomic in push_refs_with_push
Teach transport-helper how to notice if skipping a ref during push would violate atomicity on the client side. We notice that a ref would be rejected, and choose not to send it, but don't notice that if the client has asked for --atomic we are violating atomicity if all the other pushes we are sending would succeed. Asking the server end to uphold atomicity wouldn't work here as the server doesn't have any idea that we tried to update a ref that's broken. The added test-case is a succinct way to reproduce this issue that fails today. The same steps work fine when we aren't using a transport-helper to get to the upstream, i.e. when we've added a local repository as a remote: git remote add ~/upstream upstream Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emily Shaffer committed
Jul 11, 2019 at 14:19 UTC
3bca1e7f9f9708b970035a641d21f8c5cec1cd88
3 files changed
+68
t/t5541-http-push-smart.sh
+49
@@ -177,6 +177,55 @@ test_expect_success 'push (chunked)' '
177
test $HEAD = $(git rev-parse --verify HEAD))
178
'
179
180
+test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
181
+ # Setup upstream repo - empty for now
182
+ d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
183
+ git init --bare "$d" &&
184
+ test_config -C "$d" http.receivepack true &&
185
+ up="$HTTPD_URL"/smart/atomic-branches.git &&
186
+
187
+ # Tell "$up" about two branches for now
188
+ test_commit atomic1 &&
189
+ test_commit atomic2 &&
190
+ git branch collateral &&
191
+ git push "$up" master collateral &&
192
+
193
+ # collateral is a valid push, but should be failed by atomic push
194
+ git checkout collateral &&
195
+ test_commit collateral1 &&
196
+
197
+ # Make master incompatible with upstream to provoke atomic
198
+ git checkout master &&
199
+ git reset --hard HEAD^ &&
200
+
201
+ # Add a new branch which should be failed by atomic push. This is a
202
+ # regression case.
203
+ git branch atomic &&
204
+
205
+ # --atomic should cause entire push to be rejected
206
+ test_must_fail git push --atomic "$up" master atomic collateral 2>output &&
207
+
208
+ # the new branch should not have been created upstream
209
+ test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
210
+
211
+ # upstream should still reflect atomic2, the last thing we pushed
212
+ # successfully
213
+ git rev-parse atomic2 >expected &&
214
+ # on master...
215
+ git -C "$d" rev-parse refs/heads/master >actual &&
216
+ test_cmp expected actual &&
217
+ # ...and collateral.
218
+ git -C "$d" rev-parse refs/heads/collateral >actual &&
219
+ test_cmp expected actual &&
220
+
221
+ # the failed refs should be indicated to the user
222
+ grep "^ ! .*rejected.* master -> master" output &&
223
+
224
+ # the collateral failure refs should be indicated to the user
225
+ grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output &&
226
+ grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
227
+'
228
+
229
test_expect_success 'push --all can push to empty repo' '
230
d=$HTTPD_DOCUMENT_ROOT_PATH/empty-all.git &&
231
git init --bare "$d" &&
transport-helper.c
+6
@@ -853,6 +853,7 @@ static int push_refs_with_push(struct transport *transport,
853
{
854
int force_all = flags & TRANSPORT_PUSH_FORCE;
855
int mirror = flags & TRANSPORT_PUSH_MIRROR;
856
+ int atomic = flags & TRANSPORT_PUSH_ATOMIC;
857
struct helper_data *data = transport->data;
858
struct strbuf buf = STRBUF_INIT;
859
struct ref *ref;
@@ -872,6 +873,11 @@ static int push_refs_with_push(struct transport *transport,
873
case REF_STATUS_REJECT_NONFASTFORWARD:
874
case REF_STATUS_REJECT_STALE:
875
case REF_STATUS_REJECT_ALREADY_EXISTS:
876
+ if (atomic) {
877
+ string_list_clear(&cas_options, 0);
878
+ return 0;
879
+ } else
880
+ continue;
881
case REF_STATUS_UPTODATE:
882
continue;
883
default:
transport.c
+13
@@ -1226,6 +1226,19 @@ int transport_push(struct repository *r,
1226
err = push_had_errors(remote_refs);
1227
ret = push_ret | err;
1228
1229
+ if ((flags & TRANSPORT_PUSH_ATOMIC) && err) {
1230
+ for (struct ref *it = remote_refs; it; it = it->next)
1231
+ switch (it->status) {
1232
+ case REF_STATUS_NONE:
1233
+ case REF_STATUS_UPTODATE:
1234
+ case REF_STATUS_OK:
1235
+ it->status = REF_STATUS_ATOMIC_PUSH_FAILED;
1236
+ break;
1237
+ default:
1238
+ break;
1239
+ }
1240
+ }
1241
+
1242
if (!quiet || err)
1243
transport_print_push_status(transport->url, remote_refs,
1244
verbose | porcelain, porcelain,