On Mon, Dec 20 2021, Johannes Schindelin wrote:
> Hi Sean,
>
> On Sat, 18 Dec 2021, Sean Allred wrote:
>
>> I've got a custom subcommand I'm distributing in my company to integrate
>> with our bug-tracker. It's a pretty robust utility and has its own help
>> function, but running `git foo --help` doesn't pass --help to my git-foo
>> utility. I asked a question[1] about this scenario on the Windows fork
>> and they directed me upstream.
>>
>> It sounds like `git foo --help` is internally consumed as `git help
>> foo`, which forwards requests to info/man/web handlers per config.
>> Being on Windows and knowing my peers as I do, the vast majority of my
>> users won't be familiar with info or man. The HTML documentation used
>> by the web handler is in a Git4Win-controlled installation directory
>> that I'd really rather not touch/maintain. I really just want `git foo
>> --help` to call `git-foo --help`.
>>
>> What's the best way to go about this?
>>
>> In the event the best next step is to start a patch, does it sound
>> reasonable to simply not perform this `git foo --help` -> `git help
>> foo` transformation for non-builtins? Or, while I don't relish the
>> idea, would some kind of config option be needed?
>
> I think you might need to be a bit more careful than just looking whether
> the command in question is a built-in or not. It could be delivered as a
> script or executable inside `libexec/git-core`. So maybe check that,
> something like this:
>
> -- snip --
> diff --git a/git.c b/git.c
> index c802dfe98004..d609f90cc117 100644
> --- a/git.c
> +++ b/git.c
> @@ -688,6 +688,33 @@ static void strip_extension(const char **argv)
> #define strip_extension(cmd)
> #endif
>
> +static int is_in_git_exec_path(const char *command_name)
> +{
> + struct strbuf path = STRBUF_INIT;
> + int ret = 0;
> +
> + if (!command_name)
> + return 0;
> +
> + strbuf_addf(&path, "%s/git-%s", git_exec_path(), command_name);
> + ret = !access(path.buf, X_OK);
> +
> +#ifdef STRIP_EXTENSION
> + if (!ret) {
> + /*
> + * If `command_name` ended in `.exe`, strip it, otherwise
> + * append it.
> + */
> + if (!strbuf_strip_suffix(&path, STRIP_EXTENSION))
> + strbuf_addstr(&path, STRIP_EXTENSION);
> + ret = !access(path.buf, X_OK);
> + }
> +#endif
> +
> + strbuf_release(&path);
> + return ret;
> +}
> +
> static void handle_builtin(int argc, const char **argv)
> {
> struct strvec args = STRVEC_INIT;
> @@ -697,8 +724,11 @@ static void handle_builtin(int argc, const char **argv)
> strip_extension(argv);
> cmd = argv[0];
>
> + builtin = get_builtin(cmd);
> +
> /* Turn "git cmd --help" into "git help --exclude-guides cmd" */
> - if (argc > 1 && !strcmp(argv[1], "--help")) {
> + if (argc > 1 && !strcmp(argv[1], "--help") &&
> + (builtin || is_in_git_exec_path(argv[0]))) {
> int i;
>
> argv[1] = argv[0];
> @@ -714,7 +744,6 @@ static void handle_builtin(int argc, const char **argv)
> argv = args.v;
> }
>
> - builtin = get_builtin(cmd);
> if (builtin)
> exit(run_builtin(builtin, argc, argv));
> strvec_clear(&args);
> -- snap --
>
> Of course, this might break existing users' setups where they ship a Git
> command together with a manual page.
>
> A potential remedy against that would be, as you say, a config option.
> Maybe defaulting to the manual page if `help.format` is `man`, otherwise
> defaulting to passing `--help` to the command.
What are the cases that require us to inexpect our --exec-path at
runtime, as opposed to having a list of commands we know we put there at
"install" time?
The only ones I can think of are e.g. Debian's packaging which might
compile the git with "git-send-email", but it won't be there unless you
install "git-email" in addition to "git".
But for those cases any such logic would presumably want the hardcoded
full list over the dynamic access() check, since e.g. "git-doc" on that
platform orthagonally installs "git-send-email.html" and the like, and
"git help send-email" would presumably like to error saying that we know
about git-send-email, we just can't find its documentation, even if we
can't find it in --exec-path.
