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>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2] help: do not expect built-in commands to be hardlinked
Date: Wed, 07 Oct 2020 21:56:51 +0000 [thread overview]
Message-ID: <pull.745.v2.git.1602107811507.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.745.git.1602074589460.gitgitgadget@gmail.com>
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 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(+)
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);
base-commit: 8f7759d2c8c13716bfdb9ae602414fd987787e8d
--
gitgitgadget
prev parent reply other threads:[~2020-10-07 21:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 12:43 [PATCH] help: do not expect built-in commands to be hardlinked Johannes Schindelin via GitGitGadget
2020-10-07 17:21 ` 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 ` Johannes Schindelin via GitGitGadget [this message]
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.v2.git.1602107811507.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=alban.gruin@gmail.com \
--cc=git@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).