Re: [PATCH] all: new command used for multi-repo operations
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 22, 2013, 22:01 UTC
- Message-ID
- <7vmww0nahd.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1358889019-4554-1-git-send-email-hjemli@gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
Show 31 quoted lines
> 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.
Show 12 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.
Show 12 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.
"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.