Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

 object-file.c     | 12 ++++++------
 t/t7700-repack.sh | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/object-file.c b/object-file.c
index 957790098fa..ef2b762234d 100644
--- a/object-file.c
+++ b/object-file.c

@@ -508,6 +508,7 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
 {
 	struct object_directory *ent;
 	struct strbuf pathbuf = STRBUF_INIT;
+	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 
 	if (!is_absolute_path(entry->buf) && relative_base) {

@@ -516,12 +517,14 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
 	}
 	strbuf_addbuf(&pathbuf, entry);
 
-	if (strbuf_normalize_path(&pathbuf) < 0 && relative_base) {
+	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
 		error(_("unable to normalize alternate object path: %s"),
-		      pathbuf.buf);
+			pathbuf.buf);

This is a mis-indentation, it was OK in the pre-image, not now.

Strange, this came from "make style", and in the GitHub web UI, it shows
the next line as aligned with the opening ". Meh, I'll undo it.

Doesn't this leak? I've just skimmed strbuf_realpath_1() but e.g. in the
"REALPATH_MANY_MISSING" case it'll have allocated the "resolved" (the
&tmp you pass in here) and then "does a "goto error_out".

It then *resets* the strbuf, but doesn't release it, assuming that
you're going to pass it in again. So in that case we'd leak here, no?

I.e. a NULL return value from strbuf_realpath() doesn't mean that it
didn't allocate in the scratch area passed to it, so we need to
strbuf_release(&tmp) here too.

Yeah, you're right. At any rate, it's a lot of cognitive overload to
check if strbuf_realpath() will or won't allocate, so free()-ing in the
caller makes sense.

Separately, Peff mentioned that strbuf_realpath() not free()-ing is a
real bug, but I'll leave that for a future cleanup.

quoted

+		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
+'
+

I think this is better squashed in:
	
	diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
	index ce1954d0977..79eef5b4aa7 100755
	--- a/t/t7700-repack.sh
	+++ b/t/t7700-repack.sh
	@@ -91,13 +91,11 @@ test_expect_success 'loose objects in alternate ODB are not repacked' '
	 '
	 
	 test_expect_success '--local keeps packs when alternate is objectdir ' '
	-	git init alt_symlink &&
	+	test_when_finished "rm -rf repo" &&
	+	git init repo &&
	+	test_commit -C repo A &&
	 	(
	-		cd alt_symlink &&
	-		git init &&
	-		echo content >file4 &&
	-		git add file4 &&
	-		git commit -m commit_file4 &&
	+		cd repo &&
	 		git repack -a &&
	 		ls .git/objects/pack/*.pack >../expect &&
	 		ln -s objects .git/alt_objects &&

Because:

 * If it's not a setup for a later test let's call it "repo" and clean
   it up at the end.

 * The "file4" you're creating doesn't go with the existing pattern, the
   file{1..3} are created in the top-level .git, here you're making a
   file4 in another repo.

   This just calls it "A.t", and makes it with test_commit, since all
   you need is a dummy commit.

 * I think we typically use "find .. -type f", not "ls", see
   e.g. t5351-unpack-large-objects.sh, but I left it in-place. I think
   aside from that test there's some other "let's compare the packed
   before & after" in the test suite, but I can't remember offhand...

It seems like t7700-repack.sh itself isn't consistent either (which is
probably how I ended up with "ls"). I'll also leave it alone unless
someone has strong opinions.