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

[Junio C Hamano <gitster@pobox.com>]

Lars Hjemli <hjemli@gmail.com> writes:

> diff --git a/builtin/all.c b/builtin/all.c
> new file mode 100644
> index 0000000..ee9270d
> --- /dev/null
> +++ b/builtin/all.c
> @@ -0,0 +1,105 @@
> +/*
> + * "git all" builtin command.
> + *
> + * Copyright (c) 2013 Lars Hjemli <hjemli@gmail.com>
> + */
> +#include "cache.h"
> +#include "color.h"
> +#include "builtin.h"
> +#include "run-command.h"
> +#include "parse-options.h"
> +
> +static int only_dirty;
> +static int only_clean;
> +char root[PATH_MAX];
> +
> +static const char * const builtin_all_usage[] = {
> +	N_("git all [options] [cmd]"),
> +	NULL
> +};
> +
> +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.

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

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

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