On 04/13, Jacob Keller wrote:
> From: Jacob Keller <jacob.kel...@gmail.com>
>
> Since commit e77aa336f116 ("ls-files: optionally recurse into
> submodules", 2016-10-07) ls-files has known how to recurse into
> submodules when displaying files.
>
> Unfortunately this fails for certain cases, including when nesting more
> than one submodule, called from within a submodule that itself has
> submodules, or when the GIT_DIR environemnt variable is set.
>
> Prior to commit b58a68c1c187 ("setup: allow for prefix to be passed to
> git commands", 2017-03-17) this resulted in an error indicating that
> --prefix and --super-prefix were incompatible.
>
> After this commit, instead, the process loops forever with a GIT_DIR set
> to the parent and continuously reads the parent submodule files and
> recursing forever.
>
> Fix this by preparing the environment properly for submodules when
> setting up the child process. This is similar to how other commands such
> as grep behave.
>
> This was not caught by the original tests because the scenario is
> avoided if the submodules are created separately and not stored as the
> standard method of putting the submodule git directory under
> .git/modules/<name>. We can update the test to show the failure by the
> addition of "git submodule absorbgitdirs" to the test case. However,
> note that this new test would run forever without the necessary fix in
> this patch.
>
> Signed-off-by: Jacob Keller <jacob.kel...@gmail.com>
This looks good to me. Thanks again for catching this. Dealing with
submodules definitely isn't easy (I seem to have made a lot of mistakes
that have been cropping up recently)...it would be easier if we didn't
have to spin out a process for each submodule but that's not the world
we live in today :)
> ---
> I updated and reworded the description so that the problem would be more
> obvious. It doesn't occur always, but only when run nested with properly
> absorbed gitdirs for the submodules. This explains the reason why the
> test case had not caught the issue before.
>
> builtin/ls-files.c | 4 ++++
> t/t3007-ls-files-recurse-submodules.sh | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
> index d449e46db551..e9b3546ca053 100644
> --- a/builtin/ls-files.c
> +++ b/builtin/ls-files.c
> @@ -15,6 +15,7 @@
> #include "string-list.h"
> #include "pathspec.h"
> #include "run-command.h"
> +#include "submodule.h"
>
> static int abbrev;
> static int show_deleted;
> @@ -203,6 +204,9 @@ static void show_gitlink(const struct cache_entry *ce)
> struct child_process cp = CHILD_PROCESS_INIT;
> int status;
>
> + prepare_submodule_repo_env(&cp.env_array);
> + argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
> +
> if (prefix_len)
> argv_array_pushf(&cp.env_array, "%s=%s",
> GIT_TOPLEVEL_PREFIX_ENVIRONMENT,
> diff --git a/t/t3007-ls-files-recurse-submodules.sh
> b/t/t3007-ls-files-recurse-submodules.sh
> index 4cf6ccf5a8ea..c8030dd3299a 100755
> --- a/t/t3007-ls-files-recurse-submodules.sh
> +++ b/t/t3007-ls-files-recurse-submodules.sh
> @@ -77,6 +77,7 @@ test_expect_success 'ls-files recurses more than 1 level' '
> git -C submodule/subsub commit -m "add d" &&
> git -C submodule submodule add ./subsub &&
> git -C submodule commit -m "added subsub" &&
> + git submodule absorbgitdirs &&
> git ls-files --recurse-submodules >actual &&
> test_cmp expect actual
> '
> --
> 2.12.2.776.gded3dc243c29.dirty
>
--
Brandon Williams
