@matheusbsbs / matheus-bagliano / commits / bd0b42aed3

fetch-pack: do not take shallow lock unnecessarily

When fetching using protocol v2, the remote may send a "shallow-info" section if the client is shallow. If so, Git as the client currently takes the shallow file lock, even if the "shallow-info" section is empty. This is not a problem except that Git does not support taking the shallow file lock after modifying the shallow file, because is_repository_shallow() stores information that is never cleared. And this take-after-modify occurs when Git does a tag-following fetch from a shallow repository on a transport that does not support tag following (since in this case, 2 fetches are performed). To solve this issue, take the shallow file lock (and perform all other shallow processing) only if the "shallow-info" section is non-empty; otherwise, behave as if it were empty. A full solution (probably, ensuring that any action of committing shallow file locks also includes clearing the information stored by is_repository_shallow()) would solve the issue without need for this patch, but this patch is independently useful (as an optimization to prevent writing a file in an unnecessary case), hence why I wrote it. I have included a NEEDSWORK outlining the full solution. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jan 10, 2019 at 11:36 UTC bd0b42aed3084bf66557485fd7d87e975a4f6d4e

3 files changed +34 -2

fetch-pack.c

+9 -2

@@ -1232,6 +1232,8 @@ static int process_acks(struct fetch_negotiator *negotiator,

1232 static void receive_shallow_info(struct fetch_pack_args *args,

1233 struct packet_reader *reader)

1234 {

1235 + int line_received = 0;

1236 +

1237 process_section_header(reader, "shallow-info", 0);

1238 while (packet_reader_read(reader) == PACKET_READ_NORMAL) {

1239 const char *arg;

@@ -1241,6 +1243,7 @@ static void receive_shallow_info(struct fetch_pack_args *args,

1243 if (get_oid_hex(arg, &oid))

1244 die(_("invalid shallow line: %s"), reader->line);

1245 register_shallow(the_repository, &oid);

1246 + line_received = 1;

1247 continue;

1248 }

1249 if (skip_prefix(reader->line, "unshallow ", &arg)) {

@@ -1253,6 +1256,7 @@ static void receive_shallow_info(struct fetch_pack_args *args,

1256 die(_("error in object: %s"), reader->line);

1257 if (unregister_shallow(&oid))

1258 die(_("no shallow found: %s"), reader->line);

1259 + line_received = 1;

1260 continue;

1261 }

1262 die(_("expected shallow/unshallow, got %s"), reader->line);

@@ -1262,8 +1266,11 @@ static void receive_shallow_info(struct fetch_pack_args *args,

1266 reader->status != PACKET_READ_DELIM)

1267 die(_("error processing shallow info: %d"), reader->status);

1268

1265 - setup_alternate_shallow(&shallow_lock, &alternate_shallow_file, NULL);

1266 - args->deepen = 1;

1269 + if (line_received) {

1270 + setup_alternate_shallow(&shallow_lock, &alternate_shallow_file,

1271 + NULL);

1272 + args->deepen = 1;

1273 + }

1274 }

1275

1276 static void receive_wanted_refs(struct packet_reader *reader,

shallow.c

@@ -43,6 +43,13 @@ int register_shallow(struct repository *r, const struct object_id *oid)

44 int is_repository_shallow(struct repository *r)

45 {

46 + /*

47 + * NEEDSWORK: This function updates

48 + * r->parsed_objects->{is_shallow,shallow_stat} as a side effect but

49 + * there is no corresponding function to clear them when the shallow

50 + * file is updated.

51 + */

52 +

53 FILE *fp;

54 char buf[1024];

55 const char *path = r->parsed_objects->alternate_shallow_file;

t/t5702-protocol-v2.sh

+18

@@ -471,6 +471,24 @@ test_expect_success 'upload-pack respects client shallows' '

471 grep "fetch< version 2" trace

472 '

473

474 +test_expect_success 'ensure that multiple fetches in same process from a shallow repo works' '

475 + rm -rf server client trace &&

476 +

477 + test_create_repo server &&

478 + test_commit -C server one &&

479 + test_commit -C server two &&

480 + test_commit -C server three &&

481 + git clone --shallow-exclude two "file://$(pwd)/server" client &&

482 +

483 + git -C server tag -a -m "an annotated tag" twotag two &&

484 +

485 + # Triggers tag following (thus, 2 fetches in one process)

486 + GIT_TRACE_PACKET="$(pwd)/trace" git -C client -c protocol.version=2 \

487 + fetch --shallow-exclude one origin &&

488 + # Ensure that protocol v2 is used

489 + grep "fetch< version 2" trace

490 +'

491 +

492 # Test protocol v2 with 'http://' transport

493 #

494 . "$TEST_DIRECTORY"/lib-httpd.sh