git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] ls-files: fix path used when recursing into submodules

From
Jacob Keller <jacob.e.keller@intel.com>
Date
Apr 13, 2017, 17:12 UTC
Message-ID
<20170413171224.3537-2-jacob.e.keller@intel.com>
In-Reply-To
<20170413171224.3537-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>

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 <jacob.keller@gmail.com>
---
I'm not sure that I'm convinced by this method of solving the problem as
I suspect it has some corner cases (what about when run inside a
subdirectory? It seems to work for me but I'm not sure...) Additionally,
it felt weird that there's no helper function for creating a toplevel
relative path.
 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 e9b3546ca053..a6c70dbe9ec8 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 c8030dd3299a..ebb956fd16cc 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 &&
-- 
2.12.2.776.gded3dc243c29.dirty
Previous: Jacob KellerNext: Brandon Williams
Message 5 of 33 in “ls-files: properly prepare submodule environment”
  1. ls-files: properly prepare submodule environmentJacob Keller, Apr 12, 2017
  2. Brandon WilliamsApr 12, 2017
  3. Jacob KellerApr 12, 2017
  4. 1/2 ls-files: fix recurse-submodules with nested submodulesJacob Keller, Apr 13, 2017
  5. 2/2 ls-files: fix path used when recursing into submodulesJacob Keller, Apr 13, 2017
  6. Brandon WilliamsApr 13, 2017
  7. Stefan BellerApr 13, 2017
  8. Jacob KellerApr 13, 2017
  9. Junio C HamanoApr 18, 2017
  10. Jacob KellerApr 18, 2017
  11. Stefan BellerApr 18, 2017
  12. Brandon WilliamsApr 13, 2017
  13. Jacob KellerApr 13, 2017
  14. Stefan BellerApr 13, 2017
  15. Brandon WilliamsApr 13, 2017
  16. 3/2 ls-files: only recurse on active submodulesBrandon Williams, Apr 13, 2017
  17. Stefan BellerApr 13, 2017
  18. Jacob KellerApr 13, 2017
  19. Stefan BellerApr 13, 2017
  20. Jacob KellerApr 13, 2017
  21. Jacob KellerApr 14, 2017
  22. Stefan BellerApr 14, 2017
  23. Jacob KellerApr 14, 2017
  24. Junio C HamanoApr 19, 2017
  25. paul@myitcv.ioMay 12, 2017
  26. Brandon WilliamsMay 12, 2017
  27. Paul JollyMay 12, 2017
  28. Brandon WilliamsMay 12, 2017
  29. Paul JollyAug 1, 2017
  30. Brandon WilliamsAug 1, 2017
  31. Paul JollyAug 1, 2017
  32. Brandon WilliamsAug 1, 2017
  33. Junio C HamanoAug 1, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.