@matheusbsbs / matheus-bagliano / commits / a2d5156c2b

resolve_gitlink_ref: ignore non-repository paths

When we want to look up a submodule ref, we use get_ref_cache(path) to find or auto-create its ref cache. But if we feed a path that isn't actually a git repository, we blindly create the ref cache, and then may die deeper in the code when we try to access it. This is a problem because many callers speculatively feed us a path that looks vaguely like a repository, and expect us to tell them when it is not. This patch teaches resolve_gitlink_ref to reject non-repository paths without creating a ref_cache. This avoids the die(), and also performs better if you have a large number of these faux-submodule directories (because the ref_cache lookup is linear, under the assumption that there won't be a large number of submodules). To accomplish this, we also break get_ref_cache into two pieces: the lookup and auto-creation (the latter is lumped into create_ref_cache). This lets us first cheaply ask our cache "is it a submodule we know about?" If so, we can avoid repeating our filesystem lookup. So lookups of real submodules are not penalized; they examine the submodule's .git directory only once. The test in t3000 demonstrates a case where this improves correctness (we used to just die). The new perf case in p7300 shows off the speed improvement in an admittedly pathological repository: Test HEAD^ HEAD ---------------------------------------------------------------- 7300.4: ls-files -o 66.97(66.15+0.87) 0.33(0.08+0.24) -99.5% Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 22, 2016 at 17:29 UTC a2d5156c2b0e6dbffc216b4a673156487a2f8b65

3 files changed +43 -14

refs/files-backend.c

+32 -14

@@ -933,6 +933,10 @@ static void clear_loose_ref_cache(struct ref_cache *refs)

933 }

934 }

935

936 +/*

937 + * Create a new submodule ref cache and add it to the internal

938 + * set of caches.

939 + */

940 static struct ref_cache *create_ref_cache(const char *submodule)

941 {

942 int len;

@@ -942,16 +946,12 @@ static struct ref_cache *create_ref_cache(const char *submodule)

946 len = strlen(submodule) + 1;

947 refs = xcalloc(1, sizeof(struct ref_cache) + len);

948 memcpy(refs->name, submodule, len);

949 + refs->next = submodule_ref_caches;

950 + submodule_ref_caches = refs;

951 return refs;

952 }

953

948 -/*

949 - * Return a pointer to a ref_cache for the specified submodule. For

950 - * the main repository, use submodule==NULL. The returned structure

951 - * will be allocated and initialized but not necessarily populated; it

952 - * should not be freed.

953 - */

954 -static struct ref_cache *get_ref_cache(const char *submodule)

954 +static struct ref_cache *lookup_ref_cache(const char *submodule)

955 {

956 struct ref_cache *refs;

957

@@ -961,10 +961,20 @@ static struct ref_cache *get_ref_cache(const char *submodule)

961 for (refs = submodule_ref_caches; refs; refs = refs->next)

962 if (!strcmp(submodule, refs->name))

963 return refs;

964 + return NULL;

965 +}

966

965 - refs = create_ref_cache(submodule);

966 - refs->next = submodule_ref_caches;

967 - submodule_ref_caches = refs;

967 +/*

968 + * Return a pointer to a ref_cache for the specified submodule. For

969 + * the main repository, use submodule==NULL. The returned structure

970 + * will be allocated and initialized but not necessarily populated; it

971 + * should not be freed.

972 + */

973 +static struct ref_cache *get_ref_cache(const char *submodule)

974 +{

975 + struct ref_cache *refs = lookup_ref_cache(submodule);

976 + if (!refs)

977 + refs = create_ref_cache(submodule);

978 return refs;

979 }

980

@@ -1336,16 +1346,24 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,

1346 int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1)

1347 {

1348 int len = strlen(path), retval;

1339 - char *submodule;

1349 + struct strbuf submodule = STRBUF_INIT;

1350 struct ref_cache *refs;

1351

1352 while (len && path[len-1] == '/')

1353 len--;

1354 if (!len)

1355 return -1;

1346 - submodule = xstrndup(path, len);

1347 - refs = get_ref_cache(submodule);

1348 - free(submodule);

1356 +

1357 + strbuf_add(&submodule, path, len);

1358 + refs = lookup_ref_cache(submodule.buf);

1359 + if (!refs) {

1360 + if (!is_nonbare_repository_dir(&submodule)) {

1361 + strbuf_release(&submodule);

1362 + return -1;

1363 + }

1364 + refs = create_ref_cache(submodule.buf);

1365 + }

1366 + strbuf_release(&submodule);

1367

1368 retval = resolve_gitlink_ref_recursive(refs, refname, sha1, 0);

1369 return retval;

t/perf/p7300-clean.sh

@@ -28,4 +28,8 @@ test_perf 'clean many untracked sub dirs, ignore nested git' '

28 git clean -n -q -f -f -d 100000_sub_dirs/

29 '

31 +test_perf 'ls-files -o' '

32 + git ls-files -o

33 +'

34 +

35 test_done

t/t3000-ls-files-others.sh

@@ -65,6 +65,13 @@ test_expect_success '--no-empty-directory hides empty directory' '

65 test_cmp expected3 output

66 '

68 +test_expect_success 'ls-files --others handles non-submodule .git' '

69 + mkdir not-a-submodule &&

70 + echo foo >not-a-submodule/.git &&

71 + git ls-files -o >output &&

72 + test_cmp expected1 output

73 +'

74 +

75 test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '

76 git init super &&

77 git init sub &&