@matheusbsbs / matheus-bagliano / commits / a282f5a906

submodule foreach: fix "<command> --quiet" not being respected

Robin reported that git submodule foreach --quiet git pull --quiet origin is not really quiet anymore [1]. "git pull" behaves as if --quiet is not given. This happens because parseopt in submodule--helper will try to parse both --quiet options as if they are foreach's options, not git-pull's. The parsed options are removed from the command line. So when we do pull later, we execute just this git pull origin When calling submodule helper, adding "--" in front of "git pull" will stop parseopt for parsing options that do not really belong to submodule--helper foreach. PARSE_OPT_KEEP_UNKNOWN is removed as a safety measure. parseopt should never see unknown options or something has gone wrong. There are also a couple usage string update while I'm looking at them. While at it, I also add "--" to other subcommands that pass "$@" to submodule--helper. "$@" in these cases are paths and less likely to be --something-like-this. But the point still stands, git-submodule has parsed and classified what are options, what are paths. submodule--helper should never consider paths passed by git-submodule to be options even if they look like one. The test case is also contributed by Robin. [1] it should be quiet before fc1b9243cd (submodule: port submodule subcommand 'foreach' from shell to C, 2018-05-10) because parseopt can't accidentally eat options then. Reported-by: Robin H. Johnson <robbat2@gentoo.org> Tested-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Apr 12, 2019 at 17:08 UTC a282f5a90613de5f4b449749ea8738ac20872271

3 files changed +20 -9

builtin/submodule--helper.c

+4 -4

@@ -566,12 +566,12 @@ static int module_foreach(int argc, const char **argv, const char *prefix)

566 };

567

568 const char *const git_submodule_helper_usage[] = {

569 - N_("git submodule--helper foreach [--quiet] [--recursive] <command>"),

569 + N_("git submodule--helper foreach [--quiet] [--recursive] [--] <command>"),

570 NULL

571 };

572

573 argc = parse_options(argc, argv, prefix, module_foreach_options,

574 - git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);

574 + git_submodule_helper_usage, 0);

575

576 if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)

577 return 1;

@@ -709,7 +709,7 @@ static int module_init(int argc, const char **argv, const char *prefix)

709 };

710

711 const char *const git_submodule_helper_usage[] = {

712 - N_("git submodule--helper init [<path>]"),

712 + N_("git submodule--helper init [<options>] [<path>]"),

713 NULL

714 };

715

@@ -2097,7 +2097,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)

2097 };

2098

2099 const char *const git_submodule_helper_usage[] = {

2100 - N_("git submodule--helper embed-git-dir [<path>...]"),

2100 + N_("git submodule--helper absorb-git-dirs [<options>] [<path>...]"),

2101 NULL

2102 };

2103

git-submodule.sh

+6 -5

@@ -345,7 +345,7 @@ cmd_foreach()

345 shift

346 done

347

348 - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@"

348 + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"

349 }

350

351 #

@@ -376,7 +376,7 @@ cmd_init()

376 shift

377 done

378

379 - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} "$@"

379 + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@"

380 }

381

382 #

@@ -412,7 +412,7 @@ cmd_deinit()

412 shift

413 done

414

415 - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${force:+--force} ${deinit_all:+--all} "$@"

415 + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${force:+--force} ${deinit_all:+--all} -- "$@"

416 }

417

418 is_tip_reachable () (

@@ -541,6 +541,7 @@ cmd_update()

541 ${depth:+--depth "$depth"} \

542 $recommend_shallow \

543 $jobs \

544 + -- \

545 "$@" || echo "#unmatched" $?

546 } | {

547 err=

@@ -933,7 +934,7 @@ cmd_status()

934 shift

935 done

936

936 - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@"

937 + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@"

938 }

939 #

940 # Sync remote urls for submodules

@@ -966,7 +967,7 @@ cmd_sync()

967 esac

968 done

969

969 - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@"

970 + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@"

971 }

972

973 cmd_absorbgitdirs()

t/t7407-submodule-foreach.sh

+10

@@ -411,4 +411,14 @@ test_expect_success 'multi-argument command passed to foreach is not shell-evalu

411 test_cmp expected actual

412 '

413

414 +test_expect_success 'option-like arguments passed to foreach commands are not lost' '

415 + (

416 + cd super &&

417 + git submodule foreach "echo be --quiet" > ../expected &&

418 + git submodule foreach echo be --quiet > ../actual

419 + ) &&

420 + grep -sq -e "--quiet" expected &&

421 + test_cmp expected actual

422 +'

423 +

424 test_done