@matheusbsbs / matheus-bagliano / commits / 5476e1efde

fetch-pack: print and use dangling .gitmodules

Teach index-pack to print dangling .gitmodules links after its "keep" or "pack" line instead of declaring an error, and teach fetch-pack to check such lines printed. This allows the tree side of the .gitmodules link to be in one packfile and the blob side to be in another without failing the fsck check, because it is now fetch-pack which checks such objects after all packfiles have been downloaded and indexed (and not index-pack on an individual packfile, as it is before this commit). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Feb 22, 2021 at 11:20 UTC 5476e1efded571e374cd97c7d69f17962ba1c44f

9 files changed +165 -22

Documentation/git-index-pack.txt

+6 -1

@@ -78,7 +78,12 @@ OPTIONS

78 Die if the pack contains broken links. For internal use only.

80 --fsck-objects::

81 - Die if the pack contains broken objects. For internal use only.

81 + For internal use only.

82 ++

83 +Die if the pack contains broken objects. If the pack contains a tree

84 +pointing to a .gitmodules blob that does not exist, prints the hash of

85 +that blob (for the caller to check) after the hash that goes into the

86 +name of the pack/idx file (see "Notes").

88 --threads=<n>::

89 Specifies the number of threads to spawn when resolving

builtin/index-pack.c

+23 -2

@@ -1693,6 +1693,22 @@ static void show_pack_info(int stat_only)

1693 }

1694 }

1695

1696 +static int print_dangling_gitmodules(struct fsck_options *o,

1697 + const struct object_id *oid,

1698 + enum object_type object_type,

1699 + int msg_type, const char *message)

1700 +{

1701 + /*

1702 + * NEEDSWORK: Plumb the MSG_ID (from fsck.c) here and use it

1703 + * instead of relying on this string check.

1704 + */

1705 + if (starts_with(message, "gitmodulesMissing")) {

1706 + printf("%s\n", oid_to_hex(oid));

1707 + return 0;

1708 + }

1709 + return fsck_error_function(o, oid, object_type, msg_type, message);

1710 +}

1711 +

1712 int cmd_index_pack(int argc, const char **argv, const char *prefix)

1713 {

1714 int i, fix_thin_pack = 0, verify = 0, stat_only = 0;

@@ -1888,8 +1904,13 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)

1904 else

1905 close(input_fd);

1906

1891 - if (do_fsck_object && fsck_finish(&fsck_options))

1892 - die(_("fsck error in pack objects"));

1907 + if (do_fsck_object) {

1908 + struct fsck_options fo = fsck_options;

1909 +

1910 + fo.error_func = print_dangling_gitmodules;

1911 + if (fsck_finish(&fo))

1912 + die(_("fsck error in pack objects"));

1913 + }

1914

1915 free(objects);

1916 strbuf_release(&index_name_buf);

builtin/receive-pack.c

+1 -1

@@ -2275,7 +2275,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)

2275 status = start_command(&child);

2276 if (status)

2277 return "index-pack fork failed";

2278 - pack_lockfile = index_pack_lockfile(child.out);

2278 + pack_lockfile = index_pack_lockfile(child.out, NULL);

2279 close(child.out);

2280 status = finish_command(&child);

2281 if (status)

fetch-pack.c

+66 -12

@@ -796,6 +796,26 @@ static void write_promisor_file(const char *keep_name,

796 strbuf_release(&promisor_name);

797 }

798

799 +static void parse_gitmodules_oids(int fd, struct oidset *gitmodules_oids)

800 +{

801 + int len = the_hash_algo->hexsz + 1; /* hash + NL */

802 +

803 + do {

804 + char hex_hash[GIT_MAX_HEXSZ + 1];

805 + int read_len = read_in_full(fd, hex_hash, len);

806 + struct object_id oid;

807 + const char *end;

808 +

809 + if (!read_len)

810 + return;

811 + if (read_len != len)

812 + die("invalid length read %d", read_len);

813 + if (parse_oid_hex(hex_hash, &oid, &end) || *end != '\n')

814 + die("invalid hash");

815 + oidset_insert(gitmodules_oids, &oid);

816 + } while (1);

817 +}

818 +

819 /*

820 * If packfile URIs were provided, pass a non-NULL pointer to index_pack_args.

821 * The strings to pass as the --index-pack-arg arguments to http-fetch will be

@@ -804,7 +824,8 @@ static void write_promisor_file(const char *keep_name,

824 static int get_pack(struct fetch_pack_args *args,

825 int xd[2], struct string_list *pack_lockfiles,

826 struct strvec *index_pack_args,

807 - struct ref **sought, int nr_sought)

827 + struct ref **sought, int nr_sought,

828 + struct oidset *gitmodules_oids)

829 {

830 struct async demux;

831 int do_keep = args->keep_pack;

@@ -812,6 +833,7 @@ static int get_pack(struct fetch_pack_args *args,

833 struct pack_header header;

834 int pass_header = 0;

835 struct child_process cmd = CHILD_PROCESS_INIT;

836 + int fsck_objects = 0;

837 int ret;

838

839 memset(&demux, 0, sizeof(demux));

@@ -846,8 +868,15 @@ static int get_pack(struct fetch_pack_args *args,

868 strvec_push(&cmd.args, alternate_shallow_file);

869 }

870

849 - if (do_keep || args->from_promisor || index_pack_args) {

850 - if (pack_lockfiles)

871 + if (fetch_fsck_objects >= 0

872 + ? fetch_fsck_objects

873 + : transfer_fsck_objects >= 0

874 + ? transfer_fsck_objects

875 + : 0)

876 + fsck_objects = 1;

877 +

878 + if (do_keep || args->from_promisor || index_pack_args || fsck_objects) {

879 + if (pack_lockfiles || fsck_objects)

880 cmd.out = -1;

881 cmd_name = "index-pack";

882 strvec_push(&cmd.args, cmd_name);

@@ -897,11 +926,7 @@ static int get_pack(struct fetch_pack_args *args,

926 strvec_pushf(&cmd.args, "--pack_header=%"PRIu32",%"PRIu32,

927 ntohl(header.hdr_version),

928 ntohl(header.hdr_entries));

900 - if (fetch_fsck_objects >= 0

901 - ? fetch_fsck_objects

902 - : transfer_fsck_objects >= 0

903 - ? transfer_fsck_objects

904 - : 0) {

929 + if (fsck_objects) {

930 if (args->from_promisor || index_pack_args)

931 /*

932 * We cannot use --strict in index-pack because it

@@ -925,10 +950,15 @@ static int get_pack(struct fetch_pack_args *args,

950 cmd.git_cmd = 1;

951 if (start_command(&cmd))

952 die(_("fetch-pack: unable to fork off %s"), cmd_name);

928 - if (do_keep && pack_lockfiles) {

929 - char *pack_lockfile = index_pack_lockfile(cmd.out);

953 + if (do_keep && (pack_lockfiles || fsck_objects)) {

954 + int is_well_formed;

955 + char *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed);

956 +

957 + if (!is_well_formed)

958 + die(_("fetch-pack: invalid index-pack output"));

959 if (pack_lockfile)

960 string_list_append_nodup(pack_lockfiles, pack_lockfile);

961 + parse_gitmodules_oids(cmd.out, gitmodules_oids);

962 close(cmd.out);

963 }

964

@@ -963,6 +993,22 @@ static int cmp_ref_by_name(const void *a_, const void *b_)

993 return strcmp(a->name, b->name);

994 }

995

996 +static void fsck_gitmodules_oids(struct oidset *gitmodules_oids)

997 +{

998 + struct oidset_iter iter;

999 + const struct object_id *oid;

1000 + struct fsck_options fo = FSCK_OPTIONS_STRICT;

1001 +

1002 + if (!oidset_size(gitmodules_oids))

1003 + return;

1004 +

1005 + oidset_iter_init(gitmodules_oids, &iter);

1006 + while ((oid = oidset_iter_next(&iter)))

1007 + register_found_gitmodules(oid);

1008 + if (fsck_finish(&fo))

1009 + die("fsck failed");

1010 +}

1011 +

1012 static struct ref *do_fetch_pack(struct fetch_pack_args *args,

1013 int fd[2],

1014 const struct ref *orig_ref,

@@ -977,6 +1023,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,

1023 int agent_len;

1024 struct fetch_negotiator negotiator_alloc;

1025 struct fetch_negotiator *negotiator;

1026 + struct oidset gitmodules_oids = OIDSET_INIT;

1027

1028 negotiator = &negotiator_alloc;

1029 fetch_negotiator_init(r, negotiator);

@@ -1092,8 +1139,10 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,

1139 alternate_shallow_file = setup_temporary_shallow(si->shallow);

1140 else

1141 alternate_shallow_file = NULL;

1095 - if (get_pack(args, fd, pack_lockfiles, NULL, sought, nr_sought))

1142 + if (get_pack(args, fd, pack_lockfiles, NULL, sought, nr_sought,

1143 + &gitmodules_oids))

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

1145 + fsck_gitmodules_oids(&gitmodules_oids);

1146

1147 all_done:

1148 if (negotiator)

@@ -1544,6 +1593,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1593 struct string_list packfile_uris = STRING_LIST_INIT_DUP;

1594 int i;

1595 struct strvec index_pack_args = STRVEC_INIT;

1596 + struct oidset gitmodules_oids = OIDSET_INIT;

1597

1598 negotiator = &negotiator_alloc;

1599 fetch_negotiator_init(r, negotiator);

@@ -1634,7 +1684,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

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

1685 if (get_pack(args, fd, pack_lockfiles,

1686 packfile_uris.nr ? &index_pack_args : NULL,

1637 - sought, nr_sought))

1687 + sought, nr_sought, &gitmodules_oids))

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

1689 do_check_stateless_delimiter(args, &reader);

1690

@@ -1677,6 +1727,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1727

1728 packname[the_hash_algo->hexsz] = '\0';

1729

1730 + parse_gitmodules_oids(cmd.out, &gitmodules_oids);

1731 +

1732 close(cmd.out);

1733

1734 if (finish_command(&cmd))

@@ -1696,6 +1748,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,

1748 string_list_clear(&packfile_uris, 0);

1749 strvec_clear(&index_pack_args);

1750

1751 + fsck_gitmodules_oids(&gitmodules_oids);

1752 +

1753 if (negotiator)

1754 negotiator->release(negotiator);

1755

fsck.c

@@ -1243,6 +1243,11 @@ int fsck_error_function(struct fsck_options *o,

1243 return 1;

1244 }

1245

1246 +void register_found_gitmodules(const struct object_id *oid)

1247 +{

1248 + oidset_insert(&gitmodules_found, oid);

1249 +}

1250 +

1251 int fsck_finish(struct fsck_options *options)

1252 {

1253 int ret = 0;

fsck.h

@@ -62,6 +62,8 @@ int fsck_walk(struct object *obj, void *data, struct fsck_options *options);

62 int fsck_object(struct object *obj, void *data, unsigned long size,

63 struct fsck_options *options);

65 +void register_found_gitmodules(const struct object_id *oid);

66 +

67 /*

68 * Some fsck checks are context-dependent, and may end up queued; run this

69 * after completing all fsck_object() calls in order to resolve any remaining

pack-write.c

+7 -1

@@ -272,7 +272,7 @@ void fixup_pack_header_footer(int pack_fd,

272 fsync_or_die(pack_fd, pack_name);

273 }

274

275 -char *index_pack_lockfile(int ip_out)

275 +char *index_pack_lockfile(int ip_out, int *is_well_formed)

276 {

277 char packname[GIT_MAX_HEXSZ + 6];

278 const int len = the_hash_algo->hexsz + 6;

@@ -286,11 +286,17 @@ char *index_pack_lockfile(int ip_out)

286 */

287 if (read_in_full(ip_out, packname, len) == len && packname[len-1] == '\n') {

288 const char *name;

289 +

290 + if (is_well_formed)

291 + *is_well_formed = 1;

292 packname[len-1] = 0;

293 if (skip_prefix(packname, "keep\t", &name))

294 return xstrfmt("%s/pack/pack-%s.keep",

295 get_object_directory(), name);

296 + return NULL;

297 }

298 + if (is_well_formed)

299 + *is_well_formed = 0;

300 return NULL;

301 }

302

pack.h

+1 -1

@@ -85,7 +85,7 @@ int verify_pack_index(struct packed_git *);

85 int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);

86 off_t write_pack_header(struct hashfile *f, uint32_t);

87 void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);

88 -char *index_pack_lockfile(int fd);

88 +char *index_pack_lockfile(int fd, int *is_well_formed);

90 /*

91 * The "hdr" output buffer should be at least this big, which will handle sizes

t/t5702-protocol-v2.sh

+54 -4

@@ -847,8 +847,9 @@ test_expect_success 'part of packfile response provided as URI' '

847 test -f hfound &&

848 test -f h2found &&

849

850 - # Ensure that there are exactly 6 files (3 .pack and 3 .idx).

851 - ls http_child/.git/objects/pack/* >filelist &&

850 + # Ensure that there are exactly 3 packfiles with associated .idx

851 + ls http_child/.git/objects/pack/*.pack \

852 + http_child/.git/objects/pack/*.idx >filelist &&

853 test_line_count = 6 filelist

854 '

855

@@ -901,8 +902,9 @@ test_expect_success 'packfile-uri with transfer.fsckobjects' '

902 -c fetch.uriprotocols=http,https \

903 clone "$HTTPD_URL/smart/http_parent" http_child &&

904

904 - # Ensure that there are exactly 4 files (2 .pack and 2 .idx).

905 - ls http_child/.git/objects/pack/* >filelist &&

905 + # Ensure that there are exactly 2 packfiles with associated .idx

906 + ls http_child/.git/objects/pack/*.pack \

907 + http_child/.git/objects/pack/*.idx >filelist &&

908 test_line_count = 4 filelist

909 '

910

@@ -936,6 +938,54 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails on bad object'

938 test_i18ngrep "invalid author/committer line - missing email" error

939 '

940

941 +test_expect_success 'packfile-uri with transfer.fsckobjects succeeds when .gitmodules is separate from tree' '

942 + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&

943 + rm -rf "$P" http_child &&

944 +

945 + git init "$P" &&

946 + git -C "$P" config "uploadpack.allowsidebandall" "true" &&

947 +

948 + echo "[submodule libfoo]" >"$P/.gitmodules" &&

949 + echo "path = include/foo" >>"$P/.gitmodules" &&

950 + echo "url = git://example.com/git/lib.git" >>"$P/.gitmodules" &&

951 + git -C "$P" add .gitmodules &&

952 + git -C "$P" commit -m x &&

953 +

954 + configure_exclusion "$P" .gitmodules >h &&

955 +

956 + sane_unset GIT_TEST_SIDEBAND_ALL &&

957 + git -c protocol.version=2 -c transfer.fsckobjects=1 \

958 + -c fetch.uriprotocols=http,https \

959 + clone "$HTTPD_URL/smart/http_parent" http_child &&

960 +

961 + # Ensure that there are exactly 2 packfiles with associated .idx

962 + ls http_child/.git/objects/pack/*.pack \

963 + http_child/.git/objects/pack/*.idx >filelist &&

964 + test_line_count = 4 filelist

965 +'

966 +

967 +test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodules separate from tree is invalid' '

968 + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&

969 + rm -rf "$P" http_child err &&

970 +

971 + git init "$P" &&

972 + git -C "$P" config "uploadpack.allowsidebandall" "true" &&

973 +

974 + echo "[submodule \"..\"]" >"$P/.gitmodules" &&

975 + echo "path = include/foo" >>"$P/.gitmodules" &&

976 + echo "url = git://example.com/git/lib.git" >>"$P/.gitmodules" &&

977 + git -C "$P" add .gitmodules &&

978 + git -C "$P" commit -m x &&

979 +

980 + configure_exclusion "$P" .gitmodules >h &&

981 +

982 + sane_unset GIT_TEST_SIDEBAND_ALL &&

983 + test_must_fail git -c protocol.version=2 -c transfer.fsckobjects=1 \

984 + -c fetch.uriprotocols=http,https \

985 + clone "$HTTPD_URL/smart/http_parent" http_child 2>err &&

986 + test_i18ngrep "disallowed submodule name" err

987 +'

988 +

989 # DO NOT add non-httpd-specific tests here, because the last part of this

990 # test script is only executed when httpd is available and enabled.

991