From: Junio C Hamano Date: Tue, 6 Nov 2018 06:50:19 +0000 (+0900) Subject: Merge branch 'jt/upload-pack-v2-fix-shallow' X-Git-Tag: v2.20.0-rc0~72 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/3df27e0e34a634efd0aa842bc49030393deed673?ds=inline;hp=-c Merge branch 'jt/upload-pack-v2-fix-shallow' "git fetch" over protocol v2 into a shallow repository failed to fetch full history behind a new tip of history that was diverged before the cut-off point of the history that was previously fetched shallowly. * jt/upload-pack-v2-fix-shallow: upload-pack: clear flags before each v2 request upload-pack: make want_obj not global upload-pack: make have_obj not global --- 3df27e0e34a634efd0aa842bc49030393deed673 diff --combined t/t5702-protocol-v2.sh index 8360188c01,8e3b773e3f..6ab8dea8cd --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@@ -79,19 -79,6 +79,19 @@@ test_expect_success 'fetch with git:// grep "fetch< version 2" log ' +test_expect_success 'fetch by hash without tag following with protocol v2 does not list refs' ' + test_when_finished "rm -f log" && + + test_commit -C "$daemon_parent" two_a && + git -C "$daemon_parent" rev-parse two_a >two_a_hash && + + GIT_TRACE_PACKET="$(pwd)/log" git -C daemon_child -c protocol.version=2 \ + fetch --no-tags origin $(cat two_a_hash) && + + grep "fetch< version 2" log && + ! grep "fetch> command=ls-refs" log +' + test_expect_success 'pull with git:// using protocol v2' ' test_when_finished "rm -f log" && @@@ -299,10 -286,6 +299,10 @@@ test_expect_success 'dynamically fetch grep "version 2" trace ' +test_expect_success 'when dynamically fetching missing object, do not list refs' ' + ! grep "git> command=ls-refs" trace +' + test_expect_success 'partial fetch' ' rm -rf client "$(pwd)/trace" && git init client && @@@ -446,6 -429,31 +446,31 @@@ test_expect_success 'fetch supports inc git -C client cat-file -e $(git -C client rev-parse annotated_tag) ' + test_expect_success 'upload-pack respects client shallows' ' + rm -rf server client trace && + + git init server && + test_commit -C server base && + test_commit -C server client_has && + + git clone --depth=1 "file://$(pwd)/server" client && + + # Add extra commits to the client so that the whole fetch takes more + # than 1 request (due to negotiation) + for i in $(test_seq 1 32) + do + test_commit -C client c$i + done && + + git -C server checkout -b newbranch base && + test_commit -C server client_wants && + + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \ + fetch origin newbranch && + # Ensure that protocol v2 is used + grep "fetch< version 2" trace + ' + # Test protocol v2 with 'http://' transport # . "$TEST_DIRECTORY"/lib-httpd.sh diff --combined upload-pack.c index 3f906ccb40,14e42526ce..5e81f1ff24 --- a/upload-pack.c +++ b/upload-pack.c @@@ -38,6 -38,9 +38,9 @@@ #define CLIENT_SHALLOW (1u << 18) #define HIDDEN_REF (1u << 19) + #define ALL_FLAGS (THEY_HAVE | OUR_REF | WANTED | COMMON_KNOWN | SHALLOW | \ + NOT_SHALLOW | CLIENT_SHALLOW | HIDDEN_REF) + static timestamp_t oldest_have; static int deepen_relative; @@@ -53,8 -56,6 +56,6 @@@ static int no_progress, daemon_mode #define ALLOW_ANY_SHA1 07 static unsigned int allow_unadvertised_object_request; static int shallow_nr; - static struct object_array have_obj; - static struct object_array want_obj; static struct object_array extra_edge_obj; static unsigned int timeout; static int keepalive = 5; @@@ -100,7 -101,8 +101,8 @@@ static int write_one_shallow(const stru return 0; } - static void create_pack_file(void) + static void create_pack_file(const struct object_array *have_obj, + const struct object_array *want_obj) { struct child_process pack_objects = CHILD_PROCESS_INIT; char data[8193], progress[128]; @@@ -161,13 -163,13 +163,13 @@@ if (shallow_nr) for_each_commit_graft(write_one_shallow, pipe_fd); - for (i = 0; i < want_obj.nr; i++) + for (i = 0; i < want_obj->nr; i++) fprintf(pipe_fd, "%s\n", - oid_to_hex(&want_obj.objects[i].item->oid)); + oid_to_hex(&want_obj->objects[i].item->oid)); fprintf(pipe_fd, "--not\n"); - for (i = 0; i < have_obj.nr; i++) + for (i = 0; i < have_obj->nr; i++) fprintf(pipe_fd, "%s\n", - oid_to_hex(&have_obj.objects[i].item->oid)); + oid_to_hex(&have_obj->objects[i].item->oid)); for (i = 0; i < extra_edge_obj.nr; i++) fprintf(pipe_fd, "%s\n", oid_to_hex(&extra_edge_obj.objects[i].item->oid)); @@@ -304,7 -306,8 +306,8 @@@ die("git upload-pack: %s", abort_msg); } - static int got_oid(const char *hex, struct object_id *oid) + static int got_oid(const char *hex, struct object_id *oid, + struct object_array *have_obj) { struct object *o; int we_knew_they_have = 0; @@@ -332,25 -335,27 +335,27 @@@ parents->item->object.flags |= THEY_HAVE; } if (!we_knew_they_have) { - add_object_array(o, NULL, &have_obj); + add_object_array(o, NULL, have_obj); return 1; } return 0; } - static int ok_to_give_up(void) + static int ok_to_give_up(const struct object_array *have_obj, + struct object_array *want_obj) { uint32_t min_generation = GENERATION_NUMBER_ZERO; - if (!have_obj.nr) + if (!have_obj->nr) return 0; - return can_all_from_reach_with_flag(&want_obj, THEY_HAVE, + return can_all_from_reach_with_flag(want_obj, THEY_HAVE, COMMON_KNOWN, oldest_have, min_generation); } - static int get_common_commits(void) + static int get_common_commits(struct object_array *have_obj, + struct object_array *want_obj) { struct object_id oid; char last_hex[GIT_MAX_HEXSZ + 1]; @@@ -368,11 -373,11 +373,11 @@@ if (!line) { if (multi_ack == 2 && got_common - && !got_other && ok_to_give_up()) { + && !got_other && ok_to_give_up(have_obj, want_obj)) { sent_ready = 1; packet_write_fmt(1, "ACK %s ready\n", last_hex); } - if (have_obj.nr == 0 || multi_ack) + if (have_obj->nr == 0 || multi_ack) packet_write_fmt(1, "NAK\n"); if (no_done && sent_ready) { @@@ -386,10 -391,10 +391,10 @@@ continue; } if (skip_prefix(line, "have ", &arg)) { - switch (got_oid(arg, &oid)) { + switch (got_oid(arg, &oid, have_obj)) { case -1: /* they have what we do not */ got_other = 1; - if (multi_ack && ok_to_give_up()) { + if (multi_ack && ok_to_give_up(have_obj, want_obj)) { const char *hex = oid_to_hex(&oid); if (multi_ack == 2) { sent_ready = 1; @@@ -405,14 -410,14 +410,14 @@@ packet_write_fmt(1, "ACK %s common\n", last_hex); else if (multi_ack) packet_write_fmt(1, "ACK %s continue\n", last_hex); - else if (have_obj.nr == 1) + else if (have_obj->nr == 1) packet_write_fmt(1, "ACK %s\n", last_hex); break; } continue; } if (!strcmp(line, "done")) { - if (have_obj.nr > 0) { + if (have_obj->nr > 0) { if (multi_ack) packet_write_fmt(1, "ACK %s\n", last_hex); return 0; @@@ -444,7 -449,6 +449,7 @@@ static int do_reachable_revlist(struct struct object *o; char namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */ int i; + const unsigned hexsz = the_hash_algo->hexsz; cmd->argv = argv; cmd->git_cmd = 1; @@@ -463,7 -467,7 +468,7 @@@ goto error; namebuf[0] = '^'; - namebuf[GIT_SHA1_HEXSZ + 1] = '\n'; + namebuf[hexsz + 1] = '\n'; for (i = get_max_object_index(); 0 < i; ) { o = get_indexed_object(--i); if (!o) @@@ -472,11 -476,11 +477,11 @@@ o->flags &= ~TMP_MARK; if (!is_our_ref(o)) continue; - memcpy(namebuf + 1, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ); - if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 2) < 0) + memcpy(namebuf + 1, oid_to_hex(&o->oid), hexsz); + if (write_in_full(cmd->in, namebuf, hexsz + 2) < 0) goto error; } - namebuf[GIT_SHA1_HEXSZ] = '\n'; + namebuf[hexsz] = '\n'; for (i = 0; i < src->nr; i++) { o = src->objects[i].item; if (is_our_ref(o)) { @@@ -486,8 -490,8 +491,8 @@@ } if (reachable && o->type == OBJ_COMMIT) o->flags |= TMP_MARK; - memcpy(namebuf, oid_to_hex(&o->oid), GIT_SHA1_HEXSZ); - if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 1) < 0) + memcpy(namebuf, oid_to_hex(&o->oid), hexsz); + if (write_in_full(cmd->in, namebuf, hexsz + 1) < 0) goto error; } close(cmd->in); @@@ -583,7 -587,7 +588,7 @@@ error return 1; } - static void check_non_tip(void) + static void check_non_tip(struct object_array *want_obj) { int i; @@@ -594,14 -598,14 +599,14 @@@ */ if (!stateless_rpc && !(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1)) goto error; - if (!has_unreachable(&want_obj)) + if (!has_unreachable(want_obj)) /* All the non-tip ones are ancestors of what we advertised */ return; error: /* Pick one of them (we know there at least is one) */ - for (i = 0; i < want_obj.nr; i++) { - struct object *o = want_obj.objects[i].item; + for (i = 0; i < want_obj->nr; i++) { + struct object *o = want_obj->objects[i].item; if (!is_our_ref(o)) die("git upload-pack: not our ref %s", oid_to_hex(&o->oid)); @@@ -622,7 -626,8 +627,8 @@@ static void send_shallow(struct commit_ } } - static void send_unshallow(const struct object_array *shallows) + static void send_unshallow(const struct object_array *shallows, + struct object_array *want_obj) { int i; @@@ -646,7 -651,7 +652,7 @@@ parents = ((struct commit *)object)->parents; while (parents) { add_object_array(&parents->item->object, - NULL, &want_obj); + NULL, want_obj); parents = parents->next; } add_object_array(object, NULL, &extra_edge_obj); @@@ -657,7 -662,7 +663,7 @@@ } static void deepen(int depth, int deepen_relative, - struct object_array *shallows) + struct object_array *shallows, struct object_array *want_obj) { if (depth == INFINITE_DEPTH && !is_repository_shallow(the_repository)) { int i; @@@ -680,17 -685,18 +686,18 @@@ } else { struct commit_list *result; - result = get_shallow_commits(&want_obj, depth, + result = get_shallow_commits(want_obj, depth, SHALLOW, NOT_SHALLOW); send_shallow(result); free_commit_list(result); } - send_unshallow(shallows); + send_unshallow(shallows, want_obj); } static void deepen_by_rev_list(int ac, const char **av, - struct object_array *shallows) + struct object_array *shallows, + struct object_array *want_obj) { struct commit_list *result; @@@ -698,21 -704,22 +705,22 @@@ result = get_shallow_commits_by_rev_list(ac, av, SHALLOW, NOT_SHALLOW); send_shallow(result); free_commit_list(result); - send_unshallow(shallows); + send_unshallow(shallows, want_obj); } /* Returns 1 if a shallow list is sent or 0 otherwise */ static int send_shallow_list(int depth, int deepen_rev_list, timestamp_t deepen_since, struct string_list *deepen_not, - struct object_array *shallows) + struct object_array *shallows, + struct object_array *want_obj) { int ret = 0; if (depth > 0 && deepen_rev_list) die("git upload-pack: deepen and deepen-since (or deepen-not) cannot be used together"); if (depth > 0) { - deepen(depth, deepen_relative, shallows); + deepen(depth, deepen_relative, shallows, want_obj); ret = 1; } else if (deepen_rev_list) { struct argv_array av = ARGV_ARRAY_INIT; @@@ -729,11 -736,11 +737,11 @@@ } argv_array_push(&av, "--not"); } - for (i = 0; i < want_obj.nr; i++) { - struct object *o = want_obj.objects[i].item; + for (i = 0; i < want_obj->nr; i++) { + struct object *o = want_obj->objects[i].item; argv_array_push(&av, oid_to_hex(&o->oid)); } - deepen_by_rev_list(av.argc, av.argv, shallows); + deepen_by_rev_list(av.argc, av.argv, shallows, want_obj); argv_array_clear(&av); ret = 1; } else { @@@ -818,7 -825,7 +826,7 @@@ static int process_deepen_not(const cha return 0; } - static void receive_needs(void) + static void receive_needs(struct object_array *want_obj) { struct object_array shallows = OBJECT_ARRAY_INIT; struct string_list deepen_not = STRING_LIST_INIT_DUP; @@@ -896,7 -903,7 +904,7 @@@ if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1 || is_our_ref(o))) has_non_tip = 1; - add_object_array(o, NULL, &want_obj); + add_object_array(o, NULL, want_obj); } } @@@ -908,7 -915,7 +916,7 @@@ * by another process that handled the initial request. */ if (has_non_tip) - check_non_tip(); + check_non_tip(want_obj); if (!use_sideband && daemon_mode) no_progress = 1; @@@ -917,7 -924,7 +925,7 @@@ return; if (send_shallow_list(depth, deepen_rev_list, deepen_since, - &deepen_not, &shallows)) + &deepen_not, &shallows, want_obj)) packet_flush(1); object_array_clear(&shallows); } @@@ -1029,23 -1036,21 +1037,24 @@@ static int upload_pack_config(const cha keepalive = git_config_int(var, value); if (!keepalive) keepalive = -1; - } else if (current_config_scope() != CONFIG_SCOPE_REPO) { - if (!strcmp("uploadpack.packobjectshook", var)) - return git_config_string(&pack_objects_hook, var, value); } else if (!strcmp("uploadpack.allowfilter", var)) { allow_filter = git_config_bool(var, value); } else if (!strcmp("uploadpack.allowrefinwant", var)) { allow_ref_in_want = git_config_bool(var, value); } + + if (current_config_scope() != CONFIG_SCOPE_REPO) { + if (!strcmp("uploadpack.packobjectshook", var)) + return git_config_string(&pack_objects_hook, var, value); + } + return parse_hide_refs_config(var, value, "uploadpack"); } void upload_pack(struct upload_pack_options *options) { struct string_list symref = STRING_LIST_INIT_DUP; + struct object_array want_obj = OBJECT_ARRAY_INIT; stateless_rpc = options->stateless_rpc; timeout = options->timeout; @@@ -1069,10 -1074,11 +1078,11 @@@ if (options->advertise_refs) return; - receive_needs(); + receive_needs(&want_obj); if (want_obj.nr) { - get_common_commits(); - create_pack_file(); + struct object_array have_obj = OBJECT_ARRAY_INIT; + get_common_commits(&have_obj, &want_obj); + create_pack_file(&have_obj, &want_obj); } } @@@ -1122,7 -1128,7 +1132,7 @@@ static void upload_pack_data_clear(stru string_list_clear(&data->deepen_not, 0); } - static int parse_want(const char *line) + static int parse_want(const char *line, struct object_array *want_obj) { const char *arg; if (skip_prefix(line, "want ", &arg)) { @@@ -1144,7 -1150,7 +1154,7 @@@ if (!(o->flags & WANTED)) { o->flags |= WANTED; - add_object_array(o, NULL, &want_obj); + add_object_array(o, NULL, want_obj); } return 1; @@@ -1153,7 -1159,8 +1163,8 @@@ return 0; } - static int parse_want_ref(const char *line, struct string_list *wanted_refs) + static int parse_want_ref(const char *line, struct string_list *wanted_refs, + struct object_array *want_obj) { const char *arg; if (skip_prefix(line, "want-ref ", &arg)) { @@@ -1172,7 -1179,7 +1183,7 @@@ o = parse_object_or_die(&oid, arg); if (!(o->flags & WANTED)) { o->flags |= WANTED; - add_object_array(o, NULL, &want_obj); + add_object_array(o, NULL, want_obj); } return 1; @@@ -1197,16 -1204,18 +1208,18 @@@ static int parse_have(const char *line } static void process_args(struct packet_reader *request, - struct upload_pack_data *data) + struct upload_pack_data *data, + struct object_array *want_obj) { while (packet_reader_read(request) != PACKET_READ_FLUSH) { const char *arg = request->line; const char *p; /* process want */ - if (parse_want(arg)) + if (parse_want(arg, want_obj)) continue; - if (allow_ref_in_want && parse_want_ref(arg, &data->wanted_refs)) + if (allow_ref_in_want && + parse_want_ref(arg, &data->wanted_refs, want_obj)) continue; /* process have line */ if (parse_have(arg, &data->haves)) @@@ -1260,7 -1269,8 +1273,8 @@@ } } - static int process_haves(struct oid_array *haves, struct oid_array *common) + static int process_haves(struct oid_array *haves, struct oid_array *common, + struct object_array *have_obj) { int i; @@@ -1293,13 -1303,15 +1307,15 @@@ parents->item->object.flags |= THEY_HAVE; } if (!we_knew_they_have) - add_object_array(o, NULL, &have_obj); + add_object_array(o, NULL, have_obj); } return 0; } - static int send_acks(struct oid_array *acks, struct strbuf *response) + static int send_acks(struct oid_array *acks, struct strbuf *response, + const struct object_array *have_obj, + struct object_array *want_obj) { int i; @@@ -1314,7 -1326,7 +1330,7 @@@ oid_to_hex(&acks->oid[i])); } - if (ok_to_give_up()) { + if (ok_to_give_up(have_obj, want_obj)) { /* Send Ready */ packet_buf_write(response, "ready\n"); return 1; @@@ -1323,16 -1335,18 +1339,18 @@@ return 0; } - static int process_haves_and_send_acks(struct upload_pack_data *data) + static int process_haves_and_send_acks(struct upload_pack_data *data, + struct object_array *have_obj, + struct object_array *want_obj) { struct oid_array common = OID_ARRAY_INIT; struct strbuf response = STRBUF_INIT; int ret = 0; - process_haves(&data->haves, &common); + process_haves(&data->haves, &common, have_obj); if (data->done) { ret = 1; - } else if (send_acks(&common, &response)) { + } else if (send_acks(&common, &response, have_obj, want_obj)) { packet_buf_delim(&response); ret = 1; } else { @@@ -1368,7 -1382,8 +1386,8 @@@ static void send_wanted_ref_info(struc packet_delim(1); } - static void send_shallow_info(struct upload_pack_data *data) + static void send_shallow_info(struct upload_pack_data *data, + struct object_array *want_obj) { /* No shallow info needs to be sent */ if (!data->depth && !data->deepen_rev_list && !data->shallows.nr && @@@ -1379,9 -1394,10 +1398,10 @@@ if (!send_shallow_list(data->depth, data->deepen_rev_list, data->deepen_since, &data->deepen_not, - &data->shallows) && + &data->shallows, want_obj) && is_repository_shallow(the_repository)) - deepen(INFINITE_DEPTH, data->deepen_relative, &data->shallows); + deepen(INFINITE_DEPTH, data->deepen_relative, &data->shallows, + want_obj); packet_delim(1); } @@@ -1398,6 -1414,10 +1418,10 @@@ int upload_pack_v2(struct repository *r { enum fetch_state state = FETCH_PROCESS_ARGS; struct upload_pack_data data; + struct object_array have_obj = OBJECT_ARRAY_INIT; + struct object_array want_obj = OBJECT_ARRAY_INIT; + + clear_object_flags(ALL_FLAGS); git_config(upload_pack_config, NULL); @@@ -1407,7 -1427,7 +1431,7 @@@ while (state != FETCH_DONE) { switch (state) { case FETCH_PROCESS_ARGS: - process_args(request, &data); + process_args(request, &data, &want_obj); if (!want_obj.nr) { /* @@@ -1429,17 -1449,18 +1453,18 @@@ } break; case FETCH_SEND_ACKS: - if (process_haves_and_send_acks(&data)) + if (process_haves_and_send_acks(&data, &have_obj, + &want_obj)) state = FETCH_SEND_PACK; else state = FETCH_DONE; break; case FETCH_SEND_PACK: send_wanted_ref_info(&data); - send_shallow_info(&data); + send_shallow_info(&data, &want_obj); packet_write_fmt(1, "packfile\n"); - create_pack_file(); + create_pack_file(&have_obj, &want_obj); state = FETCH_DONE; break; case FETCH_DONE: @@@ -1448,6 -1469,8 +1473,8 @@@ } upload_pack_data_clear(&data); + object_array_clear(&have_obj); + object_array_clear(&want_obj); return 0; }