All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Alban Gruin <alban.gruin@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH] help: do not expect built-in commands to be hardlinked
Date: Wed, 07 Oct 2020 12:43:09 +0000	[thread overview]
Message-ID: <pull.745.git.1602074589460.gitgitgadget@gmail.com> (raw)

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
    Fix the command list with SKIP_DASHED_BUILT_INS=YesPlease
    
    In a recent patch series
    [https://lore.kernel.org/git/20201005122646.27994-12-alban.gruin@gmail.com/#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 had the long-standing bug that it expects the built-in commands
    to be available in the PATH.

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

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

diff --git a/git.c b/git.c
index d51fb5d2bf..a6224badce 100644
--- a/git.c
+++ b/git.c
@@ -641,6 +641,19 @@ 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;
+
+	if (!skip_prefix(prefix, "git-", &prefix))
+		return;
+
+	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);

base-commit: 8f7759d2c8c13716bfdb9ae602414fd987787e8d
-- 
gitgitgadget

             reply	other threads:[~2020-10-07 12:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 12:43 Johannes Schindelin via GitGitGadget [this message]
2020-10-07 17:21 ` [PATCH] help: do not expect built-in commands to be hardlinked Junio C Hamano
2020-10-07 17:48   ` Junio C Hamano
2020-10-07 21:49     ` Johannes Schindelin
2020-10-07 22:24       ` Junio C Hamano
2020-10-07 21:43   ` Johannes Schindelin
2020-10-07 21:56 ` [PATCH v2] " Johannes Schindelin via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.745.git.1602074589460.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=alban.gruin@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.