From 2e5d6503bdc92260eae9c58b9fd1add7014bb853 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Thu, 13 Apr 2017 10:12:23 -0700 Subject: ls-files: fix recurse-submodules with nested submodules 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/. 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 Signed-off-by: Junio C Hamano --- 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 d449e46db5..e9b3546ca0 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 4cf6ccf5a8..c8030dd329 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 ' -- cgit v1.2.3 From 2cfe66a8ee57fb3da18c262db8e6df95e263510b Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Thu, 13 Apr 2017 10:12:24 -0700 Subject: ls-files: fix path used when recursing into submodules Don't assume that the current working directory is the root of the repository. Correctly generate the path for the recursing child processes by building it from the work_tree() root instead. Otherwise if we run ls-files using --git-dir or --work-tree it will not work correctly as it attempts to change directory into a potentially invalid location. Best case, it doesn't exist and we produce an error. Worst case we cd into the wrong location and unknown behavior occurs. Add a new test which highlights this possibility. Signed-off-by: Jacob Keller Signed-off-by: Junio C Hamano --- builtin/ls-files.c | 5 ++++- t/t3007-ls-files-recurse-submodules.sh | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e9b3546ca0..a6c70dbe9e 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -203,6 +203,7 @@ static void show_gitlink(const struct cache_entry *ce) { struct child_process cp = CHILD_PROCESS_INIT; int status; + char *dir; prepare_submodule_repo_env(&cp.env_array); argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT); @@ -221,8 +222,10 @@ static void show_gitlink(const struct cache_entry *ce) argv_array_pushv(&cp.args, submodule_options.argv); cp.git_cmd = 1; - cp.dir = ce->name; + dir = mkpathdup("%s/%s", get_git_work_tree(), ce->name); + cp.dir = dir; status = run_command(&cp); + free(dir); if (status) exit(status); } diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh index c8030dd329..ebb956fd16 100755 --- a/t/t3007-ls-files-recurse-submodules.sh +++ b/t/t3007-ls-files-recurse-submodules.sh @@ -82,6 +82,17 @@ test_expect_success 'ls-files recurses more than 1 level' ' test_cmp expect actual ' +test_expect_success 'ls-files works with GIT_DIR' ' + cat >expect <<-\EOF && + .gitmodules + c + subsub/d + EOF + + git --git-dir=submodule/.git ls-files --recurse-submodules >actual && + test_cmp expect actual +' + test_expect_success '--recurse-submodules and pathspecs setup' ' echo e >submodule/subsub/e.txt && git -C submodule/subsub add e.txt && -- cgit v1.2.3