Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Move the "check-name" helper to a test-tool, since
> a6226fd772b (submodule--helper: convert the bulk of cmd_add() to C,
> 2021-08-10) it has only been used by this test, not git-submodule.sh.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> builtin/submodule--helper.c | 24 -------------------
> t/helper/test-submodule.c | 46 +++++++++++++++++++++++++++++++++++++
> t/t7450-bad-git-dotfiles.sh | 2 +-
> 3 files changed, 47 insertions(+), 25 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b2fc732b5d8..06307886080 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2728,29 +2728,6 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
> return 0;
> }
>
> -/*
> - * Exit non-zero if any of the submodule names given on the command line is
> - * invalid. If no names are given, filter stdin to print only valid names
> - * (which is primarily intended for testing).
> - */
> -static int check_name(int argc, const char **argv, const char *prefix)
> -{
> - if (argc > 1) {
> - while (*++argv) {
> - if (check_submodule_name(*argv) < 0)
> - return 1;
> - }
> - } else {
> - struct strbuf buf = STRBUF_INIT;
> - while (strbuf_getline(&buf, stdin) != EOF) {
> - if (!check_submodule_name(buf.buf))
> - printf("%s\n", buf.buf);
> - }
> - strbuf_release(&buf);
> - }
> - return 0;
> -}
> -
> static int module_config(int argc, const char **argv, const char *prefix)
> {
> enum {
> @@ -3305,7 +3282,6 @@ static struct cmd_struct commands[] = {
> {"summary", module_summary, 0},
> {"push-check", push_check, 0},
> {"absorbgitdirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> - {"check-name", check_name, 0},
> {"config", module_config, 0},
> {"set-url", module_set_url, 0},
> {"set-branch", module_set_branch, 0},
> diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
> index 494c6558d9f..9f0eb440192 100644
> --- a/t/helper/test-submodule.c
> +++ b/t/helper/test-submodule.c
> @@ -2,8 +2,16 @@
> #include "test-tool-utils.h"
> #include "cache.h"
> #include "parse-options.h"
> +#include "submodule-config.h"
> #include "submodule.h"
>
> +#define TEST_TOOL_CHECK_NAME_USAGE \
> + "test-tool submodule check-name <name>"
> +static const char *submodule_check_name_usage[] = {
> + TEST_TOOL_CHECK_NAME_USAGE,
> + NULL
> +};
> +
> #define TEST_TOOL_IS_ACTIVE_USAGE \
> "test-tool submodule is-active <name>"
> static const char *submodule_is_active_usage[] = {
> @@ -12,10 +20,47 @@ static const char *submodule_is_active_usage[] = {
> };
>
> static const char *submodule_usage[] = {
> + TEST_TOOL_CHECK_NAME_USAGE,
> TEST_TOOL_IS_ACTIVE_USAGE,
> NULL
> };
>
> +/*
> + * Exit non-zero if any of the submodule names given on the command line is
> + * invalid. If no names are given, filter stdin to print only valid names
> + * (which is primarily intended for testing).
> + */
> +static int check_name(int argc, const char **argv)
> +{
> + if (argc > 1) {
> + while (*++argv) {
> + if (check_submodule_name(*argv) < 0)
> + return 1;
> + }
> + } else {
> + struct strbuf buf = STRBUF_INIT;
> + while (strbuf_getline(&buf, stdin) != EOF) {
> + if (!check_submodule_name(buf.buf))
> + printf("%s\n", buf.buf);
> + }
> + strbuf_release(&buf);
> + }
> + return 0;
> +}
> +
> +static int cmd__submodule_check_name(int argc, const char **argv)
> +{
> + struct option options[] = {
> + OPT_END()
> + };
> + argc = parse_options(argc, argv, "test-tools", options,
> + submodule_check_name_usage, 0);
> + if (argc)
> + usage_with_options(submodule_check_name_usage, options);
> +
> + return check_name(argc, argv);
> +}
> +
> static int cmd__submodule_is_active(int argc, const char **argv)
> {
> struct option options[] = {
> @@ -32,6 +77,7 @@ static int cmd__submodule_is_active(int argc, const char **argv)
> }
>
> static struct test_cmd cmds[] = {
> + { "check-name", cmd__submodule_check_name },
> { "is-active", cmd__submodule_is_active },
> };
>
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index 41706c1c9ff..2c24f120da3 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -21,7 +21,7 @@ test_expect_success 'check names' '
> valid/with/paths
> EOF
>
> - git submodule--helper check-name >actual <<-\EOF &&
> + test-tool submodule check-name >actual <<-\EOF &&
> valid
> valid/with/paths
Similar to the previous patch, the only tests that use this are
verifying the behavior of "check-name" itself. But I think it makes
sense to keep this one since this seems to have always been intended to
be a sort of "unit test"; it was introduced in 0383bbb901
(submodule-config: verify submodule names as paths, 2018-04-30), and the
commit message indicates that this test protects us against certain
vulnerabilities from maliciously-crafted submodule names.
>
> --
> 2.37.1.1167.g38fda70d8c4
