[PATCH 0/3] hn/reftable "fixes" for OpenBSD

[PATCH 0/3] hn/reftable "fixes" for OpenBSD

[Carlo Marcelo Arenas Belón]

While building "seen" in OpenBSD, noticed the following issues as part
of reftable support.

the first patch also affected Linux and was indeed one of the two reasons
for the failed CI runs of that branch, hence why it might be worth
squashing into the corresponding commit as the subject implies.

the second patch might be worth squashing as well, but neither address
the segfault in t0031.14 which manifests as :

  2d732e1 refs/heads/primary@{9}: commit (initial): number 1
  ++ grep 'commit: number 10' output
  357ed45 refs/heads/primary@{0}: commit: number 10
  ++ git gc
  error: reflog died of signal 11
  fatal: failed to run reflog
  error: last command exited with $?=128
  not ok 14 - reflog, repack

and that correspond to a call of `git reflog expire --all` with the
following backtrace :

  #0  hashmap_entry_init (hash=<optimized out>, e=0xbfba1384) at hashmap.h:317
  #1  hashmap_get_from_hash (keydata=0x0, hash=<optimized out>, map=0xa027208)
      at hashmap.h:378
  #2  oidmap_get (map=0xa027208, key=0x0) at oidmap.c:38
  #3  0x08202dae in do_lookup_replace_object (r=0x83fc180 <the_repo>, oid=0x0)
      at replace-object.c:76
  #4  0x081b59d3 in lookup_replace_object (oid=0x0, r=0x83fc180 <the_repo>)
      at replace-object.h:40
  #5  parse_object (r=0x83fc180 <the_repo>, oid=0x0) at object.c:271
  #6  0x08134e87 in lookup_commit_reference_gently (r=0x83fc180 <the_repo>,
      oid=0x0, quiet=1) at commit.c:35
  #7  0x080d6e39 in reflog_expiry_prepare (
      refname=0xa026fb4 "refs/heads/primary", oid=0x0, cb_data=0xbfba161c)
      at builtin/reflog.c:363
  #8  0x081f9325 in git_reftable_reflog_expire (ref_store=0xa026c40,
      refname=0xa026fb4 "refs/heads/primary", oid=0x0, flags=0,
      prepare_fn=0x80d6c80 <reflog_expiry_prepare>,
      should_prune_fn=0x80d72b0 <should_expire_reflog_ent>,
      cleanup_fn=0x80d6990 <reflog_expiry_cleanup>, policy_cb_data=0xbfba161c)
      at refs/reftable-backend.c:1511
  #9  0x081ef820 in refs_reflog_expire (policy_cb_data=0xbfba161c,
      cleanup_fn=0x80d6990 <reflog_expiry_cleanup>,
      should_prune_fn=0x80d72b0 <should_expire_reflog_ent>,
      prepare_fn=0x80d6c80 <reflog_expiry_prepare>, flags=0, oid=0x0,
      refname=0xa026fb4 "refs/heads/primary", refs=<optimized out>)
      at refs.c:2408
  #10 reflog_expire (refname=0xa026fb4 "refs/heads/primary", oid=0x0, flags=0,
      prepare_fn=0x80d6c80 <reflog_expiry_prepare>,
      should_prune_fn=0x80d72b0 <should_expire_reflog_ent>,
      cleanup_fn=0x80d6990 <reflog_expiry_cleanup>, policy_cb_data=0xbfba161c)
      at refs.c:2420
  #11 0x080d786f in cmd_reflog_expire (argc=<optimized out>,
      argv=<optimized out>, prefix=<optimized out>) at builtin/reflog.c:634
  #12 0x0804e9fb in run_builtin (argv=<optimized out>, argc=<optimized out>,
      p=<optimized out>) at git.c:461
  #13 handle_builtin (argc=3, argv=argv@entry=0xbfba2208) at git.c:716
  #14 0x0804edcb in run_argv (argv=0xbfba1ffc, argcp=0xbfba20a0) at git.c:783
  #15 cmd_main (argc=3, argv=0xbfba2204) at git.c:914
  #16 0x0804d8dc in main (argc=4, argv=0xbfba2204) at common-main.c:52

Carlo Marcelo Arenas Belón (3):
  fixup! Provide zlib's uncompress2 from compat/zlib-compat.c
  reftable: clarify zlib version dependency
  openbsd: allow reftable building with zlib 1.2.3

 Makefile         | 2 +-
 config.mak.uname | 1 +
 reftable/block.c | 4 +++-
 3 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.32.0.826.g286871f41a

[PATCH 1/3] fixup! Provide zlib's uncompress2 from compat/zlib-compat.c

[Carlo Marcelo Arenas Belón]

124b6e20ef (Provide zlib's uncompress2 from compat/zlib-compat.c,
2021-07-20) adds a compatibility replacement function for uncompress2
that could be used for building with zlib < 1.2.9, but adds it to libgit.

this could result in link failures depending on the library order used
by the link command as shown in (from the 32bit linux CI task for the seen
branch):

  reftable/libreftable.a(block.o): In function `block_reader_init':
  /__w/git/git/reftable/block.c:222: undefined reference to `uncompress2'
  collect2: error: ld returned 1 exit status
  Makefile:2649: recipe for target 'git-daemon' failed

add it instead to the libreftable library that is also the current user.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 94ba729376..d1c2a8acc1 100644
--- a/Makefile
+++ b/Makefile
@@ -1760,7 +1760,7 @@ endif
 
 ifdef NO_UNCOMPRESS2
 	BASIC_CFLAGS += -DNO_UNCOMPRESS2
-	LIB_OBJS += compat/zlib-uncompress2.o
+	REFTABLE_OBJS += compat/zlib-uncompress2.o
 endif
 
 ifdef NO_POSIX_GOODIES
-- 
2.32.0.826.g286871f41a

[PATCH 2/3] reftable: clarify zlib version dependency

[Carlo Marcelo Arenas Belón]

3541411189 (reftable: reading/writing blocks, 2021-07-20) add a comment
explaining that a newer zlib version is required but being a little vague
about the specifics

make sure the zlib version is mentioned to aid on deciding to enable or
not the NO_UNCOMPRESS2 compatibility layer

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
Squashing this as part of a reroll of hn/reftable might be preferable

 reftable/block.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/reftable/block.c b/reftable/block.c
index 92f8e5abfa..eb5268dd3a 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -16,7 +16,9 @@ license that can be found in the LICENSE file or at
 #include <zlib.h>
 
 #ifdef NO_UNCOMPRESS2
-/* This is uncompress2, which is only available in zlib as of 2017.
+/*
+ * This is uncompress2, which is only available in zlib >= 1.2.9
+ * (released as of early 2017)
  */
 int uncompress2(Bytef *dest, uLongf *destLen, const Bytef *source,
 		uLong *sourceLen);
-- 
2.32.0.826.g286871f41a

[PATCH 3/3] openbsd: allow reftable building with zlib 1.2.3

[Carlo Marcelo Arenas Belón]

all released versions of OpenBSD (up to 6.9) include an older than 1.2.9
version of zlib in base.

enable the compat uncompress2 function until is no longer needed (likely
at least until the next 2 future releases are not yet EOL)

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 59726d6538..074dc2324e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -260,6 +260,7 @@ ifeq ($(uname_S),FreeBSD)
 	FILENO_IS_A_MACRO = UnfortunatelyYes
 endif
 ifeq ($(uname_S),OpenBSD)
+	NO_UNCOMPRESS2 = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-- 
2.32.0.826.g286871f41a

[PATCH 4/3] fixup! reftable: add dump utility

[Carlo Marcelo Arenas Belón]

avoid _XOPEN_SOURCE in alpine by making sure git-compat-util.h is included
first (through hash.h) as recommended and therefore avoid:

  In file included from ./hash.h:4,
                   from reftable/dump.c:19:
  ./git-compat-util.h:147: warning: "_XOPEN_SOURCE" redefined
    147 | #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
        |
  In file included from /usr/include/unistd.h:8,
                   from reftable/reftable-writer.h:15,
                   from reftable/dump.c:14:
  /usr/include/features.h:16: note: this is the location of the previous definition
     16 | #define _XOPEN_SOURCE 700
        |

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
This allows for the alpine CI job to build, but will still fail with the
reported segfault in the subcommand for `git reflog`.

It is important to note that it only happens in "seen" though, as the same
code merged to master pass that test (even if it might fail somewhere else)

 reftable/dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/reftable/dump.c b/reftable/dump.c
index 668cfa8996..eb7b2b4161 100644
--- a/reftable/dump.c
+++ b/reftable/dump.c
@@ -6,6 +6,7 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
+#include "hash.h"
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 #include "reftable-merged.h"
@@ -16,7 +17,6 @@ license that can be found in the LICENSE file or at
 #include "reftable-reader.h"
 #include "reftable-stack.h"
 #include "reftable-generic.h"
-#include "hash.h"
 
 #include <stddef.h>
 #include <stdio.h>
-- 
2.33.0.rc0.443.g98cc19b6c0

Re: [PATCH 4/3] fixup! reftable: add dump utility

[Junio C Hamano]

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

> avoid _XOPEN_SOURCE in alpine by making sure git-compat-util.h is included
> first (through hash.h) as recommended and therefore avoid:

The headers cache.h and builtin.h are the only ones whose inclusion
allows the source to omit an explicit inclusion of git-compat-util.h
and we'd prefer the inclusion of git-compat-util.h to be explicit
here, not through hash.h.  Just including git-compat-util.h directly
without moving inclusion of hash.h should be sufficient.


diff --git a/reftable/dump.c b/reftable/dump.c
index 668cfa8996..eb7b2b4161 100644
--- a/reftable/dump.c
+++ b/reftable/dump.c
@@ -6,6 +6,7 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
+#include "git-compat-util.h"
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 #include "reftable-merged.h"

[PATCH v2 0/7] hn/reftable "fixes" for BSD

[Carlo Marcelo Arenas Belón]

A few issues I noticed while buiding "seen" in some BSD systems.

Patches 1-3 aren't changed from v1, and 4 had the fixes suggested by
Junio[1] added.

Patch 5 affects an EOL NetBSD 7.2 system and therefore might be dropped
but I thought it was safe/generic enough and might become useful in
other old systems as well.

The last 2 patches are a well known recurring issue, that even prompted
a explicit documentation update in cc0c42975a (CodingGuidelines: spell
out post-C89 rules, 2019-07-16).

All these patches are expected to be applied on top of hn/reftable and
hopefully absorved and discarded.

Carlo Marcelo Arenas Belón (7):
  fixup! Provide zlib's uncompress2 from compat/zlib-compat.c
  reftable: clarify zlib version dependency
  openbsd: allow reftable building with zlib 1.2.3
  fixup! reftable: add dump utility
  fixup! Provide zlib's uncompress2 from compat/zlib-compat.c
  fixup! reftable: reftable file level tests
  fixup! reftable: (de)serialization for the polymorphic record type

 Makefile                  | 2 +-
 compat/zlib-uncompress2.c | 3 +++
 config.mak.uname          | 1 +
 reftable/block.c          | 4 +++-
 reftable/dump.c           | 1 +
 reftable/readwrite_test.c | 4 ++--
 reftable/record_test.c    | 4 +++-
 7 files changed, 14 insertions(+), 5 deletions(-)

[1] https://lore.kernel.org/git/xmqq35rpqg27.fsf@gitster.g/
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 1/7] fixup! Provide zlib's uncompress2 from compat/zlib-compat.c

[Carlo Marcelo Arenas Belón]

124b6e20ef (Provide zlib's uncompress2 from compat/zlib-compat.c,
2021-07-20) adds a compatibility replacement function for uncompress2
that could be used for building with zlib < 1.2.9, but adds it to libgit.

this could result in link failures depending on the library order used
by the link command as shown in (from the 32bit linux CI task for the seen
branch):

  reftable/libreftable.a(block.o): In function `block_reader_init':
  /__w/git/git/reftable/block.c:222: undefined reference to `uncompress2'
  collect2: error: ld returned 1 exit status
  Makefile:2649: recipe for target 'git-daemon' failed

add it instead to the libreftable library that is also the current user.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 6014f74a1b..b4f5cc6fc0 100644
--- a/Makefile
+++ b/Makefile
@@ -1739,7 +1739,7 @@ endif
 
 ifdef NO_UNCOMPRESS2
 	BASIC_CFLAGS += -DNO_UNCOMPRESS2
-	LIB_OBJS += compat/zlib-uncompress2.o
+	REFTABLE_OBJS += compat/zlib-uncompress2.o
 endif
 
 ifdef NO_POSIX_GOODIES
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 2/7] reftable: clarify zlib version dependency

[Carlo Marcelo Arenas Belón]

3541411189 (reftable: reading/writing blocks, 2021-07-20) add a comment
explaining that a newer zlib version is required but being a little vague
about the specifics

make sure the zlib version is mentioned to aid on deciding to enable or
not the NO_UNCOMPRESS2 compatibility layer

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 reftable/block.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/reftable/block.c b/reftable/block.c
index 92f8e5abfa..eb5268dd3a 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -16,7 +16,9 @@ license that can be found in the LICENSE file or at
 #include <zlib.h>
 
 #ifdef NO_UNCOMPRESS2
-/* This is uncompress2, which is only available in zlib as of 2017.
+/*
+ * This is uncompress2, which is only available in zlib >= 1.2.9
+ * (released as of early 2017)
  */
 int uncompress2(Bytef *dest, uLongf *destLen, const Bytef *source,
 		uLong *sourceLen);
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 3/7] openbsd: allow reftable building with zlib 1.2.3

[Carlo Marcelo Arenas Belón]

all released versions of OpenBSD (up to 6.9) include an older than 1.2.9
version of zlib in base.

enable the compat uncompress2 function until is no longer needed (likely
at least until the next 2 future releases are not yet EOL)

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index a2b156e338..019c88d5df 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -256,6 +256,7 @@ ifeq ($(uname_S),FreeBSD)
 	FILENO_IS_A_MACRO = UnfortunatelyYes
 endif
 ifeq ($(uname_S),OpenBSD)
+	NO_UNCOMPRESS2 = YesPlease
 	NO_STRCASESTR = YesPlease
 	NO_MEMMEM = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 4/7] fixup! reftable: add dump utility

[Carlo Marcelo Arenas Belón]

avoid _XOPEN_SOURCE redefinition in alpine by making sure git-compat-util.h
is included first (as recommended in CodingGuidelines) and therefore avoid:

  In file included from ./hash.h:4,
                   from reftable/dump.c:19:
  ./git-compat-util.h:147: warning: "_XOPEN_SOURCE" redefined
    147 | #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
        |
  In file included from /usr/include/unistd.h:8,
                   from reftable/reftable-writer.h:15,
                   from reftable/dump.c:14:
  /usr/include/features.h:16: note: this is the location of the previous definition
     16 | #define _XOPEN_SOURCE 700
        |

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
Using code contributed by Junio and only reusing the commit from my original

 reftable/dump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/reftable/dump.c b/reftable/dump.c
index 668cfa8996..ef017701b6 100644
--- a/reftable/dump.c
+++ b/reftable/dump.c
@@ -6,6 +6,7 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
+#include "git-compat-util.h"
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 #include "reftable-merged.h"
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 5/7] fixup! Provide zlib's uncompress2 from compat/zlib-compat.c

[Carlo Marcelo Arenas Belón]

the code imported from zlib 1.2.11 uses an optional feature from 1.2.5.2
that could cause compilation errors with older versions.

make sure it is defined and disabled in those cases to avoid
building failures.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 compat/zlib-uncompress2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/zlib-uncompress2.c b/compat/zlib-uncompress2.c
index 23b72b49c4..f49f680567 100644
--- a/compat/zlib-uncompress2.c
+++ b/compat/zlib-uncompress2.c
@@ -14,6 +14,9 @@
  */
 
 #include <zlib.h>
+#ifndef z_const
+#define z_const
+#endif
 
 /* clang-format off */
 
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 6/7] fixup! reftable: reftable file level tests

[Carlo Marcelo Arenas Belón]

C99 style loop initializers still problematic with gnu89 C in older
compilers.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 reftable/readwrite_test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index 42ca48f83c..146b00cb02 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -120,7 +120,7 @@ static void test_log_buffer_size(void)
 	struct reftable_write_options opts = {
 		.block_size = 4096,
 	};
-	int err;
+	int err, i;
 	struct reftable_log_record log = { .refname = "refs/heads/master",
 					   .update_index = 0xa,
 					   .value_type = REFTABLE_LOG_UPDATE,
@@ -138,7 +138,7 @@ static void test_log_buffer_size(void)
 	   hash, to ensure that the compressed part is larger than the original.
 	*/
 	uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];
-	for (int i = 0; i < GIT_SHA1_RAWSZ; i++) {
+	for (i = 0; i < GIT_SHA1_RAWSZ; i++) {
 		hash1[i] = (uint8_t)(rand() % 256);
 		hash2[i] = (uint8_t)(rand() % 256);
 	}
-- 
2.33.0.rc2.476.g1b09a32a73

[PATCH v2 7/7] fixup! reftable: (de)serialization for the polymorphic record type

[Carlo Marcelo Arenas Belón]

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 reftable/record_test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/reftable/record_test.c b/reftable/record_test.c
index bf5d072b20..519c0bbf18 100644
--- a/reftable/record_test.c
+++ b/reftable/record_test.c
@@ -207,9 +207,11 @@ static void test_reftable_log_record_roundtrip(void)
 			.value_type = REFTABLE_LOG_DELETION,
 		}
 	};
+	int i;
+
 	set_test_hash(in[0].update.new_hash, 1);
 	set_test_hash(in[0].update.old_hash, 2);
-	for (int i = 0; i < ARRAY_SIZE(in); i++) {
+	for (i = 0; i < ARRAY_SIZE(in); i++) {
 		struct reftable_record rec = { NULL };
 		struct strbuf key = STRBUF_INIT;
 		uint8_t buffer[1024] = { 0 };
-- 
2.33.0.rc2.476.g1b09a32a73