Re: [PATCH v5 10/11] refs/reftable: lazy-load configuration to fix chicken-and-egg

[Justin Tobler <jltobler@gmail.com>]

On 26/06/22 10:28AM, Patrick Steinhardt wrote:
> Same as with the "files" backend, the "reftable" backend also has a
> chicken-and-egg problem with "onbranch" conditions. Fix this issue the
> same as we did with the "files" backend by lazy-loading configuration.

Makes sense.

> Now that both the "files" and the "reftable" backend handle this
> properly, add a generic test to t1400 that verifies that the user can
> configure "core.logAllRefUpdates" via an "onbranch" condition. This is
> mostly a nonsensical thing to do in the first place, but it serves as a
> good sanity chekc.

s/chekc/check

> Note that we had to move `should_write_log()` around so that it can
> access the new `reftable_be_write_options()` function.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  refs/reftable-backend.c           | 146 ++++++++++++++++++++++----------------
>  t/t0613-reftable-write-options.sh |  19 +++++
>  t/t1400-update-ref.sh             |  12 ++++
>  3 files changed, 116 insertions(+), 61 deletions(-)
> 
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 608d71cf10..d74131a5ae 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -141,10 +141,21 @@ struct reftable_ref_store {
>  	 */
>  	struct strmap worktree_backends;
>  	struct reftable_stack_options stack_options;
> -	struct reftable_write_options write_options;
> +
> +	/*
> +	 * Options used when writing to or compacting the reftable stacks.
> +	 * These are parsed from the configuration lazily on first use via
> +	 * `reftable_be_write_options()` so that we don't have to access the
> +	 * configuration when initializing the ref store. Do not access these
> +	 * fields directly, but use the accessor instead.
> +	 */
> +	struct reftable_be_write_options {
> +		struct reftable_write_options opts;
> +		enum log_refs_config log_all_ref_updates;

Any reason in particular that `log_all_ref_updates` is the only option
outside of `struct reftlable_write_options` here? Isn't it also only
used during writes?

-Justin