{fetch,upload}-pack: support filter in protocol v2

The fetch-pack/upload-pack protocol v2 was developed independently of the filter parameter (used in partial fetches), thus it did not include support for it. Add support for the filter parameter. Like in the legacy protocol, the server advertises and supports "filter" only if uploadpack.allowfilter is configured. Like in the legacy protocol, the client continues with a warning if "--filter" is specified, but the server does not advertise it. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed May 3, 2018 at 16:46 UTC ba95710a3bdcb2a80495b1d93a0e482dd69905e1
4 files changed +140 -5
Documentation/technical/protocol-v2.txt
+9
@@ -290,6 +290,15 @@ included in the clients request as well as the potential addition of the
290 Cannot be used with "deepen", but can be used with
291 "deepen-since".
292
293 +If the 'filter' feature is advertised, the following argument can be
294 +included in the client's request:
295 +
296 + filter <filter-spec>
297 + Request that various objects from the packfile be omitted
298 + using one of several filtering techniques. These are intended
299 + for use with partial clone and partial fetch operations. See
300 + `rev-list` for possible "filter-spec" values.
301 +
302 The response of `fetch` is broken into a number of sections separated by
303 delimiter packets (0001), with each section beginning with its section
304 header.
fetch-pack.c
+19 -4
@@ -1191,14 +1191,29 @@ static int send_fetch_request(int fd_out, const struct fetch_pack_args *args,
1191 else if (is_repository_shallow() || args->deepen)
1192 die(_("Server does not support shallow requests"));
1193
1194 + /* Add filter */
1195 + if (server_supports_feature("fetch", "filter", 0) &&
1196 + args->filter_options.choice) {
1197 + print_verbose(args, _("Server supports filter"));
1198 + packet_buf_write(&req_buf, "filter %s",
1199 + args->filter_options.filter_spec);
1200 + } else if (args->filter_options.choice) {
1201 + warning("filtering not recognized by server, ignoring");
1202 + }
1203 +
1204 /* add wants */
1205 add_wants(wants, &req_buf);
1206
1197 - /* Add all of the common commits we've found in previous rounds */
1198 - add_common(&req_buf, common);
1207 + if (args->no_dependents) {
1208 + packet_buf_write(&req_buf, "done");
1209 + ret = 1;
1210 + } else {
1211 + /* Add all of the common commits we've found in previous rounds */
1212 + add_common(&req_buf, common);
1213
1200 - /* Add initial haves */
1201 - ret = add_haves(&req_buf, haves_to_send, in_vain);
1214 + /* Add initial haves */
1215 + ret = add_haves(&req_buf, haves_to_send, in_vain);
1216 + }
1217
1218 /* Send request */
1219 packet_buf_flush(&req_buf);
t/t5702-protocol-v2.sh
+98
@@ -215,6 +215,104 @@ test_expect_success 'upload-pack respects config using protocol v2' '
215 test_path_is_file server/.git/hookout
216 '
217
218 +test_expect_success 'setup filter tests' '
219 + rm -rf server client &&
220 + git init server &&
221 +
222 + # 1 commit to create a file, and 1 commit to modify it
223 + test_commit -C server message1 a.txt &&
224 + test_commit -C server message2 a.txt &&
225 + git -C server config protocol.version 2 &&
226 + git -C server config uploadpack.allowfilter 1 &&
227 + git -C server config uploadpack.allowanysha1inwant 1 &&
228 + git -C server config protocol.version 2
229 +'
230 +
231 +test_expect_success 'partial clone' '
232 + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \
233 + clone --filter=blob:none "file://$(pwd)/server" client &&
234 + grep "version 2" trace &&
235 +
236 + # Ensure that the old version of the file is missing
237 + git -C client rev-list master --quiet --objects --missing=print \
238 + >observed.oids &&
239 + grep "$(git -C server rev-parse message1:a.txt)" observed.oids &&
240 +
241 + # Ensure that client passes fsck
242 + git -C client fsck
243 +'
244 +
245 +test_expect_success 'dynamically fetch missing object' '
246 + rm "$(pwd)/trace" &&
247 + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
248 + cat-file -p $(git -C server rev-parse message1:a.txt) &&
249 + grep "version 2" trace
250 +'
251 +
252 +test_expect_success 'partial fetch' '
253 + rm -rf client "$(pwd)/trace" &&
254 + git init client &&
255 + SERVER="file://$(pwd)/server" &&
256 + test_config -C client extensions.partialClone "$SERVER" &&
257 +
258 + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \
259 + fetch --filter=blob:none "$SERVER" master:refs/heads/other &&
260 + grep "version 2" trace &&
261 +
262 + # Ensure that the old version of the file is missing
263 + git -C client rev-list other --quiet --objects --missing=print \
264 + >observed.oids &&
265 + grep "$(git -C server rev-parse message1:a.txt)" observed.oids &&
266 +
267 + # Ensure that client passes fsck
268 + git -C client fsck
269 +'
270 +
271 +test_expect_success 'do not advertise filter if not configured to do so' '
272 + SERVER="file://$(pwd)/server" &&
273 +
274 + rm "$(pwd)/trace" &&
275 + git -C server config uploadpack.allowfilter 1 &&
276 + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \
277 + ls-remote "$SERVER" &&
278 + grep "fetch=.*filter" trace &&
279 +
280 + rm "$(pwd)/trace" &&
281 + git -C server config uploadpack.allowfilter 0 &&
282 + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 \
283 + ls-remote "$SERVER" &&
284 + grep "fetch=" trace >fetch_capabilities &&
285 + ! grep filter fetch_capabilities
286 +'
287 +
288 +test_expect_success 'partial clone warns if filter is not advertised' '
289 + rm -rf client &&
290 + git -C server config uploadpack.allowfilter 0 &&
291 + git -c protocol.version=2 \
292 + clone --filter=blob:none "file://$(pwd)/server" client 2>err &&
293 + test_i18ngrep "filtering not recognized by server, ignoring" err
294 +'
295 +
296 +test_expect_success 'even with handcrafted request, filter does not work if not advertised' '
297 + git -C server config uploadpack.allowfilter 0 &&
298 +
299 + # Custom request that tries to filter even though it is not advertised.
300 + test-pkt-line pack >in <<-EOF &&
301 + command=fetch
302 + 0001
303 + want $(git -C server rev-parse master)
304 + filter blob:none
305 + 0000
306 + EOF
307 +
308 + test_must_fail git -C server serve --stateless-rpc <in >/dev/null 2>err &&
309 + grep "unexpected line: .filter blob:none." err &&
310 +
311 + # Exercise to ensure that if advertised, filter works
312 + git -C server config uploadpack.allowfilter 1 &&
313 + git -C server serve --stateless-rpc <in >/dev/null
314 +'
315 +
316 # Test protocol v2 with 'http://' transport
317 #
318 . "$TEST_DIRECTORY"/lib-httpd.sh
upload-pack.c
+14 -1
@@ -1205,6 +1205,7 @@ static void process_args(struct packet_reader *request,
1205 {
1206 while (packet_reader_read(request) != PACKET_READ_FLUSH) {
1207 const char *arg = request->line;
1208 + const char *p;
1209
1210 /* process want */
1211 if (parse_want(arg))
@@ -1251,6 +1252,11 @@ static void process_args(struct packet_reader *request,
1252 continue;
1253 }
1254
1255 + if (allow_filter && skip_prefix(arg, "filter ", &p)) {
1256 + parse_list_objects_filter(&filter_options, p);
1257 + continue;
1258 + }
1259 +
1260 /* ignore unknown lines maybe? */
1261 die("unexpected line: '%s'", arg);
1262 }
@@ -1430,7 +1436,14 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,
1436 int upload_pack_advertise(struct repository *r,
1437 struct strbuf *value)
1438 {
1433 - if (value)
1439 + if (value) {
1440 + int allow_filter_value;
1441 strbuf_addstr(value, "shallow");
1442 + if (!repo_config_get_bool(the_repository,
1443 + "uploadpack.allowfilter",
1444 + &allow_filter_value) &&
1445 + allow_filter_value)
1446 + strbuf_addstr(value, " filter");
1447 + }
1448 return 1;
1449 }