[v2] help: do not expect built-in commands to be hardlinked

Commit Message

Philippe Blain via GitGitGadget Oct. 7, 2020, 9:56 p.m. UTC

From: Johannes Schindelin <[e-mail]>

When building with SKIP_DASHED_BUILT_INS=YesPlease, the built-in
commands are no longer present in the `PATH` as hardlinks to `git`.

As a consequence, `load_command_list()` needs to be taught to find the
names of the built-in commands from elsewhere.

This only affected the output of `git --list-cmds=main`, but not the
output of `git help -a` because the latter includes the built-in
commands by virtue of them being listed in command-list.txt.

The bug was detected via a patch series that turns the merge strategies
included in Git into built-in commands: `git merge -s help` relies on
`load_command_list()` to determine the list of available merge
strategies.

Signed-off-by: Johannes Schindelin <[e-mail]>
---
    Fix the command list with SKIP_DASHED_BUILT_INS=YesPlease
    
    In a recent patch series
    [https://lore.kernel.org/git/[e-mail]/#r]
    , the merge strategies were converted into built-ins, which is good.
    
    Together with the change where we stop hard-linking the built-in
    commands in CI builds, this broke t9902.199.
    
    The actual root cause is that git merge -s help relies on 
    load_command_list() to find all available Git commands, and that
    function expected to find the built-in commands to on the PATH.
    
    Changes since v1:
    
     * Clarified the prefix skipping.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-745%2Fdscho%2Falways-include-builtins-in-commands-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-745/dscho/always-include-builtins-in-commands-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/745

Range-diff vs v1:

 1:  74adb00d59 ! 1:  c36a97f436 help: do not expect built-in commands to be hardlinked
     @@ git.c: static void list_builtins(struct string_list *out, unsigned int exclude_o
      +	const char *name;
      +	int i;
      +
     ++	/*
     ++	 * Callers can ask for a subset of the commands based on a certain
     ++	 * prefix, which is then dropped from the added names. The names in
     ++	 * the `commands[]` array do not have the `git-` prefix, though,
     ++	 * therefore we must expect the `prefix` to at least start with `git-`.
     ++	 */
      +	if (!skip_prefix(prefix, "git-", &prefix))
     -+		return;
     ++		BUG("prefix '%s' must start with 'git-'", prefix);
      +
      +	for (i = 0; i < ARRAY_SIZE(commands); i++)
      +		if (skip_prefix(commands[i].cmd, prefix, &name))


 git.c  | 19 +++++++++++++++++++
 help.c |  2 ++
 help.h |  1 +
 3 files changed, 22 insertions(+)


base-commit: 8f7759d2c8c13716bfdb9ae602414fd987787e8d

Patch

diff --git a/git.c b/git.c
index d51fb5d2bf..48fc81b92f 100644
--- a/git.c
+++ b/git.c
@@ -641,6 +641,25 @@  static void list_builtins(struct string_list *out, unsigned int exclude_option)
 	}
 }
 
+void load_builtin_commands(const char *prefix, struct cmdnames *cmds)
+{
+	const char *name;
+	int i;
+
+	/*
+	 * Callers can ask for a subset of the commands based on a certain
+	 * prefix, which is then dropped from the added names. The names in
+	 * the `commands[]` array do not have the `git-` prefix, though,
+	 * therefore we must expect the `prefix` to at least start with `git-`.
+	 */
+	if (!skip_prefix(prefix, "git-", &prefix))
+		BUG("prefix '%s' must start with 'git-'", prefix);
+
+	for (i = 0; i < ARRAY_SIZE(commands); i++)
+		if (skip_prefix(commands[i].cmd, prefix, &name))
+			add_cmdname(cmds, name, strlen(name));
+}
+
 #ifdef STRIP_EXTENSION
 static void strip_extension(const char **argv)
 {
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);
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);