On Thu, Feb 09 2023, Jeff King wrote: > If the gpg code used git_config_get_string(), etc, then they could just > access each key on demand (efficiently, from an internal hash table), > which reduces the risk of "oops, we forgot to initialize the config > here". It does probably mean restructuring the code a little, though > (since you'd often have an accessor function to get "foo.bar" rather > than assuming "foo.bar" was parsed into an enum already, etc). That may > not be worth the effort (and risk of regression) to convert. I'd already played around with that a bit as part of reviewing Junio's change, this goes on top of that. I found that continuing this conversion was getting harder, but these 3 cases really were trivial cases where we're just reading a variable globally, and then proceeding to use it in one specific place. Out of the remaining ones gpg.program et all looked easiest, but I didn't continue with it. For anyone interested think it would be best to continue by converting the remaining bits by having commit, tag etc. set up some "struct gpg", so that when they could directly instruct it ot do its config reading before parse_options(). The remaining complexity is mainly with the file-global & having to juggle in what order we read & set what. FWIW when poking at this I found that we have fairly robust testing support for this area, but it could be better, but it's good enough to spot that if we stop reading these we'll fail tests. But e.g. for the "gpg.program" we've got tests that'll fail if the "gpg" program variable isn't read, but not for the "ssh" variable, but as they'll both share the same/similar reader code any future migration should spot any glaring bugs, just possibly not subtle ones. Branch & passing[1] CI at: https://github.com/avar/git/tree/avar/gpg-lazy-init-configset 1. Well, passing except for the general current Windows CI dumpster fire on topics based off current "master". Ævar Arnfjörð Bjarmason (2): {am,commit-tree,verify-{commit,tag}}: refactor away config wrapper gpg-interface.c: lazily get GPG config variables on demand builtin/am.c | 7 +---- builtin/commit-tree.c | 7 +---- builtin/verify-commit.c | 7 +---- builtin/verify-tag.c | 7 +---- gpg-interface.c | 66 ++++++++++++++++------------------------- 5 files changed, 29 insertions(+), 65 deletions(-) -- 2.39.1.1475.gc2542cdc5ef
