Junio C Hamano <gitster@xxxxxxxxx> writes:
> ... in the new world order, what we see on disk plus what
> we have in the built-in table are the set of subcommands available
> to us, and the rule that was valid in the old world order can no
> longer be relied upon, and nobody noticed the breakage while
> developing or reviewing.
>> diff --git a/help.c b/help.c
>> index 4e2468a44d..919cbb9206 100644
>> --- a/help.c
>> +++ b/help.c
>> @@ -263,6 +263,8 @@ void load_command_list(const char *prefix,
>> const char *env_path = getenv("PATH");
>> const char *exec_path = git_exec_path();
>>
>> + load_builtin_commands(prefix, main_cmds);
>> +
>> if (exec_path) {
>> list_commands_in_dir(main_cmds, exec_path, prefix);
>> QSORT(main_cmds->names, main_cmds->cnt, cmdname_compare);
I wondered if we need, after this change, to worry about duplicates,
because some Git subcommands, even after they made into a built-in
and callable internally, must have on-disk footprint.
It turns out that after the post-context in this hunk we do make a
call to uniq(main_cmds) so it is fine.
This was unexpected to me, as we read only from a single directory
"exec_path" and the need to call uniq() in the old world order would
have meant that readdir in exec_path gave us duplicate entries.
In fact, the very original version of load_command_list() did not
have this unnecessary call to uniq(). It was introduced in 1f08e5ce
(Allow git help work without PATH set, 2008-08-28); perhaps Alex saw
12 years into the future and predicted that we would start needing
it ;-)
In any case, the patch is good thanks to that existing uniq() call.
>> diff --git a/help.h b/help.h
>> index dc02458855..5871e93ba2 100644
>> --- a/help.h
>> +++ b/help.h
>> @@ -32,6 +32,7 @@ const char *help_unknown_cmd(const char *cmd);
>> void load_command_list(const char *prefix,
>> struct cmdnames *main_cmds,
>> struct cmdnames *other_cmds);
>> +void load_builtin_commands(const char *prefix, struct cmdnames *cmds);
>> void add_cmdname(struct cmdnames *cmds, const char *name, int len);
>> /* Here we require that excludes is a sorted list. */
>> void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
>>
>> base-commit: 8f7759d2c8c13716bfdb9ae602414fd987787e8d
