[PATCH] object-file: use real paths when adding alternates

From: Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Date: Nov 17, 2022, 17:31 UTC
Message-ID: <pull.1382.git.git.1668706274099.gitgitgadget@gmail.com>

From: Glen Choo <chooglen@google.com>

When adding an alternate ODB, we check if the alternate has the same path as the object dir, and if so, we do nothing. However, that comparison does not resolve symlinks. This makes it possible to add the object dir as an alternate, which may result in bad behavior. For example, it can trick "git repack -a -l -d" (possibly run by "git gc") into thinking that all packs come from an alternate and delete all objects.

	rm -rf test &&
	git clone https://github.com/git/git test &&
	(
	cd test &&
	ln -s objects .git/alt-objects &&
	# -c repack.updateserverinfo=false silences a warning about not
	# being able to update "info/refs", it isn't needed to show the
	# bad behavior
	GIT_ALTERNATE_OBJECT_DIRECTORIES=".git/alt-objects" git \
		-c repack.updateserverinfo=false repack -a -l -d  &&
	# It's broken!
	git status
	# Because there are no more objects!
	ls .git/objects/pack
	)

Fix this by resolving symlinks before comparing the alternate and object dir.

Signed-off-by: Glen Choo <chooglen@google.com>
---
    object-file: use real paths when adding alternates
    
    Here's a bug that got uncovered because of some oddities in how "repo"
    [1] manages its object directories. With some tracing, I'm quite certain
    that the mechanism is that the packs are treated as non-local, but I
    don't understand "git repack" extremely well, so e.g. the test I added
    seems pretty crude.
    
    [1] https://gerrit.googlesource.com/git-repo

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1382%2Fchooglen%2Fobject-file%2Fcheck-alternate-real-path-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1382/chooglen/object-file/check-alternate-real-path-v1
Pull-Request: https://github.com/git/git/pull/1382

 object-file.c     | 17 ++++++++++++-----
 t/t7700-repack.sh | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/object-file.c b/object-file.c
index 957790098fa..f901dd272d1 100644
--- a/object-file.c
+++ b/object-file.c
@@ -455,14 +455,16 @@ static int alt_odb_usable(struct raw_object_store *o,
 			  struct strbuf *path,
 			  const char *normalized_objdir, khiter_t *pos)
 {
+	int ret = 0;
 	int r;
+	struct strbuf real_path = STRBUF_INIT;
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path->buf)) {
 		error(_("object directory %s does not exist; "
 			"check .git/objects/info/alternates"),
 		      path->buf);
-		return 0;
+		goto cleanup;
 	}
 
 	/*
@@ -478,11 +480,16 @@ static int alt_odb_usable(struct raw_object_store *o,
 		assert(r == 1); /* never used */
 		kh_value(o->odb_by_path, p) = o->odb;
 	}
-	if (fspatheq(path->buf, normalized_objdir))
-		return 0;
+
+	strbuf_realpath(&real_path, path->buf, 1);
+	if (fspatheq(real_path.buf, normalized_objdir))
+		goto cleanup;
 	*pos = kh_put_odb_path_map(o->odb_by_path, path->buf, &r);
 	/* r: 0 = exists, 1 = never used, 2 = deleted */
-	return r == 0 ? 0 : 1;
+	ret = r == 0 ? 0 : 1;
+ cleanup:
+	strbuf_release(&real_path);
+	return ret;
 }
 
 /*
@@ -596,7 +603,7 @@ static void link_alt_odb_entries(struct repository *r, const char *alt,
 		return;
 	}
 
-	strbuf_add_absolute_path(&objdirbuf, r->objects->odb->path);
+	strbuf_realpath(&objdirbuf, r->objects->odb->path, 1);
 	if (strbuf_normalize_path(&objdirbuf) < 0)
 		die(_("unable to normalize object directory: %s"),
 		    objdirbuf.buf);
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 5be483bf887..ce1954d0977 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -90,6 +90,24 @@ test_expect_success 'loose objects in alternate ODB are not repacked' '
 	test_has_duplicate_object false
 '
 
+test_expect_success '--local keeps packs when alternate is objectdir ' '
+	git init alt_symlink &&
+	(
+		cd alt_symlink &&
+		git init &&
+		echo content >file4 &&
+		git add file4 &&
+		git commit -m commit_file4 &&
+		git repack -a &&
+		ls .git/objects/pack/*.pack >../expect &&
+		ln -s objects .git/alt_objects &&
+		echo "$(pwd)/.git/alt_objects" >.git/objects/info/alternates &&
+		git repack -a -d -l &&
+		ls .git/objects/pack/*.pack >../actual
+	) &&
+	test_cmp expect actual
+'
+
 test_expect_success 'packed obs in alt ODB are repacked even when local repo is packless' '
 	mkdir alt_objects/pack &&
 	mv .git/objects/pack/* alt_objects/pack &&

base-commit: 319605f8f00e402f3ea758a02c63534ff800a711

-- 
gitgitgadget