git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "René Scharfe" <l.s.r@web.de>
Subject: [PATCH 17/18] sha1_file: always allow relative paths to alternates
Date: Mon, 3 Oct 2016 16:36:22 -0400	[thread overview]
Message-ID: <20161003203622.7uz76ay5f7bqqpfm@sigill.intra.peff.net> (raw)
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>

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>
---
 sha1_file.c               |  7 +------
 t/t5613-info-alternate.sh | 24 ++++++++++++++++++++++--
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 80a3333..b514167 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -354,12 +354,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
 		const char *entry = entries.items[i].string;
 		if (entry[0] == '\0' || entry[0] == '#')
 			continue;
-		if (!is_absolute_path(entry) && depth) {
-			error("%s: ignoring relative alternate object store %s",
-					relative_base, entry);
-		} else {
-			link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
-		}
+		link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
 	}
 	string_list_clear(&entries, 0);
 	free(alt_copy);
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 74f6770..76525a0 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -92,8 +92,28 @@ test_expect_success 'that relative alternate is possible for current dir' '
 	git fsck
 '
 
-test_expect_success 'that relative alternate is only possible for current dir' '
-	test_must_fail git -C D fsck
+test_expect_success 'that relative alternate is recursive' '
+	git -C D fsck
+'
+
+# we can reach "A" from our new repo both directly, and via "C".
+# The deep/subdir is there to make sure we are not doing a stupid
+# pure-text comparison of the alternate names.
+test_expect_success 'relative duplicates are eliminated' '
+	mkdir -p deep/subdir &&
+	git init --bare deep/subdir/duplicate.git &&
+	cat >deep/subdir/duplicate.git/objects/info/alternates <<-\EOF &&
+	../../../../C/.git/objects
+	../../../../A/.git/objects
+	EOF
+	cat >expect <<-EOF &&
+	alternate: $(pwd)/C/.git/objects
+	alternate: $(pwd)/B/.git/objects
+	alternate: $(pwd)/A/.git/objects
+	EOF
+	git -C deep/subdir/duplicate.git count-objects -v >actual &&
+	grep ^alternate: actual >actual.alternates &&
+	test_cmp expect actual.alternates
 '
 
 test_done
-- 
2.10.0.618.g82cc264


  parent reply	other threads:[~2016-10-03 20:36 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-03 20:33 [PATCH 0/18] alternate object database cleanups Jeff King
2016-10-03 20:33 ` [PATCH 01/18] t5613: drop reachable_via function Jeff King
2016-10-04  5:48   ` Jacob Keller
2016-10-04 13:43     ` Jeff King
2016-10-03 20:33 ` [PATCH 02/18] t5613: drop test_valid_repo function Jeff King
2016-10-04  5:50   ` Jacob Keller
2016-10-03 20:34 ` [PATCH 03/18] t5613: use test_must_fail Jeff King
2016-10-04  5:51   ` Jacob Keller
2016-10-03 20:34 ` [PATCH 04/18] t5613: whitespace/style cleanups Jeff King
2016-10-04  5:52   ` Jacob Keller
2016-10-04 13:47     ` Jeff King
2016-10-04 20:41       ` Jacob Keller
2016-10-03 20:34 ` [PATCH 05/18] t5613: do not chdir in main process Jeff King
2016-10-04  5:54   ` Jacob Keller
2016-10-04 21:00   ` Junio C Hamano
2016-10-03 20:34 ` [PATCH 06/18] t5613: clarify "too deep" recursion tests Jeff King
2016-10-04  5:57   ` Jacob Keller
2016-10-04 13:48     ` Jeff King
2016-10-04 20:44       ` Jacob Keller
2016-10-04 20:49         ` Jeff King
2016-10-04 20:52           ` Jacob Keller
2016-10-04 20:55             ` Jeff King
2016-10-04 20:58               ` Stefan Beller
2016-10-04 21:00                 ` Jeff King
2016-10-05 13:58                 ` Jakub Narębski
2016-10-05 14:40                   ` Jeff King
2016-10-05 16:14                     ` Junio C Hamano
2016-10-05 16:47                     ` Jacob Keller
2016-10-04 21:43               ` Jacob Keller
2016-10-04 21:49                 ` Jeff King
2016-10-04 21:50                   ` Jacob Keller
2016-10-03 20:34 ` [PATCH 07/18] link_alt_odb_entry: handle normalize_path errors Jeff King
2016-10-04  6:01   ` Jacob Keller
2016-10-04 21:08   ` Junio C Hamano
2016-10-05 18:47   ` René Scharfe
2016-10-05 19:04     ` Jeff King
2016-11-07 23:42   ` Bryan Turner
2016-11-08  0:30     ` Jeff King
2016-11-08  1:12       ` Bryan Turner
2016-11-08  5:33         ` Jeff King
2016-11-08 19:27           ` Bryan Turner
2016-10-03 20:34 ` [PATCH 08/18] link_alt_odb_entry: refactor string handling Jeff King
2016-10-04  6:05   ` Jacob Keller
2016-10-04 13:53     ` Jeff King
2016-10-04 20:46       ` Jacob Keller
2016-10-04 21:18   ` Junio C Hamano
2016-10-03 20:35 ` [PATCH 09/18] alternates: provide helper for adding to alternates list Jeff King
2016-10-04  6:07   ` Jacob Keller
2016-10-03 20:35 ` [PATCH 10/18] alternates: provide helper for allocating alternate Jeff King
2016-10-04  6:09   ` Jacob Keller
2016-10-03 20:35 ` [PATCH 11/18] alternates: encapsulate alt->base munging Jeff King
2016-10-03 20:35 ` [PATCH 12/18] alternates: use a separate scratch space Jeff King
2016-10-04  6:12   ` Jacob Keller
2016-10-04 21:29   ` Junio C Hamano
2016-10-04 21:32     ` Jeff King
2016-10-04 21:49       ` Junio C Hamano
2016-10-04 21:51         ` Jeff King
2016-10-03 20:35 ` [PATCH 13/18] fill_sha1_file: write "boring" characters Jeff King
2016-10-04  6:13   ` Jacob Keller
2016-10-04 21:46     ` Junio C Hamano
2016-10-04 21:48       ` Jeff King
2016-10-04 21:49       ` Jacob Keller
2016-10-05 19:35         ` Junio C Hamano
2016-10-03 20:36 ` [PATCH 14/18] alternates: store scratch buffer as strbuf Jeff King
2016-10-03 20:36 ` [PATCH 15/18] fill_sha1_file: write into a strbuf Jeff King
2016-10-04  6:44   ` Jacob Keller
2016-10-03 20:36 ` [PATCH 16/18] count-objects: report alternates via verbose mode Jeff King
2016-10-04  6:46   ` Jacob Keller
2016-10-04 13:56     ` Jeff King
2016-10-05 14:23   ` Jakub Narębski
2016-10-05 18:47   ` René Scharfe
2016-10-03 20:36 ` Jeff King [this message]
2016-10-04  6:50   ` [PATCH 17/18] sha1_file: always allow relative paths to alternates Jacob Keller
2016-10-04 14:00     ` Jeff King
2016-10-03 20:36 ` [PATCH 18/18] alternates: use fspathcmp to detect duplicates Jeff King
2016-10-04  6:51   ` Jacob Keller
2016-10-04 14:10     ` Jeff King
2016-10-04 21:42   ` Junio C Hamano
2016-10-05  2:34   ` Aaron Schrab
2016-10-05  3:54     ` Jeff King
2016-10-04  5:47 ` [PATCH 0/18] alternate object database cleanups Jacob Keller
2016-10-04 13:41   ` Jeff King
2016-10-04 20:40     ` Jacob Keller
2016-10-05 18:47 ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161003203622.7uz76ay5f7bqqpfm@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).