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 <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 79fb6735e1e3a3af6cae7deb9eb8c3061e7389c2..7ffe489f6a1a24e497365edf405df01ff3e271b2 100644 (file)
--- 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 74bfa2e9ba060d193747416fec565b56cf597e71..bbbf6fa4228cd741d9725156dd49f3b521941202 100755 (executable)
--- 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 <stdin &&
+       test_path_is_symlink .git/TEST_SYMREF_HEAD &&
+       test "$(test_readlink .git/TEST_SYMREF_HEAD)" = refs/heads/new
+'
+
 test_done