Re: [PATCH] all: new command used for multi-repo operations
- From
- Lars Hjemli <hjemli@gmail.com>
- Date
- Jan 22, 2013, 23:35 UTC
- Message-ID
- <CAFXTnz62H24G9Zp5EgG47MyNdDz0KddX2n34G_1-RV_yP-ELkA@mail.gmail.com>
- In-Reply-To
- <7vmww0nahd.fsf@alter.siamese.dyndns.org>
On Tue, Jan 22, 2013 at 11:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> 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.
Show 24 quoted lines
>
>> +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.
Show 15 quoted lines
>> 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.
Show 8 quoted lines
> > "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