Skip to content

Commit 88df0fa

Browse files
gitstergitster
committed
Merge branch 'jt/connectivity-check-after-unshallow'
"git fetch" failed to correctly validate the set of objects it received when making a shallow history deeper, which has been corrected. * jt/connectivity-check-after-unshallow: fetch-pack: write shallow, then check connectivity fetch-pack: implement ref-in-want fetch-pack: put shallow info in output parameter fetch: refactor to make function args narrower fetch: refactor fetch_refs into two functions fetch: refactor the population of peer ref OIDs upload-pack: test negotiation with changing repository upload-pack: implement ref-in-want test-pkt-line: add unpack-sideband subcommand
2 parents 4301330 + cf1e7c0 commit 88df0fa

22 files changed

+837
-82
lines changed

‎Documentation/config.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3489,6 +3489,13 @@ Note that this configuration variable is ignored if it is seen in the
34893489
repository-level config (this is a safety measure against fetching from
34903490
untrusted repositories).
34913491

3492+
uploadpack.allowRefInWant::
3493+
If this option is set, `upload-pack` will support the `ref-in-want`
3494+
feature of the protocol version 2 `fetch` command. This feature
3495+
is intended for the benefit of load-balanced servers which may
3496+
not have the same view of what OIDs their refs point to due to
3497+
replication delay.
3498+
34923499
url.<base>.insteadOf::
34933500
Any URL that starts with this value will be rewritten to
34943501
start, instead, with <base>. In cases where some site serves a

‎Documentation/technical/protocol-v2.txt‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,12 +298,21 @@ included in the client's request:
298298
for use with partial clone and partial fetch operations. See
299299
`rev-list` for possible "filter-spec" values.
300300

301+
If the 'ref-in-want' feature is advertised, the following argument can
302+
be included in the client's request as well as the potential addition of
303+
the 'wanted-refs' section in the server's response as explained below.
304+
305+
want-ref <ref>
306+
Indicates to the server that the client wants to retrieve a
307+
particular ref, where <ref> is the full name of a ref on the
308+
server.
309+
301310
The response of `fetch` is broken into a number of sections separated by
302311
delimiter packets (0001), with each section beginning with its section
303312
header.
304313

305314
output = *section
306-
section = (acknowledgments | shallow-info | packfile)
315+
section = (acknowledgments | shallow-info | wanted-refs | packfile)
307316
(flush-pkt | delim-pkt)
308317

309318
acknowledgments = PKT-LINE("acknowledgments" LF)
@@ -318,6 +327,10 @@ header.
318327
shallow = "shallow" SP obj-id
319328
unshallow = "unshallow" SP obj-id
320329

330+
wanted-refs = PKT-LINE("wanted-refs" LF)
331+
*PKT-LINE(wanted-ref LF)
332+
wanted-ref = obj-id SP refname
333+
321334
packfile = PKT-LINE("packfile" LF)
322335
*PKT-LINE(%x01-03 *%x00-ff)
323336

@@ -378,6 +391,19 @@ header.
378391
* This section is only included if a packfile section is also
379392
included in the response.
380393

394+
wanted-refs section
395+
* This section is only included if the client has requested a
396+
ref using a 'want-ref' line and if a packfile section is also
397+
included in the response.
398+
399+
* Always begins with the section header "wanted-refs".
400+
401+
* The server will send a ref listing ("<oid> <refname>") for
402+
each reference requested using 'want-ref' lines.
403+
404+
* The server MUST NOT send any refs which were not requested
405+
using 'want-ref' lines.
406+
381407
packfile section
382408
* This section is only included if the client has sent 'want'
383409
lines in its request and either requested that no more

‎builtin/clone.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
11561156
}
11571157

11581158
if (!is_local && !complete_refs_before_fetch)
1159-
transport_fetch_refs(transport, mapped_refs);
1159+
transport_fetch_refs(transport, mapped_refs, NULL);
11601160

11611161
remote_head = find_ref_by_name(refs, "HEAD");
11621162
remote_head_points_at =
@@ -1198,7 +1198,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
11981198
if (is_local)
11991199
clone_local(path, git_dir);
12001200
else if (refs && complete_refs_before_fetch)
1201-
transport_fetch_refs(transport, mapped_refs);
1201+
transport_fetch_refs(transport, mapped_refs, NULL);
12021202

12031203
update_remote_refs(refs, mapped_refs, remote_head_points_at,
12041204
branch_top.buf, reflog_msg.buf, transport,

‎builtin/fetch.c‎

Lines changed: 94 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -242,17 +242,17 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
242242
return 0;
243243
}
244244

245-
static void find_non_local_tags(struct transport *transport,
246-
struct ref **head,
247-
struct ref ***tail)
245+
static void find_non_local_tags(const struct ref *refs,
246+
struct ref **head,
247+
struct ref ***tail)
248248
{
249249
struct string_list existing_refs = STRING_LIST_INIT_DUP;
250250
struct string_list remote_refs = STRING_LIST_INIT_NODUP;
251251
const struct ref *ref;
252252
struct string_list_item *item = NULL;
253253

254254
for_each_ref(add_existing, &existing_refs);
255-
for (ref = transport_get_remote_refs(transport, NULL); ref; ref = ref->next) {
255+
for (ref = refs; ref; ref = ref->next) {
256256
if (!starts_with(ref->name, "refs/tags/"))
257257
continue;
258258

@@ -326,34 +326,20 @@ static void find_non_local_tags(struct transport *transport,
326326
string_list_clear(&remote_refs, 0);
327327
}
328328

329-
static struct ref *get_ref_map(struct transport *transport,
329+
static struct ref *get_ref_map(struct remote *remote,
330+
const struct ref *remote_refs,
330331
struct refspec *rs,
331332
int tags, int *autotags)
332333
{
333334
int i;
334335
struct ref *rm;
335336
struct ref *ref_map = NULL;
336337
struct ref **tail = &ref_map;
337-
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
338338

339339
/* opportunistically-updated references: */
340340
struct ref *orefs = NULL, **oref_tail = &orefs;
341341

342-
const struct ref *remote_refs;
343-
344-
if (rs->nr)
345-
refspec_ref_prefixes(rs, &ref_prefixes);
346-
else if (transport->remote && transport->remote->fetch.nr)
347-
refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);
348-
349-
if (ref_prefixes.argc &&
350-
(tags == TAGS_SET || (tags == TAGS_DEFAULT && !rs->nr))) {
351-
argv_array_push(&ref_prefixes, "refs/tags/");
352-
}
353-
354-
remote_refs = transport_get_remote_refs(transport, &ref_prefixes);
355-
356-
argv_array_clear(&ref_prefixes);
342+
struct string_list existing_refs = STRING_LIST_INIT_DUP;
357343

358344
if (rs->nr) {
359345
struct refspec *fetch_refspec;
@@ -390,15 +376,14 @@ static struct ref *get_ref_map(struct transport *transport,
390376
if (refmap.nr)
391377
fetch_refspec = &refmap;
392378
else
393-
fetch_refspec = &transport->remote->fetch;
379+
fetch_refspec = &remote->fetch;
394380

395381
for (i = 0; i < fetch_refspec->nr; i++)
396382
get_fetch_map(ref_map, &fetch_refspec->items[i], &oref_tail, 1);
397383
} else if (refmap.nr) {
398384
die("--refmap option is only meaningful with command-line refspec(s).");
399385
} else {
400386
/* Use the defaults */
401-
struct remote *remote = transport->remote;
402387
struct branch *branch = branch_get(NULL);
403388
int has_merge = branch_has_merge_config(branch);
404389
if (remote &&
@@ -437,7 +422,7 @@ static struct ref *get_ref_map(struct transport *transport,
437422
/* also fetch all tags */
438423
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
439424
else if (tags == TAGS_DEFAULT && *autotags)
440-
find_non_local_tags(transport, &ref_map, &tail);
425+
find_non_local_tags(remote_refs, &ref_map, &tail);
441426

442427
/* Now append any refs to be updated opportunistically: */
443428
*tail = orefs;
@@ -446,7 +431,23 @@ static struct ref *get_ref_map(struct transport *transport,
446431
tail = &rm->next;
447432
}
448433

449-
return ref_remove_duplicates(ref_map);
434+
ref_map = ref_remove_duplicates(ref_map);
435+
436+
for_each_ref(add_existing, &existing_refs);
437+
for (rm = ref_map; rm; rm = rm->next) {
438+
if (rm->peer_ref) {
439+
struct string_list_item *peer_item =
440+
string_list_lookup(&existing_refs,
441+
rm->peer_ref->name);
442+
if (peer_item) {
443+
struct object_id *old_oid = peer_item->util;
444+
oidcpy(&rm->peer_ref->old_oid, old_oid);
445+
}
446+
}
447+
}
448+
string_list_clear(&existing_refs, 1);
449+
450+
return ref_map;
450451
}
451452

452453
#define STORE_REF_ERROR_OTHER 1
@@ -756,7 +757,7 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid)
756757
}
757758

758759
static int store_updated_refs(const char *raw_url, const char *remote_name,
759-
struct ref *ref_map)
760+
int connectivity_checked, struct ref *ref_map)
760761
{
761762
FILE *fp;
762763
struct commit *commit;
@@ -778,10 +779,12 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
778779
else
779780
url = xstrdup("foreign");
780781

781-
rm = ref_map;
782-
if (check_connected(iterate_ref_map, &rm, NULL)) {
783-
rc = error(_("%s did not send all necessary objects\n"), url);
784-
goto abort;
782+
if (!connectivity_checked) {
783+
rm = ref_map;
784+
if (check_connected(iterate_ref_map, &rm, NULL)) {
785+
rc = error(_("%s did not send all necessary objects\n"), url);
786+
goto abort;
787+
}
785788
}
786789

787790
prepare_format_display(ref_map);
@@ -933,15 +936,32 @@ static int quickfetch(struct ref *ref_map)
933936
return check_connected(iterate_ref_map, &rm, &opt);
934937
}
935938

936-
static int fetch_refs(struct transport *transport, struct ref *ref_map)
939+
static int fetch_refs(struct transport *transport, struct ref *ref_map,
940+
struct ref **updated_remote_refs)
937941
{
938942
int ret = quickfetch(ref_map);
939943
if (ret)
940-
ret = transport_fetch_refs(transport, ref_map);
944+
ret = transport_fetch_refs(transport, ref_map,
945+
updated_remote_refs);
941946
if (!ret)
942-
ret |= store_updated_refs(transport->url,
943-
transport->remote->name,
944-
ref_map);
947+
/*
948+
* Keep the new pack's ".keep" file around to allow the caller
949+
* time to update refs to reference the new objects.
950+
*/
951+
return 0;
952+
transport_unlock_pack(transport);
953+
return ret;
954+
}
955+
956+
/* Update local refs based on the ref values fetched from a remote */
957+
static int consume_refs(struct transport *transport, struct ref *ref_map)
958+
{
959+
int connectivity_checked = transport->smart_options
960+
? transport->smart_options->connectivity_checked : 0;
961+
int ret = store_updated_refs(transport->url,
962+
transport->remote->name,
963+
connectivity_checked,
964+
ref_map);
945965
transport_unlock_pack(transport);
946966
return ret;
947967
}
@@ -1087,7 +1107,8 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
10871107
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
10881108
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
10891109
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
1090-
fetch_refs(transport, ref_map);
1110+
if (!fetch_refs(transport, ref_map, NULL))
1111+
consume_refs(transport, ref_map);
10911112

10921113
if (gsecondary) {
10931114
transport_disconnect(gsecondary);
@@ -1098,13 +1119,12 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
10981119
static int do_fetch(struct transport *transport,
10991120
struct refspec *rs)
11001121
{
1101-
struct string_list existing_refs = STRING_LIST_INIT_DUP;
11021122
struct ref *ref_map;
1103-
struct ref *rm;
11041123
int autotags = (transport->remote->fetch_tags == 1);
11051124
int retcode = 0;
1106-
1107-
for_each_ref(add_existing, &existing_refs);
1125+
const struct ref *remote_refs;
1126+
struct ref *updated_remote_refs = NULL;
1127+
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
11081128

11091129
if (tags == TAGS_DEFAULT) {
11101130
if (transport->remote->fetch_tags == 2)
@@ -1120,22 +1140,24 @@ static int do_fetch(struct transport *transport,
11201140
goto cleanup;
11211141
}
11221142

1123-
ref_map = get_ref_map(transport, rs, tags, &autotags);
1124-
if (!update_head_ok)
1125-
check_not_current_branch(ref_map);
1143+
if (rs->nr)
1144+
refspec_ref_prefixes(rs, &ref_prefixes);
1145+
else if (transport->remote && transport->remote->fetch.nr)
1146+
refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);
11261147

1127-
for (rm = ref_map; rm; rm = rm->next) {
1128-
if (rm->peer_ref) {
1129-
struct string_list_item *peer_item =
1130-
string_list_lookup(&existing_refs,
1131-
rm->peer_ref->name);
1132-
if (peer_item) {
1133-
struct object_id *old_oid = peer_item->util;
1134-
oidcpy(&rm->peer_ref->old_oid, old_oid);
1135-
}
1136-
}
1148+
if (ref_prefixes.argc &&
1149+
(tags == TAGS_SET || (tags == TAGS_DEFAULT && !rs->nr))) {
1150+
argv_array_push(&ref_prefixes, "refs/tags/");
11371151
}
11381152

1153+
remote_refs = transport_get_remote_refs(transport, &ref_prefixes);
1154+
argv_array_clear(&ref_prefixes);
1155+
1156+
ref_map = get_ref_map(transport->remote, remote_refs, rs,
1157+
tags, &autotags);
1158+
if (!update_head_ok)
1159+
check_not_current_branch(ref_map);
1160+
11391161
if (tags == TAGS_DEFAULT && autotags)
11401162
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
11411163
if (prune) {
@@ -1152,7 +1174,24 @@ static int do_fetch(struct transport *transport,
11521174
transport->url);
11531175
}
11541176
}
1155-
if (fetch_refs(transport, ref_map)) {
1177+
1178+
if (fetch_refs(transport, ref_map, &updated_remote_refs)) {
1179+
free_refs(ref_map);
1180+
retcode = 1;
1181+
goto cleanup;
1182+
}
1183+
if (updated_remote_refs) {
1184+
/*
1185+
* Regenerate ref_map using the updated remote refs. This is
1186+
* to account for additional information which may be provided
1187+
* by the transport (e.g. shallow info).
1188+
*/
1189+
free_refs(ref_map);
1190+
ref_map = get_ref_map(transport->remote, updated_remote_refs, rs,
1191+
tags, &autotags);
1192+
free_refs(updated_remote_refs);
1193+
}
1194+
if (consume_refs(transport, ref_map)) {
11561195
free_refs(ref_map);
11571196
retcode = 1;
11581197
goto cleanup;
@@ -1164,14 +1203,13 @@ static int do_fetch(struct transport *transport,
11641203
if (tags == TAGS_DEFAULT && autotags) {
11651204
struct ref **tail = &ref_map;
11661205
ref_map = NULL;
1167-
find_non_local_tags(transport, &ref_map, &tail);
1206+
find_non_local_tags(remote_refs, &ref_map, &tail);
11681207
if (ref_map)
11691208
backfill_tags(transport, ref_map);
11701209
free_refs(ref_map);
11711210
}
11721211

11731212
cleanup:
1174-
string_list_clear(&existing_refs, 1);
11751213
return retcode;
11761214
}
11771215

‎connected.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
5858
argv_array_push(&rev_list.args, "--stdin");
5959
if (repository_format_partial_clone)
6060
argv_array_push(&rev_list.args, "--exclude-promisor-objects");
61-
argv_array_push(&rev_list.args, "--not");
62-
argv_array_push(&rev_list.args, "--all");
61+
if (!opt->is_deepening_fetch) {
62+
argv_array_push(&rev_list.args, "--not");
63+
argv_array_push(&rev_list.args, "--all");
64+
}
6365
argv_array_push(&rev_list.args, "--quiet");
6466
if (opt->progress)
6567
argv_array_pushf(&rev_list.args, "--progress=%s",

‎connected.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ struct check_connected_options {
3838
* Insert these variables into the environment of the child process.
3939
*/
4040
const char **env;
41+
42+
/*
43+
* If non-zero, check the ancestry chain completely, not stopping at
44+
* any existing ref. This is necessary when deepening existing refs
45+
* during a fetch.
46+
*/
47+
unsigned is_deepening_fetch : 1;
4148
};
4249

4350
#define CHECK_CONNECTED_INIT { 0 }

0 commit comments

Comments
 (0)