sha1_file: always allow relative paths to alternates

We recursively expand alternates repositories, so that if A borrows from B which borrows from C, A can see all objects. For the root object database, we allow relative paths, so A can point to B as "../B/objects". However, we currently do not allow relative paths when recursing, so B must use an absolute path to reach C. That is an ancient protection from c2f493a (Transitively read alternatives, 2006-05-07) that tries to avoid adding the same alternate through two different paths. Since 5bdf0a8 (sha1_file: normalize alt_odb path before comparing and storing, 2011-09-07), we use a normalized absolute path for each alt_odb entry. This means that in most cases the protection is no longer necessary; we will detect the duplicate no matter how we got there (but see below). And it's a good idea to get rid of it, as it creates an unnecessary complication when setting up recursive alternates (B has to know that A is going to borrow from it and make sure to use an absolute path). Note that our normalization doesn't actually look at the filesystem, so it can still be fooled by crossing symbolic links. But that's also true of absolute paths, so it's not a good reason to disallow only relative paths (it's potentially a reason to switch to real_path(), but that's a separate and non-trivial change). We adjust the test script here to demonstrate that this now works, and add new tests to show that the normalization does indeed suppress duplicates. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 3, 2016 at 16:36 UTC 087b6d584062f5b704356286d6445bcc84d686fb
2 files changed +23 -8
sha1_file.c
+1 -6
@@ -354,12 +354,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
354 const char *entry = entries.items[i].string;
355 if (entry[0] == '\0' || entry[0] == '#')
356 continue;
357 - if (!is_absolute_path(entry) && depth) {
358 - error("%s: ignoring relative alternate object store %s",
359 - relative_base, entry);
360 - } else {
361 - link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
362 - }
357 + link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
358 }
359 string_list_clear(&entries, 0);
360 free(alt_copy);
t/t5613-info-alternate.sh
+22 -2
@@ -95,8 +95,28 @@ test_expect_success 'that relative alternate is possible for current dir' '
95 git fsck
96 '
97
98 -test_expect_success 'that relative alternate is only possible for current dir' '
99 - test_must_fail git -C D fsck
98 +test_expect_success 'that relative alternate is recursive' '
99 + git -C D fsck
100 +'
101 +
102 +# we can reach "A" from our new repo both directly, and via "C".
103 +# The deep/subdir is there to make sure we are not doing a stupid
104 +# pure-text comparison of the alternate names.
105 +test_expect_success 'relative duplicates are eliminated' '
106 + mkdir -p deep/subdir &&
107 + git init --bare deep/subdir/duplicate.git &&
108 + cat >deep/subdir/duplicate.git/objects/info/alternates <<-\EOF &&
109 + ../../../../C/.git/objects
110 + ../../../../A/.git/objects
111 + EOF
112 + cat >expect <<-EOF &&
113 + alternate: $(pwd)/C/.git/objects
114 + alternate: $(pwd)/B/.git/objects
115 + alternate: $(pwd)/A/.git/objects
116 + EOF
117 + git -C deep/subdir/duplicate.git count-objects -v >actual &&
118 + grep ^alternate: actual >actual.alternates &&
119 + test_cmp expect actual.alternates
120 '
121
122 test_done