@matheusbsbs / matheus-bagliano / commits / 5374a290aa

fetch-pack: write fetched refs to .promisor

The specification of promisor packfiles (in partial-clone.txt) states that the .promisor files that accompany packfiles do not matter (just like .keep files), so whenever a packfile is fetched from the promisor remote, Git has been writing empty .promisor files. But these files could contain more useful information. So instead of writing empty files, write the refs fetched to these files. This makes it easier to debug issues with partial clones, as we can identify what refs (and their associated hashes) were fetched at the time the packfile was downloaded, and if necessary, compare those hashes against what the promisor remote reports now. This is implemented by teaching fetch-pack to write its own non-empty .promisor file whenever it knows the name of the pack's lockfile. This covers the case wherein the user runs "git fetch" with an internal protocol or HTTP protocol v2 (fetch_refs_via_pack() in transport.c sets lock_pack) and with HTTP protocol v0/v1 (fetch_git() in remote-curl.c passes "--lock-pack" to "fetch-pack"). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Acked-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Oct 14, 2019 at 17:12 UTC 5374a290aa56390f9f44547d52f8f30fb2e866aa

3 files changed +58 -4

builtin/repack.c

@@ -233,6 +233,13 @@ static void repack_promisor_objects(const struct pack_objects_args *args,

233 /*

234 * pack-objects creates the .pack and .idx files, but not the

235 * .promisor file. Create the .promisor file, which is empty.

236 + *

237 + * NEEDSWORK: fetch-pack sometimes generates non-empty

238 + * .promisor files containing the ref names and associated

239 + * hashes at the point of generation of the corresponding

240 + * packfile, but this would not preserve their contents. Maybe

241 + * concatenate the contents of all .promisor files instead of

242 + * just creating a new empty file.

243 */

244 promisor_name = mkpathdup("%s-%s.promisor", packtmp,

245 line.buf);

fetch-pack.c

+43 -4

@@ -754,8 +754,33 @@ static int sideband_demux(int in, int out, void *data)

754 return ret;

755 }

756

757 +static void write_promisor_file(const char *keep_name,

758 + struct ref **sought, int nr_sought)

759 +{

760 + struct strbuf promisor_name = STRBUF_INIT;

761 + int suffix_stripped;

762 + FILE *output;

763 + int i;

764 +

765 + strbuf_addstr(&promisor_name, keep_name);

766 + suffix_stripped = strbuf_strip_suffix(&promisor_name, ".keep");

767 + if (!suffix_stripped)

768 + BUG("name of pack lockfile should end with .keep (was '%s')",

769 + keep_name);

770 + strbuf_addstr(&promisor_name, ".promisor");

771 +

772 + output = xfopen(promisor_name.buf, "w");

773 + for (i = 0; i < nr_sought; i++)

774 + fprintf(output, "%s %s\n", oid_to_hex(&sought[i]->old_oid),

775 + sought[i]->name);

776 + fclose(output);

777 +

778 + strbuf_release(&promisor_name);

779 +}

780 +

781 static int get_pack(struct fetch_pack_args *args,

758 - int xd[2], char **pack_lockfile)

782 + int xd[2], char **pack_lockfile,

783 + struct ref **sought, int nr_sought)

784 {

785 struct async demux;

786 int do_keep = args->keep_pack;

@@ -817,7 +842,13 @@ static int get_pack(struct fetch_pack_args *args,

842 }

843 if (args->check_self_contained_and_connected)

844 argv_array_push(&cmd.args, "--check-self-contained-and-connected");

820 - if (args->from_promisor)

845 + /*

846 + * If we're obtaining the filename of a lockfile, we'll use

847 + * that filename to write a .promisor file with more

848 + * information below. If not, we need index-pack to do it for

849 + * us.

850 + */

851 + if (!(do_keep && pack_lockfile) && args->from_promisor)

852 argv_array_push(&cmd.args, "--promisor");

853 }

854 else {

@@ -871,6 +902,14 @@ static int get_pack(struct fetch_pack_args *args,

902 die(_("%s failed"), cmd_name);

903 if (use_sideband && finish_async(&demux))

904 die(_("error in sideband demultiplexer"));

905 +

906 + /*

907 + * Now that index-pack has succeeded, write the promisor file using the

908 + * obtained .keep filename if necessary

909 + */

910 + if (do_keep && pack_lockfile && args->from_promisor)

911 + write_promisor_file(*pack_lockfile, sought, nr_sought);

912 +

913 return 0;

914 }

915

@@ -1006,7 +1045,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,

1045 alternate_shallow_file = setup_temporary_shallow(si->shallow);

1046 else

1047 alternate_shallow_file = NULL;

1009 - if (get_pack(args, fd, pack_lockfile))

1048 + if (get_pack(args, fd, pack_lockfile, sought, nr_sought))

1049 die(_("git fetch-pack: fetch failed."));

1050

1051 all_done:

@@ -1453,7 +1492,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1492

1493 /* get the pack */

1494 process_section_header(&reader, "packfile", 0);

1456 - if (get_pack(args, fd, pack_lockfile))

1495 + if (get_pack(args, fd, pack_lockfile, sought, nr_sought))

1496 die(_("git fetch-pack: fetch failed."));

1497

1498 state = FETCH_DONE;

t/t5616-partial-clone.sh

@@ -46,6 +46,14 @@ test_expect_success 'do partial clone 1' '

46 test "$(git -C pc1 config --local remote.origin.partialclonefilter)" = "blob:none"

47 '

49 +test_expect_success 'verify that .promisor file contains refs fetched' '

50 + ls pc1/.git/objects/pack/pack-*.promisor >promisorlist &&

51 + test_line_count = 1 promisorlist &&

52 + git -C srv.bare rev-list HEAD >headhash &&

53 + grep "$(cat headhash) HEAD" $(cat promisorlist) &&

54 + grep "$(cat headhash) refs/heads/master" $(cat promisorlist)

55 +'

56 +

57 # checkout master to force dynamic object fetch of blobs at HEAD.

58 test_expect_success 'verify checkout with dynamic object fetch' '

59 git -C pc1 rev-list --quiet --objects --missing=print HEAD >observed &&