Junio C Hamano <gitster@xxxxxxxxx> writes: > As this adds a new call to git_config_get_string(), which will only > be available by including <config.h>, a merge-fix into 'seen' of > this topic needs to revert what b1bda751 (parse: separate out > parsing functions from config.h, 2023-09-29) did, which made this > file include only <parse.h>. > > As this configuration variable was invented to improve the way the > attribute source tree is supported by emulating how mailmap.blob is > done, it deserves a bit of comparison. > > The way mailmap.c does this is not have any code that reads or > parses configuration in mailmap.c (which is a rather library-ish > place), and leaves it up to callers to pre-populate the global > variable git_mailmap_blob with config.c:git_default_config(). That > way, they do not need to include <config.h> (nor <parse.h>) that is > closer to the UI layer. I am wondering why we are not doing the > same, and instead making an ad-hoc call to git_config_get_string() > in this code, and if it is a good direction to move the codebase to > (in which case we may want to make sure that the same pattern is > followed in other places). > > Folks interested in libification, as to the direction of that > effort, what's your plan on where to draw a line between "library" > and "userland"? Should library-ish code be allowed to call > git_config_anything()? I somehow suspect that it might be cleaner > if they didn't, and instead have the user of the "attr" module to > supply the necessary values from outside. I think that ideally library-ish code shouldn't be allowed to call config, yes. However I think what's practical would be for libraries that use very few config variables to get the necessary values from outside, and libraries that use many config variables (e.g. fetch, if it becomes a library) to call config. > On the other hand, once the part we have historically called > "config" API gets a reasonably solid abstraction so that they become > pluggable and replaceable, random ad-hoc calls from library code > outside the "config" library code may not be a huge problem, as long > as we plumb the necessary object handles around (so "attr" library > would need to be told which "config" backend is in use, probably in > the form of a struct that holds the various states in to replace > the current use of globals, plus a vtable to point at > implementations of the "config" service, and git_config_get_string() > call in such a truly libified world would grab the value of the named > variable transparently from whichever "config" backend is currently > in use). This is true, but if we were ever to use the attr library elsewhere (whether in the git.git repo itself to unit test this library, or in another software project), we would need to supply a mock/stub of config. If attr uses very few config variables, I think it's clearer if it takes in the information from outside.
