[v3,0/4] propagate fsck message severity for bundle fetch

Message

Justin Tobler Nov. 27, 2024, 11:33 p.m. UTC

Greetings,

With 63d903ff52 (unbundle: extend object verification for fetches,
2024-06-19), fsck checks are now performed on fetched bundles depending
on `transfer.fsckObjects` and `fetch.fsckObjects` configuration. This
works, but provides no means to override the default fsck message
severity as is done for other git-fetch(1) operations. This series aims
to propagate fsck message severity configuration to the underlying
git-index-pack(1) process executed on the bundle in a similar manner as
done with git-fetch-pack(1).

  - Patches 1 and 2 adapt the bundle subsystem to support additional
    options when performing `unbundle()`.

  - Patch 3 adapts the fetch-pack subsystem to expose a means to
    generate the message configuration arguments.

  - Patch 4 wires the newly generated fsck configuration options to the
    bundle options used when fetching from a bundle.

Changes in V3:

  - The `fetch_pack_fsck_config()` has been updated to now return 1 when
    the provided config variable is undhandled instead of returning -1.
    This allows call sites to properly differentiate between errors and
    unhandled config variables. To make this change, the handling of
    `git_config_path()` errors was updated to return 0 instead of 1.
    Both of these return values considered by `git_config()` to be a
    success so there is no functional change. This allows returning 1 to
    now indicate that the config variable was not process only.

  - Added comment to document expected `fetch_pack_fsck_config()`
    behavior and return values.

  - Small comment style change.

Thanks
-Justin

Justin Tobler (4):
  bundle: add bundle verification options type
  bundle: support fsck message configuration
  fetch-pack: split out fsck config parsing
  transport: propagate fsck configuration during bundle fetch

 builtin/bundle.c        |  2 +-
 bundle-uri.c            |  7 +++++--
 bundle.c                | 13 +++++++++----
 bundle.h                | 17 ++++++++++++++---
 fetch-pack.c            | 26 ++++++++++++++++++--------
 fetch-pack.h            | 11 +++++++++++
 t/t5607-clone-bundle.sh |  7 +++++++
 transport.c             | 26 ++++++++++++++++++++++++--
 8 files changed, 89 insertions(+), 20 deletions(-)

Range-diff against v2:
1:  da47f0aa0f = 1:  da47f0aa0f bundle: add bundle verification options type
2:  19e91c9f99 ! 2:  5dbd0fa6b7 bundle: support fsck message configuration
    @@ bundle.h: 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 '='.
3:  527874e73d ! 3:  b8db9af9e7 fetch-pack: split out fsck config parsing
    @@ Commit message
         `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 <[e-mail]>
     
      ## fetch-pack.c ##
    @@ fetch-pack.c: static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
      	const char *msg_id;
      
     @@ fetch-pack.c: static int fetch_pack_config_cb(const char *var, const char *value,
    + 		char *path ;
      
      		if (git_config_pathname(&path, var, value))
    - 			return 1;
    +-			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);
    @@ fetch-pack.c: static int fetch_pack_config_cb(const char *var, const char *value
      	}
      
     -	return git_default_config(var, value, ctx, cb);
    -+	return -1;
    ++	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)
    ++	if (ret > 0)
     +		return git_default_config(var, value, ctx, cb);
     +
     +	return ret;
    @@ fetch-pack.h: 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);
     +
4:  b1a3f73561 ! 4:  cc8ae0a1c4 transport: propagate fsck configuration during bundle fetch
    @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport
     +	int ret;
     +
     +	ret = fetch_pack_fsck_config(var, value, msg_types);
    -+	if (ret < 0)
    ++	if (ret > 0)
     +		return 0;
     +
     +	return ret;

base-commit: 4083a6f05206077a50af7658bedc17a94c54607d