Re: [PATCH] packed_ref_store: handle a packed-refs file that is a symlink

From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Michael Haggerty" <mhagger@alum.mit.edu>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"David Turner" <novalis@novalis.org>,
	"Brandon Williams" <bmwill@google.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] packed_ref_store: handle a packed-refs file that is a symlink
Date: Thu, 3 Jun 2021 15:39:53 -0400	[thread overview]
Message-ID: <YLkwCTcRT/9s8+5R@coredump.intra.peff.net> (raw)
In-Reply-To: <87y2bv0yvl.fsf@evledraar.gmail.com>

On Mon, May 31, 2021 at 04:18:46PM +0200, Ævar Arnfjörð Bjarmason wrote:

> 
> On Wed, Jul 26 2017, Michael Haggerty wrote:
> 
> > [...]
> > +test_expect_success 'pack symlinked packed-refs' '
> > +	# First make sure that symlinking works when reading:
> > +	git update-ref refs/heads/loosy refs/heads/master &&
> > +	git for-each-ref >all-refs-before &&
> > +	mv .git/packed-refs .git/my-deviant-packed-refs &&
> > +	ln -s my-deviant-packed-refs .git/packed-refs &&
> > +	git for-each-ref >all-refs-linked &&
> > +	test_cmp all-refs-before all-refs-linked &&
> > +	git pack-refs --all --prune &&
> > +	git for-each-ref >all-refs-packed &&
> > +	test_cmp all-refs-before all-refs-packed &&
> > +	test -h .git/packed-refs &&
> > +	test "$(readlink .git/packed-refs)" = "my-deviant-packed-refs"
> > +'
> 
> FWIW this broke tests on AIX because we can't assume readlink(1) exists
> at all. See d2addc3b96 (t7800: readlink may not be available,
> 2016-05-31) for a workaround.

Hmm. So obviously we can use a fix similar to the one in t7800 (though
it's sufficiently complicated that I'd be tempted to wrap it in a helper
function). There are a few other calls that could be changed, too.

But it's interesting to me that it sounds like the tests have been
broken on AIX for 4 years, and nobody noticed. I assume you ran into
this on the gcc build-farm machines. Our traditional approach for
portability has been: if somebody is using the platform and cares enough
to submit patches, then we'll support it. But testing on the build-farm
means preemptively finding these problems, whether anyone actually cares
about AIX or not. :)

I'm not really arguing either way here, just thinking out loud.

Preemptively finding portability problems may save work in the long
term. And people may even be using Git on AIX and just ignoring test
failures, or they have GNU coreutils installed anyway, etc. But it would
also save work if we can ignore platforms that nobody uses.

-Peff