Subject: Re: [PATCH 2/8] refs: extract out reflog config to generic layer

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 22 Apr 2026 13:15:30 +0200

On Mon, Apr 20, 2026 at 12:12:00PM +0200, Karthik Nayak wrote:
> The reference backends need to know when to create reflog entries, this

s/this/which/

> is dictated by the 'core.logallrefupdates' config. Instead of relying on

s/dictated/controlled/

> the backends to call `repo_settings_get_log_all_ref_updates()` to obtain
> this config value, let's do this in the generic layer and pass down the
> value to the backends.
> 
> Instead of passing this in as a new argument, let's create a new
> `ref_init_options` structure which will house information required to
> initialize a reference backend. Move the access flags here as well.

I agree with this direction. It's also something that I'm doing for many
callbacks in the ODB layer, and I'm moving more and more into that
direction.

> diff --git a/refs.c b/refs.c
> index bfcb9c7ac3..aa66c6b28e 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2295,6 +2295,10 @@ static struct ref_store *ref_store_init(struct repository *repo,
>  {
>  	const struct ref_storage_be *be;
>  	struct ref_store *refs;
> +	struct ref_store_init_options options = {
> +		.access_flags = flags,
> +		.log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo),
> +	};
>  
>  	be = find_ref_storage_backend(format);
>  	if (!be)

Tiniest nit, please feel free to ignore: we often call the structure
itself `_options`, but the variables just `opts`. May just be my own
preference though.

> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 2d963cc4f4..eed13af4eb 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -385,6 +385,21 @@ struct ref_store;
>  				 REF_STORE_ODB | \
>  				 REF_STORE_MAIN)
>  
> +/*
> + * Options for initializing the ref backend. All backend-agnostic information
> + * which backends required will be held here.
> + */
> +struct ref_store_init_options {
> +	/* The kind of operations that the ref_store is allowed to perform. */
> +	unsigned int access_flags;
> +
> +	/*
> +	 * Denotes under what conditions reflogs should be created when updating
> +	 * references.
> +	 */
> +	enum log_refs_config log_all_ref_updates;
> +};

Nit: it might've made sense to split this up into two steps: the
introduction of the struct, and then moving the config in there.

Patrick