Re: [PATCH v2 1/2] attr: add attr.tree for setting the treeish to read attributes from

> 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.

> 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).