Brandon Williams <bmwill@xxxxxxxxxx> writes:
> Pass through some known-safe options when recursing into submodules.
> (--cached, --stage, -v, -t, -z, --debug, --eol)
>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
> builtin/ls-files.c | 34 ++++++++++++++++++++++++++++++----
> t/t3007-ls-files-recurse-submodules.sh | 17 ++++++++++++-----
> 2 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index d4bfc60..a39367f 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -31,6 +31,7 @@ static int debug_mode;
> static int show_eol;
> static int recurse_submodules;
> static const char *submodule_prefix;
> +static struct argv_array recurse_submodules_opts = ARGV_ARRAY_INIT;
I'd imagine that this is also thread-unsafe, but we do not have to
comment it ;-)
> @@ -170,6 +171,27 @@ static void show_killed_files(struct dir_struct *dir)
> }
> }
>
> +/*
> + * Compile an argv_array with all of the options supported by --recurse_submodules
> + */
> +static void compile_submodule_options(int show_tag)
> +{
> + if (show_cached)
> + argv_array_push(&recurse_submodules_opts, "--cached");
> + if (show_stage)
> + argv_array_push(&recurse_submodules_opts, "--stage");
> + if (show_valid_bit)
> + argv_array_push(&recurse_submodules_opts, "-v");
> + if (show_tag)
> + argv_array_push(&recurse_submodules_opts, "-t");
> + if (line_terminator == '\0')
> + argv_array_push(&recurse_submodules_opts, "-z");
> + if (debug_mode)
> + argv_array_push(&recurse_submodules_opts, "--debug");
> + if (show_eol)
> + argv_array_push(&recurse_submodules_opts, "--eol");
> +}
OK. These are only the safe ones to pass through? "compile" or
"assemble" is much less important thing to say than how these are
chosen. "pass_supported_options()" or something? I dunno.
> if (recurse_submodules &&
> - (show_stage || show_deleted || show_others || show_unmerged ||
> + (show_deleted || show_others || show_unmerged ||
> show_killed || show_modified || show_resolve_undo ||
> - show_valid_bit || show_tag || show_eol || with_tree ||
> - (line_terminator == '\0')))
> + with_tree))
> die("ls-files --recurse-submodules unsupported mode");
Makes sense.
> +test_expect_success 'ls-files correctly outputs files in submodule with -z' '
> + cat | tr "\n" "\0" >expect <<-\EOF &&
> + .gitmodules
> + a
> + b/b
> + submodule/c
> + EOF
Hmm, what do you need "cat" for here?
In nul_to_q and q_to_nul implementations (t/test-lib-functions.sh)
we seem to avoid using "tr", even though q_to_cr and others do use
it. I wonder if we had some portability issues with passing NUL
through tr or something?
... digs and finds e85fe4d8 ("more tr portability test script
fixes", 2008-03-12)
So use something like
perl -pe 'y/\012/\000/' <<\-EOF
...
EOF
instead, perhaps?
> + git ls-files --recurse-submodules -z >actual &&
> + test_cmp expect actual
> +'