Re: [PATCH v4 0/5] config: introduce discovery.bare and protected config

Jonathan Tan <jonathantanmy@google.com> writes:

Makes sense.

I suppose that the idea behind this is that we only parse and store each config file exactly once. It's a good goal, but the whole point of the configset is that we can query a single struct to figure out the value of a config variable. Having multiple configsets starts to shift more of the burden to the callers because they now have to query multiple configsets to find their desired config value, and we already start to see some of this unpleasantness in this series.

An alternative that I'd been thinking about is to make a few changes to the git_config_* + configset API to allow us to use a single configset for all of our needs:

1. Keep track of what config we've read when reading into
   the_repository->config, i.e. instead of a boolean "all config has
   been [un]read", we can express "system and global config has been
   read, but not local or command config". Then, use this information to
   load config from sources as they become available. This will allow us
   to read incomplete config for trace2 and setup.c (discovery.bare and
   safe.directory), and only read what we need later on.

   This assumes that when Git reads config, that config is always valid
   later on. So this is broken if, e.g. we read global config file A
   during setup, but when we discover the repo, we discard A and read
   global config file B instead. I don't know if we do this or if we are
   planning to in the future.

2. Add an additional argument that specifies what scopes to respect when
   reading config (maybe as a set of flags). This gives us extra
   specificity when using the git_config*() functions, so we could get
   rid of git_protected_config() like so:

    /* Change enum config_scope into flags first... */

    #define WIP_SCOPES_PROTECTED = CONFIG_SCOPE_SYSTEM & \
      CONFIG_SCOPE_GLOBAL & CONFIG_SCOPE_COMMAND

    static enum discovery_bare_allowed get_discovery_bare(void)
    {
      enum discovery_bare_allowed result = DISCOVERY_BARE_ALWAYS;
      git_config(discovery_bare_cb, &result, WIP_SCOPES_PROTECTED);
      return result;
    }

   And as an added bonus, this gives us an easy way to implement the
   constant time git_config_*() functions for protected config. We could
   even do this without doing 1. first. I haven't looked into whether
   we could turn the enum into flags, but otherwise, I think this is
   pretty feasible.