On 8/25/2020 6:19 PM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> +SYNOPSIS >> +-------- >> +[verse] >> +'git for-each-repo' --config=<config> [--] <arguments> >> + ... >> +--config=<config>:: >> + Use the given config variable as a multi-valued list storing >> + absolute path names. > > Would it make sense to allow this config to be read from the current > repository, I wonder. It is probably designed to be written to > either ~/.gitconfig or /etc/gitconfig because it is probably a need > that is not per-repository to list repositories for various purposes > specified by the config key, but I suspect there _might_ be a good > use case for storing some custom list of repositories in the > configuration file local to a repository, but it is not quite > obvious what it is. > > If we have a good example, we may want to spell it out---that would > help future readers who wonder about this (just like I am doing now). > > Also, if we do read from local config, should there be a way to say > "ah, you may have read values from /etc/gitconfig and ~/.gitconfig, > but please forget them---I have a full list I care when you are > running in this repository", i.e. clear the list. It is purely a > convention and there is no built-in mechanism for this in the config > API, but often it is signalled by giving an empty string as a value. I guess I should test this, but if I ask for a multi-valued config, will I not get _all_ of the results from /etc/gitconfig, ~/.gitconfig, AND .git/config? That was my expectation, which is why I don't specify "local" or "global" config anywhere in the discussion. > By the way, I do not have a good concrete suggestion, but can we use > something better than <config> as the placeholder? I first thought > this was naming the name of a file that lists repositories, not the > config variable name in our usual config namespace. Sure. How about "<key>"? >> +static int run_command_on_repo(const char *path, >> + void *cbdata) > > Is that on repo or in repo? When I saw "-C" on the command line, I > immediately thought of "in repo". "in" is better. >> +{ >> + int i; >> + struct child_process child = CHILD_PROCESS_INIT; >> + struct strvec *args = (struct strvec *)cbdata; >> + >> + child.git_cmd = 1; >> + strvec_pushl(&child.args, "-C", path, NULL); >> + >> + for (i = 0; i < args->nr; i++) >> + strvec_push(&child.args, args->v[i]); > > Would strvec_pushv() work, or is args->v[] not NULL terminated? Yeah, pushv should work. >> + return run_command(&child); >> +} > > >> + values = repo_config_get_value_multi(the_repository, >> + config_key); > > Not your fault, but it is a bit unsatisfactory that we do not have > special "type" meant for paths in the config API, unlike the > parse-options API where there is a "filename" type that is a bit > richer than a vanilla "string" type by allowing "prefix" handling. > For the purposes of this, as the values are limited to absolute/full > pathnames, it does not hurt as much, though. Interesting. Noted. Thanks, -Stolee