upload-pack: clear filter_options for each v2 fetch command
Because of the request/response model of protocol v2, the upload_pack_v2() function is sometimes called twice in the same process, while 'struct list_objects_filter_options filter_options' was declared as static at the beginning of 'upload-pack.c'. This made the check in list_objects_filter_die_if_populated(), which is called by process_args(), fail the second time upload_pack_v2() is called, as filter_options had already been populated the first time. To fix that, filter_options is not static any more. It's now owned directly by upload_pack(). It's now also part of 'struct upload_pack_data', so that it's owned indirectly by upload_pack_v2(). In the long term, the goal is to also have upload_pack() use 'struct upload_pack_data', so adding filter_options to this struct makes more sense than to have it owned directly by upload_pack_v2(). This fixes the first of the 2 bugs documented by d0badf8797 (partial-clone: demonstrate bugs in partial fetch, 2020-02-21). Helped-by: Derrick Stolee <dstolee@microsoft.com> Helped-by: Jeff King <peff@peff.net> Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder committed
May 8, 2020 at 10:01 UTC
08450ef7918ef2f24bc3bf6afcd1782aa0677015
2 files changed
+25
-16
t/t5616-partial-clone.sh
+3
-4
@@ -384,12 +384,11 @@ test_expect_success 'fetch lazy-fetches only to resolve deltas, protocol v2' '
384
grep "want $(cat hash)" trace
385
'
386
387
-# The following two tests must be in this order, or else
388
-# the first will not fail. It is important that the srv.bare
389
-# repository did not have tags during clone, but has tags
387
+# The following two tests must be in this order. It is important that
388
+# the srv.bare repository did not have tags during clone, but has tags
389
# in the fetch.
390
392
-test_expect_failure 'verify fetch succeeds when asking for new tags' '
391
+test_expect_success 'verify fetch succeeds when asking for new tags' '
392
git clone --filter=blob:none "file://$(pwd)/srv.bare" tag-test &&
393
for i in I J K
394
do
upload-pack.c
+22
-12
@@ -68,7 +68,6 @@ static const char *pack_objects_hook;
68
static int filter_capability_requested;
69
static int allow_filter;
70
static int allow_ref_in_want;
71
-static struct list_objects_filter_options filter_options;
71
72
static int allow_sideband_all;
73
@@ -103,7 +102,8 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
102
}
103
104
static void create_pack_file(const struct object_array *have_obj,
106
- const struct object_array *want_obj)
105
+ const struct object_array *want_obj,
106
+ struct list_objects_filter_options *filter_options)
107
{
108
struct child_process pack_objects = CHILD_PROCESS_INIT;
109
char data[8193], progress[128];
@@ -140,9 +140,9 @@ static void create_pack_file(const struct object_array *have_obj,
140
argv_array_push(&pack_objects.args, "--delta-base-offset");
141
if (use_include_tag)
142
argv_array_push(&pack_objects.args, "--include-tag");
143
- if (filter_options.choice) {
143
+ if (filter_options->choice) {
144
const char *spec =
145
- expand_list_objects_filter_spec(&filter_options);
145
+ expand_list_objects_filter_spec(filter_options);
146
if (pack_objects.use_shell) {
147
struct strbuf buf = STRBUF_INIT;
148
sq_quote_buf(&buf, spec);
@@ -848,7 +848,9 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
848
return 0;
849
}
850
851
-static void receive_needs(struct packet_reader *reader, struct object_array *want_obj)
851
+static void receive_needs(struct packet_reader *reader,
852
+ struct object_array *want_obj,
853
+ struct list_objects_filter_options *filter_options)
854
{
855
struct object_array shallows = OBJECT_ARRAY_INIT;
856
struct string_list deepen_not = STRING_LIST_INIT_DUP;
@@ -883,8 +885,8 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan
885
if (skip_prefix(reader->line, "filter ", &arg)) {
886
if (!filter_capability_requested)
887
die("git upload-pack: filtering capability not negotiated");
886
- list_objects_filter_die_if_populated(&filter_options);
887
- parse_list_objects_filter(&filter_options, arg);
888
+ list_objects_filter_die_if_populated(filter_options);
889
+ parse_list_objects_filter(filter_options, arg);
890
continue;
891
}
892
@@ -1087,11 +1089,14 @@ void upload_pack(struct upload_pack_options *options)
1089
struct string_list symref = STRING_LIST_INIT_DUP;
1090
struct object_array want_obj = OBJECT_ARRAY_INIT;
1091
struct packet_reader reader;
1092
+ struct list_objects_filter_options filter_options;
1093
1094
stateless_rpc = options->stateless_rpc;
1095
timeout = options->timeout;
1096
daemon_mode = options->daemon_mode;
1097
1098
+ memset(&filter_options, 0, sizeof(filter_options));
1099
+
1100
git_config(upload_pack_config, NULL);
1101
1102
head_ref_namespaced(find_symref, &symref);
@@ -1114,12 +1119,14 @@ void upload_pack(struct upload_pack_options *options)
1119
PACKET_READ_CHOMP_NEWLINE |
1120
PACKET_READ_DIE_ON_ERR_PACKET);
1121
1117
- receive_needs(&reader, &want_obj);
1122
+ receive_needs(&reader, &want_obj, &filter_options);
1123
if (want_obj.nr) {
1124
struct object_array have_obj = OBJECT_ARRAY_INIT;
1125
get_common_commits(&reader, &have_obj, &want_obj);
1121
- create_pack_file(&have_obj, &want_obj);
1126
+ create_pack_file(&have_obj, &want_obj, &filter_options);
1127
}
1128
+
1129
+ list_objects_filter_release(&filter_options);
1130
}
1131
1132
struct upload_pack_data {
@@ -1134,6 +1141,8 @@ struct upload_pack_data {
1141
int deepen_rev_list;
1142
int deepen_relative;
1143
1144
+ struct list_objects_filter_options filter_options;
1145
+
1146
struct packet_writer writer;
1147
1148
unsigned stateless_rpc : 1;
@@ -1169,6 +1178,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
1178
oid_array_clear(&data->haves);
1179
object_array_clear(&data->shallows);
1180
string_list_clear(&data->deepen_not, 0);
1181
+ list_objects_filter_release(&data->filter_options);
1182
}
1183
1184
static int parse_want(struct packet_writer *writer, const char *line,
@@ -1306,8 +1316,8 @@ static void process_args(struct packet_reader *request,
1316
}
1317
1318
if (allow_filter && skip_prefix(arg, "filter ", &p)) {
1309
- list_objects_filter_die_if_populated(&filter_options);
1310
- parse_list_objects_filter(&filter_options, p);
1319
+ list_objects_filter_die_if_populated(&data->filter_options);
1320
+ parse_list_objects_filter(&data->filter_options, p);
1321
continue;
1322
}
1323
@@ -1511,7 +1521,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1521
send_shallow_info(&data, &want_obj);
1522
1523
packet_writer_write(&data.writer, "packfile\n");
1514
- create_pack_file(&have_obj, &want_obj);
1524
+ create_pack_file(&have_obj, &want_obj, &data.filter_options);
1525
state = FETCH_DONE;
1526
break;
1527
case FETCH_DONE: