From: Patrick Steinhardt Date: Thu, 25 Jun 2026 09:20:06 +0000 (+0200) Subject: refs/files: lazy-load configuration to fix chicken-and-egg X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b450e45b2f10f68e0c1e3954f1d05e7158a52ab;p=thirdparty%2Fgit.git refs/files: lazy-load configuration to fix chicken-and-egg When initializing the "files" reference backend we read the repository's config to parse "core.preferSymlinkRefs" and "core.logAllRefUpdates". This results in a chicken-and-egg problem though, because parsing the configuration may require us to have access to the reference store already when an "onbranch" condition exists. Luckily, all the configuration that we honor only relates to writing references. Consequently, we don't strictly need that configuration to be readily available at initialization time, and we can easiliy defer parsing it to a later point in time. Implement this fix and add tests that verify that we can indeed properly parse these config knobs via an "onbranch" condition. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- diff --git a/refs/files-backend.c b/refs/files-backend.c index 79fb6735e1..7ffe489f6a 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -84,12 +84,21 @@ struct files_ref_store { unsigned int store_flags; char *gitcommondir; - enum log_refs_config log_all_ref_updates; - int prefer_symlink_refs; - struct ref_cache *loose; - struct ref_store *packed_ref_store; + + /* + * Options used when writing references. These are parsed from the + * config lazily on first use via `files_ref_store_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 files_ref_store_write_options { + enum log_refs_config log_all_ref_updates; + int prefer_symlink_refs; + bool initialized; + } write_opts_lazy_loaded; }; static void clear_loose_ref_cache(struct files_ref_store *refs) @@ -121,17 +130,31 @@ static int files_ref_store_config(const char *var, const char *value, const struct config_context *ctx UNUSED, void *payload) { - struct files_ref_store *refs = payload; + struct files_ref_store_write_options *opts = payload; if (!strcmp(var, "core.prefersymlinkrefs")) { - refs->prefer_symlink_refs = git_config_bool(var, value); + opts->prefer_symlink_refs = git_config_bool(var, value); } else if (!strcmp(var, "core.logallrefupdates")) { - refs->log_all_ref_updates = refs_parse_log_all_ref_updates_config(value); + opts->log_all_ref_updates = refs_parse_log_all_ref_updates_config(value); } return 0; } +static const struct files_ref_store_write_options *files_ref_store_write_options(struct files_ref_store *refs) +{ + struct files_ref_store_write_options *opts = &refs->write_opts_lazy_loaded; + + if (opts->initialized) + return opts; + + opts->log_all_ref_updates = LOG_REFS_UNSET; + repo_config(refs->base.repo, files_ref_store_config, opts); + + opts->initialized = true; + return opts; +} + /* * Create a new submodule ref cache and add it to the internal * set of caches. @@ -156,9 +179,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo, refs->packed_ref_store = packed_ref_store_init(repo, NULL, refs->gitcommondir, opts); refs->store_flags = opts->access_flags; - refs->log_all_ref_updates = LOG_REFS_UNSET; - repo_config(repo, files_ref_store_config, refs); chdir_notify_register(NULL, files_ref_store_reparent, refs); strbuf_release(&refdir); @@ -1890,7 +1911,7 @@ static int log_ref_setup(struct files_ref_store *refs, const char *refname, int force_create, int *logfd, struct strbuf *err) { - enum log_refs_config log_refs_cfg = refs->log_all_ref_updates; + enum log_refs_config log_refs_cfg = files_ref_store_write_options(refs)->log_all_ref_updates; struct strbuf logfile_sb = STRBUF_INIT; char *logfile; @@ -3301,6 +3322,7 @@ static int files_transaction_finish(struct ref_store *ref_store, { struct files_ref_store *refs = files_downcast(ref_store, 0, "ref_transaction_finish"); + const struct files_ref_store_write_options *write_opts = files_ref_store_write_options(refs); size_t i; int ret = 0; struct strbuf sb = STRBUF_INIT; @@ -3340,7 +3362,7 @@ static int files_transaction_finish(struct ref_store *ref_store, * We try creating a symlink, if that succeeds we continue to the * next update. If not, we try and create a regular symref. */ - if (update->new_target && refs->prefer_symlink_refs) + if (update->new_target && write_opts->prefer_symlink_refs) /* * By using the `NOT_CONSTANT()` trick, we can avoid * errors by `clang`'s `-Wunreachable` logic that would diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh index 74bfa2e9ba..bbbf6fa422 100755 --- a/t/t0600-reffiles-backend.sh +++ b/t/t0600-reffiles-backend.sh @@ -519,4 +519,25 @@ test_expect_success 'symref transaction supports false symlink config' ' test_cmp expect actual ' +test_expect_success SYMLINKS,!MINGW,!WITH_BREAKING_CHANGES 'core.preferSymlinkRefs can be set up via onbranch condition' ' + test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD" && + test_when_finished "rm -f .git/include" && + git update-ref refs/heads/new @ && + cat >.git/include <<-\EOF && + [core] + preferSymlinkRefs = true + EOF + test_config includeIf.onbranch:"$(git branch --show-current)".path \ + "$(pwd)/.git/include" && + cat >stdin <<-EOF && + start + symref-create TEST_SYMREF_HEAD refs/heads/new + prepare + commit + EOF + git update-ref --no-deref --stdin