Skip to content

Commit 066f6cd

Browse files
committed
Merge branch 'jt/push-negotiation-fixes'
Bugfix for common ancestor negotiation recently introduced in "git push" code path. * jt/push-negotiation-fixes: fetch: die on invalid --negotiation-tip hash send-pack: fix push nego. when remote has refs send-pack: fix push.negotiate with remote helper
2 parents 6f64eea + 8282311 commit 066f6cd

File tree

6 files changed

+96
-4
lines changed

6 files changed

+96
-4
lines changed

builtin/fetch.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,9 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
14281428
if (!has_glob_specials(s)) {
14291429
struct object_id oid;
14301430
if (get_oid(s, &oid))
1431-
die("%s is not a valid object", s);
1431+
die(_("%s is not a valid object"), s);
1432+
if (!has_object(the_repository, &oid, 0))
1433+
die(_("the object %s does not exist"), s);
14321434
oid_array_append(oids, &oid);
14331435
continue;
14341436
}

builtin/send-pack.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
230230
args.atomic = atomic;
231231
args.stateless_rpc = stateless_rpc;
232232
args.push_options = push_options.nr ? &push_options : NULL;
233+
args.url = dest;
233234

234235
if (from_stdin) {
235236
if (args.stateless_rpc) {

send-pack.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,8 +425,10 @@ static void get_commons_through_negotiation(const char *url,
425425
child.no_stdin = 1;
426426
child.out = -1;
427427
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
428-
for (ref = remote_refs; ref; ref = ref->next)
429-
strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
428+
for (ref = remote_refs; ref; ref = ref->next) {
429+
if (!is_null_oid(&ref->new_oid))
430+
strvec_pushf(&child.args, "--negotiation-tip=%s", oid_to_hex(&ref->new_oid));
431+
}
430432
strvec_push(&child.args, url);
431433

432434
if (start_command(&child))

t/t5510-fetch.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,19 @@ test_expect_success '--negotiation-tip understands abbreviated SHA-1' '
12141214
check_negotiation_tip
12151215
'
12161216

1217+
test_expect_success '--negotiation-tip rejects missing OIDs' '
1218+
setup_negotiation_tip server server 0 &&
1219+
test_must_fail git -C client fetch \
1220+
--negotiation-tip=alpha_1 \
1221+
--negotiation-tip=$(test_oid zero) \
1222+
origin alpha_s beta_s 2>err &&
1223+
cat >fatal-expect <<-EOF &&
1224+
fatal: the object $(test_oid zero) does not exist
1225+
EOF
1226+
grep fatal: err >fatal-actual &&
1227+
test_cmp fatal-expect fatal-actual
1228+
'
1229+
12171230
. "$TEST_DIRECTORY"/lib-httpd.sh
12181231
start_httpd
12191232

t/t5516-fetch-push.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ test_expect_success 'push with negotiation' '
201201
# Without negotiation
202202
mk_empty testrepo &&
203203
git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
204+
test_commit -C testrepo unrelated_commit &&
204205
git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
205206
echo now pushing without negotiation &&
206207
GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 push testrepo refs/heads/main:refs/remotes/origin/main &&
@@ -210,6 +211,7 @@ test_expect_success 'push with negotiation' '
210211
rm event &&
211212
mk_empty testrepo &&
212213
git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
214+
test_commit -C testrepo unrelated_commit &&
213215
git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
214216
GIT_TRACE2_EVENT="$(pwd)/event" git -c protocol.version=2 -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main &&
215217
grep_wrote 2 event # 1 commit, 1 tree
@@ -219,6 +221,7 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f
219221
rm event &&
220222
mk_empty testrepo &&
221223
git push testrepo $the_first_commit:refs/remotes/origin/first_commit &&
224+
test_commit -C testrepo unrelated_commit &&
222225
git -C testrepo config receive.hideRefs refs/remotes/origin/first_commit &&
223226
GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" \
224227
git -c push.negotiate=1 push testrepo refs/heads/main:refs/remotes/origin/main 2>err &&
@@ -1767,5 +1770,4 @@ test_expect_success 'denyCurrentBranch and worktrees' '
17671770
git -C cloned push origin HEAD:new-wt &&
17681771
test_must_fail git -C cloned push --delete origin new-wt
17691772
'
1770-
17711773
test_done

t/t5549-fetch-push-http.sh

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#!/bin/sh
2+
3+
test_description='fetch/push functionality using the HTTP protocol'
4+
5+
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6+
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7+
8+
. ./test-lib.sh
9+
. "$TEST_DIRECTORY"/lib-httpd.sh
10+
start_httpd
11+
12+
SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server"
13+
URI="$HTTPD_URL/smart/server"
14+
15+
grep_wrote () {
16+
object_count=$1
17+
file_name=$2
18+
grep 'write_pack_file/wrote.*"value":"'$1'"' $2
19+
}
20+
21+
setup_client_and_server () {
22+
git init client &&
23+
test_when_finished 'rm -rf client' &&
24+
test_commit -C client first_commit &&
25+
test_commit -C client second_commit &&
26+
27+
git init "$SERVER" &&
28+
test_when_finished 'rm -rf "$SERVER"' &&
29+
test_config -C "$SERVER" http.receivepack true &&
30+
test_commit -C "$SERVER" unrelated_commit &&
31+
git -C client push "$URI" first_commit:refs/remotes/origin/first_commit &&
32+
git -C "$SERVER" config receive.hideRefs refs/remotes/origin/first_commit
33+
}
34+
35+
test_expect_success 'push without negotiation (for comparing object counts with the next test)' '
36+
setup_client_and_server &&
37+
38+
GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 \
39+
push "$URI" refs/heads/main:refs/remotes/origin/main &&
40+
test_when_finished "rm -f event" &&
41+
grep_wrote 6 event # 2 commits, 2 trees, 2 blobs
42+
'
43+
44+
test_expect_success 'push with negotiation' '
45+
setup_client_and_server &&
46+
47+
GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c protocol.version=2 -c push.negotiate=1 \
48+
push "$URI" refs/heads/main:refs/remotes/origin/main &&
49+
test_when_finished "rm -f event" &&
50+
grep_wrote 3 event # 1 commit, 1 tree, 1 blob
51+
'
52+
53+
test_expect_success 'push with negotiation proceeds anyway even if negotiation fails' '
54+
setup_client_and_server &&
55+
56+
# Use protocol v0 to make negotiation fail (because protocol v0 does
57+
# not support the "wait-for-done" capability, which is required for
58+
# push negotiation)
59+
GIT_TEST_PROTOCOL_VERSION=0 GIT_TRACE2_EVENT="$(pwd)/event" git -C client -c push.negotiate=1 \
60+
push "$URI" refs/heads/main:refs/remotes/origin/main 2>err &&
61+
test_when_finished "rm -f event" &&
62+
grep_wrote 6 event && # 2 commits, 2 trees, 2 blobs
63+
64+
cat >warning-expect <<-EOF &&
65+
warning: --negotiate-only requires protocol v2
66+
warning: push negotiation failed; proceeding anyway with push
67+
EOF
68+
grep warning: err >warning-actual &&
69+
test_cmp warning-expect warning-actual
70+
'
71+
72+
test_done

0 commit comments

Comments
 (0)