From 87c01003cdff8c99ebdf053441e4527d85952284 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Wed, 27 Nov 2024 17:33:09 -0600 Subject: bundle: add bundle verification options type When `unbundle()` is invoked, fsck verification may be configured by passing the `VERIFY_BUNDLE_FSCK` flag. This mechanism allows fsck checks on the bundle to be enabled or disabled entirely. To facilitate more fine-grained fsck configuration, additional context must be provided to `unbundle()`. Introduce the `unbundle_opts` type, which wraps the existing `verify_bundle_flags`, to facilitate future extension of `unbundle()` configuration. Also update `unbundle()` and its call sites to accept this new options type instead of the flags directly. The end behavior is functionally the same, but allows for the set of configurable options to be extended. This is leveraged in a subsequent commit to enable fsck message severity configuration. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/bundle.c | 2 +- bundle-uri.c | 7 +++++-- bundle.c | 6 +++++- bundle.h | 10 +++++++--- transport.c | 6 ++++-- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/builtin/bundle.c b/builtin/bundle.c index 127518c2a8..15ac75ab51 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -218,7 +218,7 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) strvec_pushl(&extra_index_pack_args, "-v", "--progress-title", _("Unbundling objects"), NULL); ret = !!unbundle(the_repository, &header, bundle_fd, - &extra_index_pack_args, 0) || + &extra_index_pack_args, NULL) || list_bundle_refs(&header, argc, argv); bundle_header_release(&header); diff --git a/bundle-uri.c b/bundle-uri.c index 0df66e2872..cdf9e4f9e1 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -367,6 +367,10 @@ static int unbundle_from_file(struct repository *r, const char *file) struct string_list_item *refname; struct strbuf bundle_ref = STRBUF_INIT; size_t bundle_prefix_len; + struct unbundle_opts opts = { + .flags = VERIFY_BUNDLE_QUIET | + (fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0), + }; bundle_fd = read_bundle_header(file, &header); if (bundle_fd < 0) { @@ -379,8 +383,7 @@ static int unbundle_from_file(struct repository *r, const char *file) * a reachable ref pointing to the new tips, which will reach * the prerequisite commits. */ - result = unbundle(r, &header, bundle_fd, NULL, - VERIFY_BUNDLE_QUIET | (fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0)); + result = unbundle(r, &header, bundle_fd, NULL, &opts); if (result) { result = 1; goto cleanup; diff --git a/bundle.c b/bundle.c index 4773b51eb1..485033ea3f 100644 --- a/bundle.c +++ b/bundle.c @@ -628,9 +628,13 @@ out: int unbundle(struct repository *r, struct bundle_header *header, int bundle_fd, struct strvec *extra_index_pack_args, - enum verify_bundle_flags flags) + struct unbundle_opts *opts) { struct child_process ip = CHILD_PROCESS_INIT; + enum verify_bundle_flags flags = 0; + + if (opts) + flags = opts->flags; if (verify_bundle(r, header, flags)) return -1; diff --git a/bundle.h b/bundle.h index 5ccc9a061a..6a09cc7bfb 100644 --- a/bundle.h +++ b/bundle.h @@ -39,6 +39,10 @@ enum verify_bundle_flags { int verify_bundle(struct repository *r, struct bundle_header *header, enum verify_bundle_flags flags); +struct unbundle_opts { + enum verify_bundle_flags flags; +}; + /** * Unbundle after reading the header with read_bundle_header(). * @@ -49,12 +53,12 @@ int verify_bundle(struct repository *r, struct bundle_header *header, * (e.g. "-v" for verbose/progress), NULL otherwise. The provided * "extra_index_pack_args" (if any) will be strvec_clear()'d for you. * - * Before unbundling, this method will call verify_bundle() with the - * given 'flags'. + * Before unbundling, this method will call verify_bundle() with 'flags' + * provided in 'opts'. */ int unbundle(struct repository *r, struct bundle_header *header, int bundle_fd, struct strvec *extra_index_pack_args, - enum verify_bundle_flags flags); + struct unbundle_opts *opts); int list_bundle_refs(struct bundle_header *header, int argc, const char **argv); diff --git a/transport.c b/transport.c index 47fda6a773..8536b14181 100644 --- a/transport.c +++ b/transport.c @@ -176,6 +176,9 @@ static int fetch_refs_from_bundle(struct transport *transport, int nr_heads UNUSED, struct ref **to_fetch UNUSED) { + struct unbundle_opts opts = { + .flags = fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0, + }; struct bundle_transport_data *data = transport->data; struct strvec extra_index_pack_args = STRVEC_INIT; int ret; @@ -186,8 +189,7 @@ static int fetch_refs_from_bundle(struct transport *transport, if (!data->get_refs_from_bundle_called) get_refs_from_bundle_inner(transport); ret = unbundle(the_repository, &data->header, data->fd, - &extra_index_pack_args, - fetch_pack_fsck_objects() ? VERIFY_BUNDLE_FSCK : 0); + &extra_index_pack_args, &opts); transport->hash_algo = data->header.hash_algo; strvec_clear(&extra_index_pack_args); -- cgit v1.2.3 From 187574ce869f1244de83fc6a0a5b6d614fe979f2 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Wed, 27 Nov 2024 17:33:10 -0600 Subject: bundle: support fsck message configuration If the `VERIFY_BUNDLE_FLAG` is set during `unbundle()`, the git-index-pack(1) spawned is configured with the `--fsck-options` flag to perform fsck verification. With this flag enabled, there is not a way to configure fsck message severity though. Extend the `unbundle_opts` type to store fsck message severity configuration and update `unbundle()` to conditionally append it to the `--fsck-objects` flag if provided. This enables `unbundle()` call sites to support optionally setting the severity for specific fsck messages. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- bundle.c | 13 +++++++------ bundle.h | 7 +++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index 485033ea3f..4e53ddfca2 100644 --- a/bundle.c +++ b/bundle.c @@ -631,12 +631,12 @@ int unbundle(struct repository *r, struct bundle_header *header, struct unbundle_opts *opts) { struct child_process ip = CHILD_PROCESS_INIT; - enum verify_bundle_flags flags = 0; + struct unbundle_opts opts_fallback = { 0 }; - if (opts) - flags = opts->flags; + if (!opts) + opts = &opts_fallback; - if (verify_bundle(r, header, flags)) + if (verify_bundle(r, header, opts->flags)) return -1; strvec_pushl(&ip.args, "index-pack", "--fix-thin", "--stdin", NULL); @@ -645,8 +645,9 @@ int unbundle(struct repository *r, struct bundle_header *header, if (header->filter.choice) strvec_push(&ip.args, "--promisor=from-bundle"); - if (flags & VERIFY_BUNDLE_FSCK) - strvec_push(&ip.args, "--fsck-objects"); + if (opts->flags & VERIFY_BUNDLE_FSCK) + strvec_pushf(&ip.args, "--fsck-objects%s", + opts->fsck_msg_types ? opts->fsck_msg_types : ""); if (extra_index_pack_args) strvec_pushv(&ip.args, extra_index_pack_args->v); diff --git a/bundle.h b/bundle.h index 6a09cc7bfb..a80aa8ad9b 100644 --- a/bundle.h +++ b/bundle.h @@ -41,6 +41,13 @@ int verify_bundle(struct repository *r, struct bundle_header *header, struct unbundle_opts { enum verify_bundle_flags flags; + /* + * fsck_msg_types may optionally contain fsck message severity + * configuration. If present, this configuration gets directly appended + * to a '--fsck-objects' option and therefore must be prefixed with '='. + * (E.g. "=missingEmail=ignore,gitmodulesUrl=ignore") + */ + const char *fsck_msg_types; }; /** -- cgit v1.2.3 From 05596e93c50b286fa445af8ae572759be079092d Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Wed, 27 Nov 2024 17:33:11 -0600 Subject: fetch-pack: split out fsck config parsing When `fetch_pack_config()` is invoked, fetch-pack configuration is parsed from the config. As part of this operation, fsck message severity configuration is assigned to the `fsck_msg_types` global variable. This is optionally used to configure the downstream git-index-pack(1) when the `--strict` option is specified. The same parsed fsck message severity configuration is also needed outside of fetch-pack. Instead of exposing/relying on the existing global state, split out the fsck config parsing logic into `fetch_pack_fsck_config()` and expose it. In a subsequent commit, this is used to provide fsck configuration when invoking `unbundle()`. For `fetch_pack_fsck_config()` to discern between errors and unhandled config variables, the return code when `git_config_path()` errors is changed to a different value also indicating success. This frees up the previous return code to now indicate the provided config variable was unhandled. The behavior remains functionally the same. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- fetch-pack.c | 26 ++++++++++++++++++-------- fetch-pack.h | 11 +++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index fe1fb3c1b7..c095f3a84b 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1857,8 +1857,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, return ref; } -static int fetch_pack_config_cb(const char *var, const char *value, - const struct config_context *ctx, void *cb) +int fetch_pack_fsck_config(const char *var, const char *value, + struct strbuf *msg_types) { const char *msg_id; @@ -1866,9 +1866,9 @@ static int fetch_pack_config_cb(const char *var, const char *value, char *path ; if (git_config_pathname(&path, var, value)) - return 1; - strbuf_addf(&fsck_msg_types, "%cskiplist=%s", - fsck_msg_types.len ? ',' : '=', path); + return 0; + strbuf_addf(msg_types, "%cskiplist=%s", + msg_types->len ? ',' : '=', path); free(path); return 0; } @@ -1877,14 +1877,24 @@ static int fetch_pack_config_cb(const char *var, const char *value, if (!value) return config_error_nonbool(var); if (is_valid_msg_type(msg_id, value)) - strbuf_addf(&fsck_msg_types, "%c%s=%s", - fsck_msg_types.len ? ',' : '=', msg_id, value); + strbuf_addf(msg_types, "%c%s=%s", + msg_types->len ? ',' : '=', msg_id, value); else warning("Skipping unknown msg id '%s'", msg_id); return 0; } - return git_default_config(var, value, ctx, cb); + return 1; +} + +static int fetch_pack_config_cb(const char *var, const char *value, + const struct config_context *ctx, void *cb) +{ + int ret = fetch_pack_fsck_config(var, value, &fsck_msg_types); + if (ret > 0) + return git_default_config(var, value, ctx, cb); + + return ret; } static void fetch_pack_config(void) diff --git a/fetch-pack.h b/fetch-pack.h index b5c579cdae..9d3470366f 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -106,4 +106,15 @@ int report_unmatched_refs(struct ref **sought, int nr_sought); */ int fetch_pack_fsck_objects(void); +/* + * Check if the provided config variable pertains to fetch fsck and if so append + * the configuration to the provided strbuf. + * + * When a fetch fsck config option is successfully processed the function + * returns 0. If the provided config option is unrelated to fetch fsck, 1 is + * returned. Errors return -1. + */ +int fetch_pack_fsck_config(const char *var, const char *value, + struct strbuf *msg_types); + #endif -- cgit v1.2.3 From baa159137be36965e03192528b001ffc39b8352f Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Wed, 27 Nov 2024 17:33:12 -0600 Subject: transport: propagate fsck configuration during bundle fetch When fetching directly from a bundle, fsck message severity configuration is not propagated to the underlying git-index-pack(1). It is only capable of enabling or disabling fsck checks entirely. This does not align with the fsck behavior for fetches through git-fetch-pack(1). Use the fsck config parsing from fetch-pack to populate fsck message severity configuration and wire it through to `unbundle()` to enable the same fsck verification as done through fetch-pack. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- t/t5607-clone-bundle.sh | 7 +++++++ transport.c | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh index 7ceaa8194d..c69aa88eae 100755 --- a/t/t5607-clone-bundle.sh +++ b/t/t5607-clone-bundle.sh @@ -171,6 +171,13 @@ test_expect_success 'clone bundle with different fsckObjects configurations' ' test_must_fail git -c transfer.fsckObjects=true \ clone bundle-fsck/bad.bundle bundle-transfer-fsck 2>err && + test_grep "missingEmail" err && + + git -c fetch.fsckObjects=true -c fetch.fsck.missingEmail=ignore \ + clone bundle-fsck/bad.bundle bundle-fsck-ignore && + + test_must_fail git -c fetch.fsckObjects=true -c fetch.fsck.missingEmail=error \ + clone bundle-fsck/bad.bundle bundle-fsck-error 2>err && test_grep "missingEmail" err ' diff --git a/transport.c b/transport.c index 8536b14181..6966df51a8 100644 --- a/transport.c +++ b/transport.c @@ -19,6 +19,7 @@ #include "branch.h" #include "url.h" #include "submodule.h" +#include "strbuf.h" #include "string-list.h" #include "oid-array.h" #include "sigchain.h" @@ -172,6 +173,19 @@ static struct ref *get_refs_from_bundle(struct transport *transport, return result; } +static int fetch_fsck_config_cb(const char *var, const char *value, + const struct config_context *ctx UNUSED, void *cb) +{ + struct strbuf *msg_types = cb; + int ret; + + ret = fetch_pack_fsck_config(var, value, msg_types); + if (ret > 0) + return 0; + + return ret; +} + static int fetch_refs_from_bundle(struct transport *transport, int nr_heads UNUSED, struct ref **to_fetch UNUSED) @@ -181,6 +195,7 @@ static int fetch_refs_from_bundle(struct transport *transport, }; struct bundle_transport_data *data = transport->data; struct strvec extra_index_pack_args = STRVEC_INIT; + struct strbuf msg_types = STRBUF_INIT; int ret; if (transport->progress) @@ -188,11 +203,16 @@ static int fetch_refs_from_bundle(struct transport *transport, if (!data->get_refs_from_bundle_called) get_refs_from_bundle_inner(transport); + + git_config(fetch_fsck_config_cb, &msg_types); + opts.fsck_msg_types = msg_types.buf; + ret = unbundle(the_repository, &data->header, data->fd, &extra_index_pack_args, &opts); transport->hash_algo = data->header.hash_algo; strvec_clear(&extra_index_pack_args); + strbuf_release(&msg_types); return ret; } -- cgit v1.2.3