From: Danh Doan <congdanhqx@gmail.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v13 1/5] help: move list_config_help to builtin/help
Date: Fri, 17 Apr 2020 09:04:26 +0700 [thread overview]
Message-ID: <20200417020426.GD2285@danh.dev> (raw)
In-Reply-To: <20200416211807.60811-2-emilyshaffer@google.com>
On 2020-04-16 14:18:03-0700, Emily Shaffer <emilyshaffer@google.com> wrote:
> Starting in 3ac68a93fd2, help.o began to depend on builtin/branch.o,
> builtin/clean.o, and builtin/config.o. This meant that help.o was
> unusable outside of the context of the main Git executable.
>
> To make help.o usable by other commands again, move list_config_help()
> into builtin/help.c (where it makes sense to assume other builtin libraries
> are present).
>
> When command-list.h is included but a member is not used, we start to
> hear a compiler warning. Since the config list is generated in a fairly
> different way than the command list, and since commands and config
> options are semantically different, move the config list into its own
> header and move the generator into its own script and build rule.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
>
> msvc: the bugreport topic depends on a generated config-list.h file
>
> For reasons explained in 976aaedc (msvc: add a Makefile target to
> pre-generate the Visual Studio solution, 2019-07-29), some build
> artifacts we consider non-source files cannot be generated in the
> Visual Studio environment, and we already have some Makefile tweaks
> to help Visual Studio to use generated command-list.h header file.
>
> As this topic starts to depend on another such generated header file,
> config-list.h, let's do the same to it.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> .gitignore | 1 +
> Makefile | 13 +++++--
> builtin/help.c | 86 ++++++++++++++++++++++++++++++++++++++++++
> compat/vcbuild/README | 4 +-
> config.mak.uname | 6 +--
> generate-cmdlist.sh | 19 ----------
> generate-configlist.sh | 21 +++++++++++
> help.c | 85 -----------------------------------------
> help.h | 1 -
> 9 files changed, 123 insertions(+), 113 deletions(-)
> create mode 100755 generate-configlist.sh
>
> diff --git a/.gitignore b/.gitignore
> index 188bd1c3de..61bf5142a9 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -188,6 +188,7 @@
> /gitweb/gitweb.cgi
> /gitweb/static/gitweb.js
> /gitweb/static/gitweb.min.*
> +/config-list.h
> /command-list.h
> *.tar.gz
> *.dsc
> diff --git a/Makefile b/Makefile
> index ef1ff2228f..d4aff7f9b5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -815,6 +815,7 @@ LIB_FILE = libgit.a
> XDIFF_LIB = xdiff/lib.a
> VCSSVN_LIB = vcs-svn/lib.a
>
> +GENERATED_H += config-list.h
> GENERATED_H += command-list.h
>
> LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
> @@ -2133,7 +2134,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
>
> help.sp help.s help.o: command-list.h
>
> -builtin/help.sp builtin/help.s builtin/help.o: command-list.h GIT-PREFIX
> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
> builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
> '-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
> '-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
> @@ -2153,6 +2154,12 @@ $(BUILT_INS): git$X
> ln -s $< $@ 2>/dev/null || \
> cp $< $@
>
> +config-list.h: generate-configlist.sh
> +
> +config-list.h:
> + $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \
> + >$@+ && mv $@+ $@
> +
> command-list.h: generate-cmdlist.sh command-list.txt
>
> command-list.h: $(wildcard Documentation/git*.txt) Documentation/*config.txt Documentation/config/*.txt
> @@ -2786,7 +2793,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
> .PHONY: sparse $(SP_OBJ)
> sparse: $(SP_OBJ)
>
> -EXCEPT_HDRS := command-list.h unicode-width.h compat/% xdiff/%
> +EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%
> ifndef GCRYPT_SHA256
> EXCEPT_HDRS += sha256/gcrypt.h
> endif
> @@ -2808,7 +2815,7 @@ hdr-check: $(HCO)
> style:
> git clang-format --style file --diff --extensions c,h
>
> -check: command-list.h
> +check: config-list.h command-list.h
> @if sparse; \
> then \
> echo >&2 "Use 'make sparse' instead"; \
> diff --git a/builtin/help.c b/builtin/help.c
> index e5590d7787..1c5f2b9255 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -8,6 +8,7 @@
> #include "parse-options.h"
> #include "run-command.h"
> #include "column.h"
> +#include "config-list.h"
> #include "help.h"
> #include "alias.h"
>
> @@ -62,6 +63,91 @@ static const char * const builtin_help_usage[] = {
> NULL
> };
>
> +struct slot_expansion {
> + const char *prefix;
> + const char *placeholder;
> + void (*fn)(struct string_list *list, const char *prefix);
> + int found;
> +};
> +
> +static void list_config_help(int for_human)
> +{
> + struct slot_expansion slot_expansions[] = {
> + { "advice", "*", list_config_advices },
> + { "color.branch", "<slot>", list_config_color_branch_slots },
> + { "color.decorate", "<slot>", list_config_color_decorate_slots },
> + { "color.diff", "<slot>", list_config_color_diff_slots },
> + { "color.grep", "<slot>", list_config_color_grep_slots },
> + { "color.interactive", "<slot>", list_config_color_interactive_slots },
> + { "color.remote", "<slot>", list_config_color_sideband_slots },
> + { "color.status", "<slot>", list_config_color_status_slots },
> + { "fsck", "<msg-id>", list_config_fsck_msg_ids },
> + { "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> + { NULL, NULL, NULL }
> + };
> + const char **p;
> + struct slot_expansion *e;
> + struct string_list keys = STRING_LIST_INIT_DUP;
> + int i;
> +
> + for (p = config_name_list; *p; p++) {
> + const char *var = *p;
> + struct strbuf sb = STRBUF_INIT;
> +
> + for (e = slot_expansions; e->prefix; e++) {
> +
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> + if (!strcasecmp(var, sb.buf)) {
> + e->fn(&keys, e->prefix);
> + e->found++;
> + break;
> + }
> + }
> + strbuf_release(&sb);
> + if (!e->prefix)
> + string_list_append(&keys, var);
> + }
> +
> + for (e = slot_expansions; e->prefix; e++)
> + if (!e->found)
> + BUG("slot_expansion %s.%s is not used",
> + e->prefix, e->placeholder);
> +
> + string_list_sort(&keys);
> + for (i = 0; i < keys.nr; i++) {
> + const char *var = keys.items[i].string;
> + const char *wildcard, *tag, *cut;
> +
> + if (for_human) {
> + puts(var);
> + continue;
> + }
> +
> + wildcard = strchr(var, '*');
> + tag = strchr(var, '<');
> +
> + if (!wildcard && !tag) {
> + puts(var);
> + continue;
> + }
> +
> + if (wildcard && !tag)
> + cut = wildcard;
> + else if (!wildcard && tag)
> + cut = tag;
> + else
> + cut = wildcard < tag ? wildcard : tag;
> +
> + /*
> + * We may produce duplicates, but that's up to
> + * git-completion.bash to handle
> + */
> + printf("%.*s\n", (int)(cut - var), var);
> + }
> + string_list_clear(&keys, 0);
> +}
> +
> static enum help_format parse_help_format(const char *format)
> {
> if (!strcmp(format, "man"))
> diff --git a/compat/vcbuild/README b/compat/vcbuild/README
> index 1b6dabf5a2..42292e7c09 100644
> --- a/compat/vcbuild/README
> +++ b/compat/vcbuild/README
> @@ -92,8 +92,8 @@ The Steps of Build Git with VS2008
> the git operations.
>
> 3. Inside Git's directory run the command:
> - make command-list.h
> - to generate the command-list.h file needed to compile git.
> + make command-list.h config-list.h
> + to generate the header file needed to compile git.
>
> 4. Then either build Git with the GNU Make Makefile in the Git projects
> root
> diff --git a/config.mak.uname b/config.mak.uname
> index 0ab8e00938..f880cc2792 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -721,9 +721,9 @@ vcxproj:
> echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets
> git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets
>
> - # Add command-list.h
> - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 command-list.h
> - git add -f command-list.h
> + # Add command-list.h and config-list.h
> + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h
In <nycvar.QRO.7.76.6.2004161406480.46@tvgsbejvaqbjf.bet>,
Dscho suggested to squash his patch instead:
https://lore.kernel.org/git/nycvar.QRO.7.76.6.2002261649550.46@tvgsbejvaqbjf.bet/
> + git add -f config-list.h command-list.h
>
> # Add scripts
> rm -f perl/perl.mak
> diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
> index 71158f7d8b..45fecf8bdf 100755
> --- a/generate-cmdlist.sh
> +++ b/generate-cmdlist.sh
> @@ -76,23 +76,6 @@ print_command_list () {
> echo "};"
> }
>
> -print_config_list () {
> - cat <<EOF
> -static const char *config_name_list[] = {
> -EOF
> - grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> - sed '/deprecated/d; s/::$//; s/, */\n/g' |
> - sort |
> - while read line
> - do
> - echo " \"$line\","
> - done
> - cat <<EOF
> - NULL,
> -};
> -EOF
> -}
> -
> exclude_programs=
> while test "--exclude-program" = "$1"
> do
> @@ -113,5 +96,3 @@ echo
> define_category_names "$1"
> echo
> print_command_list "$1"
> -echo
> -print_config_list
> diff --git a/generate-configlist.sh b/generate-configlist.sh
> new file mode 100755
> index 0000000000..8692fe5cf4
> --- /dev/null
> +++ b/generate-configlist.sh
> @@ -0,0 +1,21 @@
> +#!/bin/sh
> +
> +echo "/* Automatically generated by generate-configlist.sh */"
> +echo
> +
> +print_config_list () {
> + cat <<EOF
> +static const char *config_name_list[] = {
> +EOF
> + grep -h '^[a-zA-Z].*\..*::$' Documentation/*config.txt Documentation/config/*.txt |
> + sed '/deprecated/d; s/::$//; s/, */\n/g' |
> + sort |
> + sed 's/^.*$/ "&",/'
> + cat <<EOF
> + NULL,
> +};
> +EOF
> +}
> +
> +echo
> +print_config_list
> diff --git a/help.c b/help.c
> index cf67624a94..a21487db77 100644
> --- a/help.c
> +++ b/help.c
> @@ -407,91 +407,6 @@ void list_common_guides_help(void)
> putchar('\n');
> }
>
> -struct slot_expansion {
> - const char *prefix;
> - const char *placeholder;
> - void (*fn)(struct string_list *list, const char *prefix);
> - int found;
> -};
> -
> -void list_config_help(int for_human)
> -{
> - struct slot_expansion slot_expansions[] = {
> - { "advice", "*", list_config_advices },
> - { "color.branch", "<slot>", list_config_color_branch_slots },
> - { "color.decorate", "<slot>", list_config_color_decorate_slots },
> - { "color.diff", "<slot>", list_config_color_diff_slots },
> - { "color.grep", "<slot>", list_config_color_grep_slots },
> - { "color.interactive", "<slot>", list_config_color_interactive_slots },
> - { "color.remote", "<slot>", list_config_color_sideband_slots },
> - { "color.status", "<slot>", list_config_color_status_slots },
> - { "fsck", "<msg-id>", list_config_fsck_msg_ids },
> - { "receive.fsck", "<msg-id>", list_config_fsck_msg_ids },
> - { NULL, NULL, NULL }
> - };
> - const char **p;
> - struct slot_expansion *e;
> - struct string_list keys = STRING_LIST_INIT_DUP;
> - int i;
> -
> - for (p = config_name_list; *p; p++) {
> - const char *var = *p;
> - struct strbuf sb = STRBUF_INIT;
> -
> - for (e = slot_expansions; e->prefix; e++) {
> -
> - strbuf_reset(&sb);
> - strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
> - if (!strcasecmp(var, sb.buf)) {
> - e->fn(&keys, e->prefix);
> - e->found++;
> - break;
> - }
> - }
> - strbuf_release(&sb);
> - if (!e->prefix)
> - string_list_append(&keys, var);
> - }
> -
> - for (e = slot_expansions; e->prefix; e++)
> - if (!e->found)
> - BUG("slot_expansion %s.%s is not used",
> - e->prefix, e->placeholder);
> -
> - string_list_sort(&keys);
> - for (i = 0; i < keys.nr; i++) {
> - const char *var = keys.items[i].string;
> - const char *wildcard, *tag, *cut;
> -
> - if (for_human) {
> - puts(var);
> - continue;
> - }
> -
> - wildcard = strchr(var, '*');
> - tag = strchr(var, '<');
> -
> - if (!wildcard && !tag) {
> - puts(var);
> - continue;
> - }
> -
> - if (wildcard && !tag)
> - cut = wildcard;
> - else if (!wildcard && tag)
> - cut = tag;
> - else
> - cut = wildcard < tag ? wildcard : tag;
> -
> - /*
> - * We may produce duplicates, but that's up to
> - * git-completion.bash to handle
> - */
> - printf("%.*s\n", (int)(cut - var), var);
> - }
> - string_list_clear(&keys, 0);
> -}
> -
> static int get_alias(const char *var, const char *value, void *data)
> {
> struct string_list *list = data;
> diff --git a/help.h b/help.h
> index 7a455beeb7..9071894e8c 100644
> --- a/help.h
> +++ b/help.h
> @@ -22,7 +22,6 @@ static inline void mput_char(char c, unsigned int num)
> void list_common_cmds_help(void);
> void list_all_cmds_help(void);
> void list_common_guides_help(void);
> -void list_config_help(int for_human);
>
> void list_all_main_cmds(struct string_list *list);
> void list_all_other_cmds(struct string_list *list);
> --
> 2.26.1.301.g55bc3eb7cb9-goog
>
--
Danh
next prev parent reply other threads:[~2020-04-17 2:04 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 21:18 [PATCH v13 0/5] git-bugreport with fixed VS build Emily Shaffer
2020-04-16 21:18 ` [PATCH v13 1/5] help: move list_config_help to builtin/help Emily Shaffer
2020-04-16 22:21 ` Junio C Hamano
2020-04-16 22:28 ` Junio C Hamano
2020-04-17 19:36 ` Emily Shaffer
2020-04-17 2:04 ` Danh Doan [this message]
2020-04-17 2:11 ` Danh Doan
2021-04-08 21:29 ` [PATCH] Makefile: add missing dependencies of 'config-list.h' SZEDER Gábor
2021-04-08 22:08 ` Ævar Arnfjörð Bjarmason
2021-04-08 23:40 ` Jeff King
2021-04-09 21:20 ` SZEDER Gábor
2021-04-16 19:03 ` Junio C Hamano
2021-04-16 21:33 ` SZEDER Gábor
2021-04-16 22:25 ` Junio C Hamano
2021-04-13 19:07 ` Ævar Arnfjörð Bjarmason
2020-04-16 21:18 ` [PATCH v13 2/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-08-12 15:53 ` SZEDER Gábor
2021-04-08 21:36 ` SZEDER Gábor
2020-04-16 21:18 ` [PATCH v13 3/5] bugreport: gather git version and build info Emily Shaffer
2020-04-16 21:18 ` [PATCH v13 4/5] bugreport: add uname info Emily Shaffer
2021-04-08 22:19 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:25 ` Junio C Hamano
2021-04-08 22:33 ` Ævar Arnfjörð Bjarmason
2021-04-08 23:41 ` Emily Shaffer
2021-04-08 23:58 ` Junio C Hamano
2021-04-09 21:27 ` SZEDER Gábor
2021-04-11 14:33 ` [PATCH] t0091-bugreport.sh: actually verify some content of report Martin Ågren
2021-04-12 17:17 ` Junio C Hamano
2021-04-13 18:32 ` Martin Ågren
2021-04-13 19:27 ` Ævar Arnfjörð Bjarmason
2021-04-13 22:21 ` Emily Shaffer
2023-07-01 19:26 ` [PATCH v2] " Martin Ågren
2023-07-03 15:47 ` Phillip Wood
2023-07-05 18:31 ` Martin Ågren
2023-07-05 18:40 ` [PATCH v3] " Martin Ågren
2023-07-05 19:46 ` Phillip Wood
2021-04-13 19:44 ` [PATCH] " SZEDER Gábor
2020-04-16 21:18 ` [PATCH v13 5/5] bugreport: add compiler info Emily Shaffer
2021-04-08 22:23 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:59 ` Đoàn Trần Công Danh
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200417020426.GD2285@danh.dev \
--to=congdanhqx@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).