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

From: Brandon Williams <bmwill@google.com>
Date: May 12, 2017, 17:26 UTC
Message-ID: <20170512172657.GA98586@google.com>
In-Reply-To: <20170512162109.49752-1-paul@myitcv.io>

On 05/12, paul@myitcv.io wrote:

Show 17 quoted lines

> > ...
> 
> 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.

Show 12 quoted lines

> 
> 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