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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Jun 03 2021, Jeff King wrote:

> 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.

I believe that the main packager for AIX is Michael Felt (CC'd). He's
occasionally posted here on-list, e.g. at [1], the package is at [2].

I think in the grand scheme of things if we break things completely for
IBM xlc and/or Oracle's C compiler git won't be that much worse
off. Plenty of third party packages for those platforms simply use the
full GNU toolchain.

From memory the most commonly used package for Solaris is built with the
full GNU toolchain, so in that sense portability to that platform isn't
a practical issue.

I just send patches for these as part of doing rc-phase testing on the
GCC farm to see if anything's broken since the last release. Issues we
have on OSX, Linux, Windows etc. tend to be resolved already, which
leaves more obscure issues on these platforms.

I've said before that I do think that porting to "obscure" platforms
like those has some value in itself, even now there's e.g. C compiler
warnings on those proprietary compilers that cover blind spots that
neither gcc nor clang have. Often they're false alarms, but there's the
occasional gold nuggets in there.

E.g. I have some post-release patches queued up for fixing various cmd_*
functions of ours to use "return" instead of "exit(code)". Even though
they return "int" neither GCC nor Clang complain about the early exit,
but SunCC does. As a result we don't run the usual teardown in git.c for
some built-ins.

I was trying to get AIX in particular to the point of passing 100% a
while ago, but sort of lost steam with Junio not wanting to take my
patches at [3].

Without those re-running the tests usually takes manual intervention on
the box, so and since my usual test setup is more of a fire-and-forget
affair I mostly stopped bothering with the AIX box as a result...

1. https://lore.kernel.org/git/1200106e-b75d-5b15-0608-427cd923578a@felt.demon.nl/
2. http://www.aixtools.net/index.php/git
3. https://lore.kernel.org/git/patch-1.6-3330cdbccc0-20210329T161723Z-avarab@gmail.com/