Re: [PATCH] all: new command used for multi-repo operations

[Lars Hjemli <hjemli@gmail.com>]

On Tue, Jan 22, 2013 at 11:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
>> +static struct option builtin_all_options[] = {
>> +     OPT_BOOLEAN('c', "clean", &only_clean, N_("only show clean repositories")),
>> +     OPT_BOOLEAN('d', "dirty", &only_dirty, N_("only show dirty repositories")),
>> +     OPT_END(),
>> +};
>
> Shouldn't this be more like OPT_SET_INT() on a same variable that is
> initialized to "all"?  Alternatively you could validate the input
> and die when both are given.

OPT_SET_INT() seems appropriate, will fix.

>
>> +int cmd_all(int argc, const char **argv, const char *prefix)
>> +{
>> +     struct strbuf path = STRBUF_INIT;
>> +
>> +     if (!getcwd(root, sizeof(root)))
>> +             return 1;
>> +
>> +     argc = parse_options(argc, argv, prefix, builtin_all_options,
>> +                          builtin_all_usage, PARSE_OPT_STOP_AT_NON_OPTION);
>> +
>> +     unsetenv(GIT_DIR_ENVIRONMENT);
>> +     unsetenv(GIT_WORK_TREE_ENVIRONMENT);
>
> Don't you need to clear other variables whose uses are closely tied
> to a single repository, like GIT_INDEX_FILE, etc.?
>
> I suspect that explicitly exporting GIT_DIR and GIT_WORK_TREE (and
> nothing else) in handle_repo() to the location you discovered before
> you run the per-repository command via run_command_v_opt(), might be
> a better alternative.  The user could be sharing objects in all
> repositories by permanently setting GIT_OBJECT_DIRECTORY to a single
> place.
>

This sounds like a nice plan, I'll test it and send an updated patch.


>> diff --git a/command-list.txt b/command-list.txt
>> index 7e8cfec..f955895 100644
>> --- a/command-list.txt
>> +++ b/command-list.txt
>> @@ -1,6 +1,7 @@
>>  # List of known git commands.
>>  # command name                               category [deprecated] [common]
>>  git-add                                 mainporcelain common
>> +git-all                                 mainporcelain
>>  git-am                                  mainporcelain
>>  git-annotate                            ancillaryinterrogators
>>  git-apply                               plumbingmanipulators
>
> I am not very interested in this topic in the first place, but this
> does not (at least not yet) sound like a main Porcelain to me.

There doesn't seem to be a better category, but I'm open for suggestions.

>
> "all" may be a word other people may want to use to call collections
> of things other than "Git repositories", and that use may turn out
> to be more useful in general.  A name that makes it clear that this
> is about "repositories", i.e. along the lines of "git for-each-repo"
> or something, would be a better name that does not squat on such a
> short and sweet name.
>

In principle I agree with your reasoning on this, but in practice I
fail to see what other kind of things `git all` could naturally refer
to. Also, having a short and sweet way to perform the tasks
implemented by this patch was my main motivation for writing it [1],
hence `git for-each-repo` isn't as compelling (too much typing).
There's always aliases, but I'd prefer it if future git supported `git
all` by default.

-- 
larsh

[1] Originally as a shell script, used at $WORK for ~2 years