Re: [PATCHv3 2/3] submodule: implement `module-name` as a builtin helper
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 1, 2015, 02:31 UTC
- Message-ID
- <CAPig+cTTiW_tZBNoz-kj6CkViR+MFegv1Tz5xmVvbGxZ0pbqwQ@mail.gmail.com>
- In-Reply-To
- <1441068029-19158-3-git-send-email-sbeller@google.com>
On Mon, Aug 31, 2015 at 8:40 PM, Stefan Beller <sbeller@google.com> wrote:
Show 30 quoted lines
> This implements the helper `module_name` in C instead of shell,
> yielding a nice performance boost.
>
> Before this patch, I measured a time (best out of three):
>
> $ time ./t7400-submodule-basic.sh >/dev/null
> real 0m11.066s
> user 0m3.348s
> sys 0m8.534s
>
> With this patch applied I measured (also best out of three)
>
> $ time ./t7400-submodule-basic.sh >/dev/null
> real 0m10.063s
> user 0m3.044s
> sys 0m7.487s
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 44310f5..91bd420 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -102,16 +105,38 @@ static int module_list(int argc, const char **argv, const char *prefix)
> +static int module_name(int argc, const char **argv, const char *prefix)
> +{
> + const struct submodule *sub;
> +
> + if (argc != 1)
> + usage("git submodule--helper module_name <path>\n");usage(_("..."));s/module_name/module-name/
I think you also need to drop the newline from the usage() argument.
Show 18 quoted lines
> + gitmodules_config();
> + sub = submodule_from_path(null_sha1, argv[0]);
> +
> + if (!sub)
> + die(N_("No submodule mapping found in .gitmodules for path '%s'"),
> + argv[0]);
> +
> + printf("%s\n", sub->name);
> +
> + return 0;
> +}
> +
> int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> {
> if (argc < 2)
> die(N_("fatal: submodule--helper subcommand must be called with "
> - "a subcommand, which is module-list\n"));
> + "a subcommand, which are module-list, module-name\n"));Manually maintaining this list is likely to become a maintenance issue pretty quickly. Isn't there an OPT_CMDMODE for this sort of thing?
Also, it would like be more readable if the possible commands were printed one per line rather than all squashed together.
Show 9 quoted lines
> if (!strcmp(argv[1], "module-list"))
> return module_list(argc - 1, argv + 1, prefix);
>
> + if (!strcmp(argv[1], "module-name"))
> + return module_name(argc - 2, argv + 2, prefix);
> +
> die(N_("fatal: '%s' is not a valid submodule--helper subcommand, "
> - "which is module-list\n"),
> + "which are module-list, module-name\n"),And, it's duplicated here, making it even more of a maintenance problem.
> argv[1]); > }