From d20dd4fd44d8f7bc1e570871ab50c56b08f6b3c1 Mon Sep 17 00:00:00 2001 From: Friel Date: Sat, 29 Aug 2026 23:12:48 +0000 Subject: [PATCH] fetch-pack: defer .gitattributes checks for packfile URIs With fetch.fsckObjects or transfer.fsckObjects enabled, index-pack checks each pack as soon as it has been installed. Some checks need objects from another pack, though. A tree in the inline pack can name a .gitattributes blob that arrives in a packfile URI response. index-pack then reports the missing blob before fetch-pack has downloaded the URI pack. The same split already works for .gitmodules. index-pack writes any unresolved .gitmodules object IDs after its pack result. fetch-pack collects those IDs and runs fsck_finish() after all packs have been installed. Extend that handoff to .gitattributes. Prefix .gitattributes records so fetch-pack can distinguish them while leaving the existing .gitmodules output unchanged. The existing fsck_finish() call then checks both sets after every packfile URI has been indexed. Test that two concurrently indexed URI packs accept a valid split .gitattributes file and reject an overlong one. Signed-off-by: Friel --- Documentation/git-index-pack.adoc | 8 +++-- builtin/index-pack.c | 3 +- fetch-pack.c | 57 +++++++++++++++++-------------- fsck.c | 23 ++++++++----- fsck.h | 11 +++--- t/t5702-protocol-v2.sh | 45 ++++++++++++++++++++++++ 6 files changed, 103 insertions(+), 44 deletions(-) diff --git a/Documentation/git-index-pack.adoc b/Documentation/git-index-pack.adoc index 0076d6c183c220..3b4833b8fd1b04 100644 --- a/Documentation/git-index-pack.adoc +++ b/Documentation/git-index-pack.adoc @@ -107,9 +107,11 @@ default and "Indexing objects" when `--stdin` is specified. --fsck-objects[==...]:: Die if the pack contains broken objects, but unlike `--strict`, don't choke on broken links. If the pack contains a tree pointing to a - .gitmodules blob that does not exist, prints the hash of that blob - (for the caller to check) after the hash that goes into the name of the - pack/idx file (see "Notes"). + .gitmodules or .gitattributes blob that does not exist, prints a record + for that blob (for the caller to check) after the hash that goes into + the name of the pack/idx file (see "Notes"). The record for a + .gitmodules blob is its hash. The record for a .gitattributes blob is + `gitattributes` followed by a space and its hash. + An optional comma-separated list of `=` can be passed to change the severity of some possible issues, e.g., diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 4028b8ce3ed2f8..625486b10f7f2a 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1919,7 +1919,8 @@ int cmd_index_pack(int argc, disable_replace_refs(); - fsck_options_init(&fsck_options, the_repository, FSCK_OPTIONS_MISSING_GITMODULES); + fsck_options_init(&fsck_options, the_repository, + FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES); fsck_options.walk = mark_link; reset_pack_idx_option(&opts); diff --git a/fetch-pack.c b/fetch-pack.c index b6925c2238dfd2..e3906226dacd6c 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -923,24 +923,29 @@ static void create_promisor_file(const char *keep_name, strbuf_release(&promisor_name); } -static void parse_gitmodules_oids(int fd, struct oidset *gitmodules_oids) +static void parse_deferred_fsck_oids(int fd, struct fsck_options *options) { - int len = the_hash_algo->hexsz + 1; /* hash + NL */ + struct strbuf line = STRBUF_INIT; - do { - char hex_hash[GIT_MAX_HEXSZ + 1]; - int read_len = read_in_full(fd, hex_hash, len); + while (1) { struct object_id oid; - const char *end; + const char *end, *hex; + struct oidset *found = &options->gitmodules_found; + int ret = strbuf_getwholeline_fd(&line, fd, '\n'); - if (!read_len) - return; - if (read_len != len) - die("invalid length read %d", read_len); - if (parse_oid_hex(hex_hash, &oid, &end) || *end != '\n') + if (ret == EOF) { + if (!line.len) + break; + die("invalid fsck object ID"); + } + hex = line.buf; + if (skip_prefix(hex, "gitattributes ", &hex)) + found = &options->gitattributes_found; + if (parse_oid_hex(hex, &oid, &end) || *end != '\n') die("invalid hash"); - oidset_insert(gitmodules_oids, &oid); - } while (1); + oidset_insert(found, &oid); + } + strbuf_release(&line); } static void add_index_pack_keep_option(struct strvec *args) @@ -963,7 +968,7 @@ static int get_pack(struct fetch_pack_args *args, struct strvec *index_pack_args, int no_ref_delta, struct ref **sought, int nr_sought, - struct oidset *gitmodules_oids) + struct fsck_options *fsck_options) { struct async demux; int do_keep = args->keep_pack; @@ -1091,7 +1096,7 @@ static int get_pack(struct fetch_pack_args *args, string_list_append_nodup(pack_lockfiles, pack_lockfile); else free(pack_lockfile); - parse_gitmodules_oids(cmd.out, gitmodules_oids); + parse_deferred_fsck_oids(cmd.out, fsck_options); close(cmd.out); } @@ -1269,9 +1274,10 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, } else alternate_shallow_file = NULL; - fsck_options_init(&fsck_options, the_repository, FSCK_OPTIONS_MISSING_GITMODULES); + fsck_options_init(&fsck_options, the_repository, + FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES); if (get_pack(args, fd, pack_lockfiles, NULL, 0, sought, nr_sought, - &fsck_options.gitmodules_found)) + &fsck_options)) die(_("git fetch-pack: fetch failed.")); if (fsck_finish(&fsck_options)) die("fsck failed"); @@ -1743,7 +1749,7 @@ static void start_packfile_uri_task( } static void finish_packfile_uri_task(struct packfile_uri_task *task, - struct oidset *gitmodules_oids) + struct fsck_options *fsck_options) { char packhash[GIT_MAX_HEXSZ + 1]; struct object_id oid; @@ -1759,7 +1765,7 @@ static void finish_packfile_uri_task(struct packfile_uri_task *task, if (get_oid_hex(packhash, &oid)) die("fetch-pack: expected hash then LF in http-fetch output"); - parse_gitmodules_oids(task->cmd.out, gitmodules_oids); + parse_deferred_fsck_oids(task->cmd.out, fsck_options); close(task->cmd.out); task->cmd.out = -1; @@ -1772,7 +1778,7 @@ static void finish_packfile_uri_task(struct packfile_uri_task *task, static void fetch_packfile_uris_parallel( struct string_list *uris, struct strvec *index_pack_args, - struct string_list *pack_lockfiles, struct oidset *gitmodules_oids) + struct string_list *pack_lockfiles, struct fsck_options *fsck_options) { size_t task_nr = uris->nr; struct packfile_uri_task *tasks; @@ -1813,7 +1819,7 @@ static void fetch_packfile_uris_parallel( if (ready == task_nr) BUG("poll returned without a ready http-fetch"); - finish_packfile_uri_task(&tasks[ready], gitmodules_oids); + finish_packfile_uri_task(&tasks[ready], fsck_options); if (next < uris->nr) { start_packfile_uri_task(&tasks[ready], uris->items[next].string, @@ -1872,7 +1878,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, struct strvec index_pack_args = STRVEC_INIT; const char *promisor_remote_config; - fsck_options_init(&fsck_options, the_repository, FSCK_OPTIONS_MISSING_GITMODULES); + fsck_options_init(&fsck_options, the_repository, + FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES); if (server_feature_v2("promisor-remote", &promisor_remote_config)) promisor_remote_reply(promisor_remote_config, NULL); @@ -2020,7 +2027,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, if (get_pack(args, fd, pack_lockfiles, packfile_uris.nr ? &index_pack_args : NULL, no_ref_delta, - sought, nr_sought, &fsck_options.gitmodules_found)) + sought, nr_sought, &fsck_options)) die(_("git fetch-pack: fetch failed.")); do_check_stateless_delimiter(args->stateless_rpc, &reader); @@ -2044,7 +2051,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, strvec_push(&index_pack_args, "--threads=1"); fetch_packfile_uris_parallel(&packfile_uris, &index_pack_args, pack_lockfiles, - &fsck_options.gitmodules_found); + &fsck_options); goto packfile_uris_done; } @@ -2081,7 +2088,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, die("fetch-pack: expected hash then LF in http-fetch output"); packhash[the_hash_algo->hexsz] = '\0'; - parse_gitmodules_oids(cmd.out, &fsck_options.gitmodules_found); + parse_deferred_fsck_oids(cmd.out, &fsck_options); close(cmd.out); diff --git a/fsck.c b/fsck.c index 94c8651c7dfa28..72991b44a54a45 100644 --- a/fsck.c +++ b/fsck.c @@ -1399,13 +1399,14 @@ void fsck_options_init(struct fsck_options *options, .gitattributes_done = OIDSET_INIT, .error_func = fsck_objects_error_function, }, - [FSCK_OPTIONS_MISSING_GITMODULES] = { + [FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES] = { .strict = 1, .gitmodules_found = OIDSET_INIT, .gitmodules_done = OIDSET_INIT, .gitattributes_found = OIDSET_INIT, .gitattributes_done = OIDSET_INIT, - .error_func = fsck_objects_error_cb_print_missing_gitmodules, + .error_func = + fsck_objects_error_cb_print_missing_gitmodules_and_gitattributes, }, [FSCK_OPTIONS_REFS] = { .error_func = fsck_refs_error_function, @@ -1415,7 +1416,7 @@ void fsck_options_init(struct fsck_options *options, switch (type) { case FSCK_OPTIONS_DEFAULT: case FSCK_OPTIONS_STRICT: - case FSCK_OPTIONS_MISSING_GITMODULES: + case FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES: case FSCK_OPTIONS_REFS: memcpy(options, &defaults[type], sizeof(*options)); break; @@ -1472,17 +1473,21 @@ int git_fsck_config(const char *var, const char *value, * Custom error callbacks that are used in more than one place. */ -int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, - void *fsck_report, - enum fsck_msg_type msg_type, - enum fsck_msg_id msg_id, - const char *message) +int fsck_objects_error_cb_print_missing_gitmodules_and_gitattributes( + struct fsck_options *o, void *fsck_report, + enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, + const char *message) { + struct fsck_object_report *report = fsck_report; + if (msg_id == FSCK_MSG_GITMODULES_MISSING) { - struct fsck_object_report *report = fsck_report; puts(oid_to_hex(report->oid)); return 0; } + if (msg_id == FSCK_MSG_GITATTRIBUTES_MISSING) { + printf("gitattributes %s\n", oid_to_hex(report->oid)); + return 0; + } return fsck_objects_error_function(o, fsck_report, msg_type, msg_id, message); } diff --git a/fsck.h b/fsck.h index e77935c8a9ff6e..ee4e876821674b 100644 --- a/fsck.h +++ b/fsck.h @@ -145,11 +145,10 @@ int fsck_objects_error_function(struct fsck_options *o, void *fsck_report, enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, const char *message); -int fsck_objects_error_cb_print_missing_gitmodules(struct fsck_options *o, - void *fsck_report, - enum fsck_msg_type msg_type, - enum fsck_msg_id msg_id, - const char *message); +int fsck_objects_error_cb_print_missing_gitmodules_and_gitattributes( + struct fsck_options *o, void *fsck_report, + enum fsck_msg_type msg_type, enum fsck_msg_id msg_id, + const char *message); int fsck_refs_error_function(struct fsck_options *options, void *fsck_report, @@ -233,7 +232,7 @@ bool fsck_has_queued_checks(struct fsck_options *options); enum fsck_options_type { FSCK_OPTIONS_DEFAULT, FSCK_OPTIONS_STRICT, - FSCK_OPTIONS_MISSING_GITMODULES, + FSCK_OPTIONS_MISSING_GITMODULES_AND_GITATTRIBUTES, FSCK_OPTIONS_REFS, }; diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 0b87bf4779cd6f..58c780909b2424 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -1517,6 +1517,51 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul test_grep "disallowed submodule name" err ' +test_expect_success 'parallel packfile URIs defer valid .gitattributes fsck' ' + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && + rm -rf "$P" http_child && + + git init "$P" && + git -C "$P" config uploadpack.allowsidebandall true && + git -C "$P" config uploadpack.allowNoRefDelta true && + + echo "*.txt text" >"$P/.gitattributes" && + echo other >"$P/other" && + git -C "$P" add .gitattributes other && + git -C "$P" commit -m x && + configure_exclusion "$P" .gitattributes >/dev/null && + configure_exclusion "$P" other >/dev/null && + + sane_unset GIT_TEST_SIDEBAND_ALL && + git -c protocol.version=2 -c transfer.fsckobjects=1 \ + -c fetch.uriprotocols=http,https \ + -c fetch.packfileUriJobs=2 \ + clone "$HTTPD_URL/smart/http_parent" http_child +' + +test_expect_success 'parallel packfile URIs reject invalid .gitattributes' ' + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && + rm -rf "$P" http_child err && + + git init "$P" && + git -C "$P" config uploadpack.allowsidebandall true && + git -C "$P" config uploadpack.allowNoRefDelta true && + + printf "pattern %02048d" 1 >"$P/.gitattributes" && + echo other >"$P/other" && + git -C "$P" add .gitattributes other && + git -C "$P" commit -m x && + configure_exclusion "$P" .gitattributes >/dev/null && + configure_exclusion "$P" other >/dev/null && + + sane_unset GIT_TEST_SIDEBAND_ALL && + test_must_fail git -c protocol.version=2 -c transfer.fsckobjects=1 \ + -c fetch.uriprotocols=http,https \ + -c fetch.packfileUriJobs=2 \ + clone "$HTTPD_URL/smart/http_parent" http_child 2>err && + test_grep "gitattributes has too long lines" err +' + test_expect_success 'packfile-uri path redacted in trace' ' P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && rm -rf "$P" http_child log &&