git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 4, 2021, 21:12 UTC
Message-ID
<87o8cl5n31.fsf@evledraar.gmail.com>
In-Reply-To
<YLkwCTcRT/9s8+5R@coredump.intra.peff.net>
On Thu, Jun 03 2021, Jeff King wrote:
Show 43 quoted lines
> 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/
Previous: Felipe Contreras
Message 52 of 52 in “Create a reference backend for packed refs”
  1. 00/30 Create a reference backend for packed refsMichael Haggerty, Jul 1, 2017
  2. 01/30 t1408: add a test of stale packed refs covered by loose refsMichael Haggerty, Jul 1, 2017
  3. 03/30 packed_ref_store: new structMichael Haggerty, Jul 1, 2017
  4. 04/30 packed_ref_store: move `packed_refs_path` hereMichael Haggerty, Jul 1, 2017
  5. 02/30 add_packed_ref(): teach function to overwrite existing refsMichael Haggerty, Jul 1, 2017
  6. 06/30 clear_packed_ref_cache(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  7. 05/30 packed_ref_store: move `packed_refs_lock` member hereMichael Haggerty, Jul 1, 2017
  8. 09/30 get_packed_refs(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  9. 07/30 validate_packed_ref_cache(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  10. 12/30 commit_packed_refs(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  11. 08/30 get_packed_ref_cache(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  12. 10/30 add_packed_ref(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  13. 11/30 lock_packed_refs(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  14. 16/30 packed_peel_ref(): new function, extracted from `files_peel_ref()`Michael Haggerty, Jul 1, 2017
  15. 15/30 repack_without_refs(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  16. 14/30 get_packed_ref(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  17. 17/30 packed_ref_store: support iterationMichael Haggerty, Jul 1, 2017
  18. 19/30 packed-backend: new module for handling packed referencesMichael Haggerty, Jul 1, 2017
  19. 18/30 packed_read_raw_ref(): new function, replacing `resolve_packed_ref()`Michael Haggerty, Jul 1, 2017
  20. 22/30 commit_packed_refs(): use a staging file separate from the lockfileMichael Haggerty, Jul 1, 2017
  21. 20/30 packed_ref_store: make class into a subclass of `ref_store`Michael Haggerty, Jul 1, 2017
  22. 13/30 rollback_packed_refs(): take a `packed_ref_store *` parameterMichael Haggerty, Jul 1, 2017
  23. 23/30 packed_refs_lock(): function renamed from lock_packed_refs()Michael Haggerty, Jul 1, 2017
  24. 28/30 repack_without_refs(): don't lock or unlock the packed refsMichael Haggerty, Jul 1, 2017
  25. 29/30 t3210: add some tests of bogus packed-refs file contentsMichael Haggerty, Jul 1, 2017
  26. 26/30 clear_packed_ref_cache(): don't protest if the lock is heldMichael Haggerty, Jul 1, 2017
  27. 30/30 read_packed_refs(): die if `packed-refs` contains bogus dataMichael Haggerty, Jul 1, 2017
  28. 27/30 commit_packed_refs(): remove call to `packed_refs_unlock()`Michael Haggerty, Jul 1, 2017
  29. 21/30 commit_packed_refs(): report errors rather than dyingMichael Haggerty, Jul 1, 2017
  30. 25/30 packed_refs_unlock(), packed_refs_is_locked(): new functionsMichael Haggerty, Jul 1, 2017
  31. 24/30 packed_refs_lock(): report errors via a `struct strbuf *err`Michael Haggerty, Jul 1, 2017
  32. Jeff KingJul 5, 2017
  33. Stefan BellerJul 20, 2017
  34. Jonathan NiederJul 20, 2017
  35. packed_ref_store: handle a packed-refs file that is a symlinkMichael Haggerty, Jul 26, 2017
  36. Stefan BellerJul 27, 2017
  37. Jonathan NiederJul 27, 2017
  38. Michael HaggertyJul 27, 2017
  39. Junio C HamanoJul 27, 2017
  40. Jeff KingJul 27, 2017
  41. Junio C HamanoJul 27, 2017
  42. Michael HaggertyJul 28, 2017
  43. Ævar Arnfjörð BjarmasonMay 31, 2021
  44. Jeff KingJun 3, 2021
  45. t: use portable wrapper for readlink(1)Jeff King, Jun 3, 2021
  46. Ævar Arnfjörð BjarmasonJun 4, 2021
  47. Felipe ContrerasJun 3, 2021
  48. Jeff KingJun 3, 2021
  49. Felipe ContrerasJun 3, 2021
  50. Ævar Arnfjörð BjarmasonJun 4, 2021
  51. Felipe ContrerasJun 5, 2021
  52. Ævar Arnfjörð BjarmasonJun 4, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.