Re: [PATCH 3/2] ls-files: only recurse on active submodules

[Brandon Williams <bmwill@google.com>]

On 05/12, paul@myitcv.io wrote:
> > ...
> 
> I'm nowhere near up to speed with the entire history of this chain so
> please excuse me if I've missed some detail somewhere.
> 
> As of b06d3643105c8758ed019125a4399cb7efdcce2c, the following command
> "hangs" at near 100% CPU in a repo of mine:
> 
> git ls-files --recurse-submodules
> 
> The "hang" appears to be an explosion of subprocesses (snipped the
> COMMAND detail from ps):
> 
> git ls-files --recurse-submodules
> git --super-prefix=g/_vendor/src/github.com/ramya-rao-a/go-outline/ ls-files --recurse-submodules --
> git --super-prefix=g/_vendor/src/github.com/ramya-rao-a/go-outline/g/_vendor/src/github.com/ramya-rao-a/go-outline/ ls-files --recurse-submodules --
> ....

Yeah the way the recursion works as of 2.13 is that a new process is
spawned for each submodule.  This really is kind of clunky and I'd like
to move to an implementation which doesn't require spawning a ton of
processes, so that's a goal of mine I'm trying to work towards.  This is
still a relatively new feature so I expect there's still a lot of bugs to
iron out.

> 
> In v2.11.0 I simply get a fatal error (this repo doesn't actually
> contain any submodules):
> 
> fatal: can't use --super-prefix from a subdirectory
> 
> I haven't yet been able to work out what's special about the
> subdirectory g/_vendor/src/github.com/ramya-rao-a/go-outline/
> 
> Is this expected? (I'm guessing not)
> 
> How can I help diagnose what's going on here?

Looking at this it appears to me that you have a submodule loop, either
directly or indirectly.  That is if you have a repo 'a' which contains a
submodule 'b' and repo 'b' happens to contain 'a' as a submodule you'd
find yourself with an infinite loop.  But that's what I'm guessing
based on that output you've shown me.

But you're saying that this repo doesn't actually contain submodules?
Hmm then I'm at a loss...unless!

The directory 'go-outline' is a place holder for a submodule so ls-files
spawns a process in that directory. But since its not populated, during
the gitdir discovery it falls back to the superproject's gitdir,
restarting the whole process and creating an infinite loop...

Welp that's a pretty terrible bug which stems from
missing a check to see if a submodule is initialized, and not explicitly
setting GIT_DIR=.git (theres cases where we don't want this but turns
out we probably should do that here).  Let me know if this fixes the
problem:


diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a6c70dbe9..b0796fc10 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -206,7 +206,6 @@ static void show_gitlink(const struct cache_entry *ce)
 	char *dir;
 
 	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",
@@ -242,6 +241,7 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
 	if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
+	    is_submodule_initialized(ce->name) &&
 	    submodule_path_match(&pathspec, name.buf, ps_matched)) {
 		show_gitlink(ce);
 	} else if (match_pathspec(&pathspec, name.buf, name.len,
@@ -611,8 +611,10 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	if (recurse_submodules)
+	if (recurse_submodules) {
+		gitmodules_config();
 		compile_submodule_options(argv, &dir, show_tag);
+	}
 
 	if (recurse_submodules &&
 	    (show_stage || show_deleted || show_others || show_unmerged ||

-- 
Brandon Williams