Re: [PATCH/RFC 3/3] ls-files/dir: use is_git_repo to detect submodules

[Jeff King <peff@peff.net>]

On Fri, Dec 04, 2015 at 07:15:17PM +0100, Andreas Krey wrote:

> Using resolve_gitlink_ref to check for submodules
> creates submodule list entries even when it isn't
> one, and causes O(n^2) runtime behaviour in repos
> with many untracked directories.
> 
> Use is_git_repo instead for detection.
> 
> Signed-off-by: Andreas Krey <a.krey@gmx.de>
> ---
> This looks good, but it breaks test - at least
> number 67 ('../bar/a/b/c works with relative local
> path - ../foo/bar.git') in t7400-submodule-basic.sh
> 
> I don't really understand yet how to fix that,
> but it is due to resolve_gitlink_ref looking
> for a valid HEAD which is_git_repo doesn't.

Hrm. Weird. You'd think it would break with the existing code if I do
this, then:

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 540771c..944e9f5 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -754,7 +754,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		mkdir -p a/b/c &&
-		(cd a/b/c; git init) &&
+		(cd a/b/c; git init && git commit --allow-empty -m foo) &&
 		git config remote.origin.url ../foo/bar.git &&
 		git submodule add ../bar/a/b/c ./a/b/c &&
 		git submodule init &&

But it doesn't. So presumably there is a mismatch where some other code
expects that anything which gets marked as a repo in the code you
changed can also be resolved to a sha1. I'm not sure where that other
code is though (in git-submodule.sh, or elsewhere in add).

Perhaps we end up in index_path(), which then barfs? That would explain
this (run in the trash directory after the test fails):

  $ cd reltest && git add a/b/c
  error: unable to index file a/b/c/
  fatal: adding files failed

We know it is a git dir, but there is no sha1 for us to actually add as
the gitlink entry.

If that is the case, then there is either some very tricky refactoring
required, or what we are trying to do here is simply wrong. Maybe it
would be simpler to just speed up resolve_gitlink_ref with a better data
structure.

-Peff