standardize and improve lookup rules for external local repos

When you specify a local repository on the command line of clone, ls-remote, upload-pack, receive-pack, or upload-archive, or in a request to git-daemon, we perform a little bit of lookup magic, doing things like looking in working trees for .git directories and appending ".git" for bare repos. For clone, this magic happens in get_repo_path. For everything else, it happens in enter_repo. In both cases, there are some ambiguous or confusing cases that aren't handled well, and there is one case that is not handled the same by both methods. This patch tries to provide (and test!) standard, sensible lookup rules for both code paths. The intended changes are: 1. When looking up "foo", we have always preferred a working tree "foo" (containing "foo/.git" over the bare "foo.git". But we did not prefer a bare "foo" over "foo.git". With this patch, we do so. 2. We would select directories that existed but didn't actually look like git repositories. With this patch, we make sure a selected directory looks like a git repo. Not only is this more sensible in general, but it will help anybody who is negatively affected by change (1) negatively (e.g., if they had "foo.git" next to its separate work tree "foo", and expect to keep finding "foo.git" when they reference "foo"). 3. The enter_repo code path would, given "foo", look for "foo.git/.git" (i.e., do the ".git" append magic even for a repo with working tree). The clone code path did not; with this patch, they now behave the same. In the unlikely case of a working tree overlaying a bare repo (i.e., a ".git" directory _inside_ a bare repo), we continue to treat it as a working tree (prefering the "inner" .git over the bare repo). This is mainly because the combination seems nonsensical, and I'd rather stick with existing behavior on the off chance that somebody is relying on it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 2, 2012 at 16:59 UTC b3256eb8b35937192e85725d0c2bcb422295790c
5 files changed +109 -5
builtin/clone.c
+2 -2
@@ -105,7 +105,7 @@ static const char *argv_submodule[] = {
105
106 static char *get_repo_path(const char *repo, int *is_bundle)
107 {
108 - static char *suffix[] = { "/.git", ".git", "" };
108 + static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
109 static char *bundle_suffix[] = { ".bundle", "" };
110 struct stat st;
111 int i;
@@ -115,7 +115,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
115 path = mkpath("%s%s", repo, suffix[i]);
116 if (stat(path, &st))
117 continue;
118 - if (S_ISDIR(st.st_mode)) {
118 + if (S_ISDIR(st.st_mode) && is_git_directory(path)) {
119 *is_bundle = 0;
120 return xstrdup(absolute_path(path));
121 } else if (S_ISREG(st.st_mode) && st.st_size > 8) {
cache.h
+1
@@ -432,6 +432,7 @@ extern char *git_work_tree_cfg;
432 extern int is_inside_work_tree(void);
433 extern int have_git_dir(void);
434 extern const char *get_git_dir(void);
435 +extern int is_git_directory(const char *path);
436 extern char *get_object_directory(void);
437 extern char *get_index_file(void);
438 extern char *get_graft_file(void);
path.c
+5 -2
@@ -293,7 +293,7 @@ const char *enter_repo(const char *path, int strict)
293
294 if (!strict) {
295 static const char *suffix[] = {
296 - ".git/.git", "/.git", ".git", "", NULL,
296 + "/.git", "", ".git/.git", ".git", NULL,
297 };
298 const char *gitfile;
299 int len = strlen(path);
@@ -324,8 +324,11 @@ const char *enter_repo(const char *path, int strict)
324 return NULL;
325 len = strlen(used_path);
326 for (i = 0; suffix[i]; i++) {
327 + struct stat st;
328 strcpy(used_path + len, suffix[i]);
328 - if (!access(used_path, F_OK)) {
329 + if (!stat(used_path, &st) &&
330 + (S_ISREG(st.st_mode) ||
331 + (S_ISDIR(st.st_mode) && is_git_directory(used_path)))) {
332 strcat(validated_path, suffix[i]);
333 break;
334 }
setup.c
+1 -1
@@ -247,7 +247,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
247 * a proper "ref:", or a regular file HEAD that has a properly
248 * formatted sha1 object name.
249 */
250 -static int is_git_directory(const char *suspect)
250 +int is_git_directory(const char *suspect)
251 {
252 char path[PATH_MAX];
253 size_t len = strlen(suspect);
t/t5900-repo-selection.sh new
+100
@@ -0,0 +1,100 @@
1 +#!/bin/sh
2 +
3 +test_description='selecting remote repo in ambiguous cases'
4 +. ./test-lib.sh
5 +
6 +reset() {
7 + rm -rf foo foo.git fetch clone
8 +}
9 +
10 +make_tree() {
11 + git init "$1" &&
12 + (cd "$1" && test_commit "$1")
13 +}
14 +
15 +make_bare() {
16 + git init --bare "$1" &&
17 + (cd "$1" &&
18 + tree=`git hash-object -w -t tree /dev/null` &&
19 + commit=$(echo "$1" | git commit-tree $tree) &&
20 + git update-ref HEAD $commit
21 + )
22 +}
23 +
24 +get() {
25 + git init --bare fetch &&
26 + (cd fetch && git fetch "../$1") &&
27 + git clone "$1" clone
28 +}
29 +
30 +check() {
31 + echo "$1" >expect &&
32 + (cd fetch && git log -1 --format=%s FETCH_HEAD) >actual.fetch &&
33 + (cd clone && git log -1 --format=%s HEAD) >actual.clone &&
34 + test_cmp expect actual.fetch &&
35 + test_cmp expect actual.clone
36 +}
37 +
38 +test_expect_success 'find .git dir in worktree' '
39 + reset &&
40 + make_tree foo &&
41 + get foo &&
42 + check foo
43 +'
44 +
45 +test_expect_success 'automagically add .git suffix' '
46 + reset &&
47 + make_bare foo.git &&
48 + get foo &&
49 + check foo.git
50 +'
51 +
52 +test_expect_success 'automagically add .git suffix to worktree' '
53 + reset &&
54 + make_tree foo.git &&
55 + get foo &&
56 + check foo.git
57 +'
58 +
59 +test_expect_success 'prefer worktree foo over bare foo.git' '
60 + reset &&
61 + make_tree foo &&
62 + make_bare foo.git &&
63 + get foo &&
64 + check foo
65 +'
66 +
67 +test_expect_success 'prefer bare foo over bare foo.git' '
68 + reset &&
69 + make_bare foo &&
70 + make_bare foo.git &&
71 + get foo &&
72 + check foo
73 +'
74 +
75 +test_expect_success 'disambiguate with full foo.git' '
76 + reset &&
77 + make_bare foo &&
78 + make_bare foo.git &&
79 + get foo.git &&
80 + check foo.git
81 +'
82 +
83 +test_expect_success 'we are not fooled by non-git foo directory' '
84 + reset &&
85 + make_bare foo.git &&
86 + mkdir foo &&
87 + get foo &&
88 + check foo.git
89 +'
90 +
91 +test_expect_success 'prefer inner .git over outer bare' '
92 + reset &&
93 + make_tree foo &&
94 + make_bare foo.git &&
95 + mv foo/.git foo.git &&
96 + get foo.git &&
97 + check foo
98 +'
99 +
100 +test_done