[v3,00/18] reftable: stop using "git-compat-util.h"

Message

Patrick Steinhardt Feb. 3, 2025, 8:03 a.m. UTC

Hi,

this patch series is the final step to fully decouple the reftable
library from the rest of the Git codebase. The goal of this is to make
the library reusable by other projects like libgit2 by simply copying
over the source files, making Git the canonical upstream for reftable
functionality.

This patch series stops using all kinds of helpers exposed by our
"git-compat-util.h" header and open-codes them instead. In order to keep
us from using these helpers by accident the final step is to pull out
POSIX-related bits and pieces into a new "compat/posix.h" header, which
the reftable library then uses instead of "git-compat-util.h".

The series is built on top of master at 5f8f7081f7 (The third batch,
2025-01-23) with ps/reftable-sign-compare at 33319b0976 (reftable:
address trivial -Wsign-compare warnings, 2025-01-20) merged into it.
There is a trivial merge conflict with ps/zlib-ng that can be solved
like this:

    diff --cc reftable/system.h
    index e4a8944a70,d02eacea8f..0000000000
    --- a/reftable/system.h
    +++ b/reftable/system.h
    @@@ -11,15 -11,9 +11,15 @@@ https://developers.google.com/open-sour
      
      /* This header glues the reftable library to the rest of Git */
      
     -#include "git-compat-util.h"
     +#include "compat/posix.h"
    - #include <zlib.h>
    + #include "compat/zlib-compat.h"
      
     +/*
     + * Return a random 32 bit integer. This function is expected to return
     + * pre-seeded data.
     + */
     +uint32_t reftable_rand(void);
     +
      /*
       * An implementation-specific temporary file. By making this specific to the
       * implementation it becomes possible to tie temporary files into any kind of

Changes in v2:
  - The splitup of Windows headers has broken compilation because some
    of the headers couldn't be found anymore. I've fixed this more
    generally by converting includes in "compat/" to always be relative
    to the project source directory, dropping the platform-specific
    `-Icompat/` include.
  - Explain why we don't port over `EWOULDBLOCK` handling.
  - Fix commit message typos.
  - Link to v1: https://lore.kernel.org/r/[e-mail]

Changes in v3:
  - Fix type of `total_read` variable used to track how many bytes we
    have read in `fd_read_lines()`.
  - Drop the patch use root-relative includes again. Let's rather
    discuss this outside of the scope of this series.
  - Link to v2: https://lore.kernel.org/r/[e-mail]

Thanks!

Patrick

---
Patrick Steinhardt (18):
      reftable/stack: stop using `read_in_full()`
      reftable/stack: stop using `write_in_full()`
      reftable/blocksource: stop using `xmmap()`
      reftable/record: stop using `COPY_ARRAY()`
      reftable/record: stop using `BUG()` in `reftable_record_init()`
      reftable/record: don't `BUG()` in `reftable_record_cmp()`
      reftable: stop using `BUG()` in trivial cases
      reftable/basics: stop using `st_mult()` in array allocators
      reftable/basics: provide wrappers for big endian conversion
      reftable/reader: stop using `ARRAY_SIZE()` macro
      reftable/system: introduce `reftable_rand()`
      reftable/stack: stop using `sleep_millisec()`
      reftable/basics: stop using `SWAP()` macro
      reftable/basics: stop using `UNUSED` annotation
      compat/mingw: split out POSIX-related bits
      git-compat-util.h: split out POSIX-emulating bits
      reftable: decouple from Git codebase by pulling in "compat/posix.h"
      Makefile: skip reftable library for Coccinelle

 Makefile                                |   2 +-
 compat/{mingw.c => mingw/compat-util.c} |   0
 compat/mingw/compat-util.h              | 220 +++++++++++++
 compat/{mingw.h => mingw/posix.h}       | 216 +------------
 compat/{msvc.c => msvc/compat-util.c}   |   2 +-
 compat/msvc/compat-util.h               |   7 +
 compat/{msvc.h => msvc/posix.h}         |   8 +-
 compat/posix.h                          | 541 ++++++++++++++++++++++++++++++++
 config.mak.uname                        |   6 +-
 contrib/buildsystems/CMakeLists.txt     |   2 +-
 git-compat-util.h                       | 535 +------------------------------
 meson.build                             |   8 +-
 reftable/basics.c                       |  19 --
 reftable/basics.h                       | 123 +++++++-
 reftable/block.c                        |  16 +-
 reftable/blocksource.c                  |  21 +-
 reftable/iter.c                         |  20 +-
 reftable/merged.c                       |  27 +-
 reftable/pq.c                           |  40 ++-
 reftable/pq.h                           |   2 +-
 reftable/reader.c                       |  33 +-
 reftable/record.c                       | 109 ++++---
 reftable/record.h                       |   6 +-
 reftable/stack.c                        |  52 ++-
 reftable/system.c                       |   7 +
 reftable/system.h                       |   9 +-
 reftable/writer.c                       |  29 +-
 t/unit-tests/t-reftable-basics.c        |  28 +-
 t/unit-tests/t-reftable-pq.c            |  22 +-
 t/unit-tests/t-reftable-record.c        |  42 ++-
 30 files changed, 1223 insertions(+), 929 deletions(-)

Range-diff versus v2:

 1:  e9364a8d05 !  1:  8488db3ac9 reftable/stack: stop using `read_in_full()`
    @@ reftable/stack.c: static int fd_read_lines(int fd, char ***namesp)
     -	if (read_in_full(fd, buf, size) != size) {
     -		err = REFTABLE_IO_ERROR;
     -		goto done;
    -+	for (size_t total_read = 0; total_read < (size_t) size; ) {
    ++	for (off_t total_read = 0; total_read < size; ) {
     +		ssize_t bytes_read = read(fd, buf + total_read, size - total_read);
     +		if (bytes_read < 0 && (errno == EAGAIN || errno == EINTR))
     +			continue;
 2:  41e411c00d =  2:  19db682279 reftable/stack: stop using `write_in_full()`
 3:  1abf9a77e4 =  3:  17fa559a68 reftable/blocksource: stop using `xmmap()`
 4:  458837e511 <  -:  ---------- reftable/record: stop using `COPY_ARRAY()`
 -:  ---------- >  4:  1667a37d38 reftable/record: stop using `COPY_ARRAY()`
 5:  a620950cb4 =  5:  1fe9563380 reftable/record: stop using `BUG()` in `reftable_record_init()`
 6:  0590780f55 =  6:  12c1fdc125 reftable/record: don't `BUG()` in `reftable_record_cmp()`
 7:  adcba48303 =  7:  8cfb77ce2c reftable: stop using `BUG()` in trivial cases
 8:  353d958e38 =  8:  c7227123fa reftable/basics: stop using `st_mult()` in array allocators
 9:  c6abf6ca1e =  9:  204a7ffed7 reftable/basics: provide wrappers for big endian conversion
10:  095c6805d1 = 10:  533f2cdbd5 reftable/reader: stop using `ARRAY_SIZE()` macro
11:  816a984361 = 11:  f5a9edc360 reftable/system: introduce `reftable_rand()`
12:  eb09c1d79d = 12:  4b066d5be8 reftable/stack: stop using `sleep_millisec()`
13:  1595e4d771 = 13:  8580d01ee2 reftable/basics: stop using `SWAP()` macro
14:  448ec74479 = 14:  72847f6d69 reftable/basics: stop using `UNUSED` annotation
15:  838f2bcd3a <  -:  ---------- compat: consistently resolve headers via project root
16:  cf6b915daf ! 15:  0348eed321 compat/mingw: split out POSIX-related bits
    @@ compat/mingw/compat-util.h (new)
     +#ifndef COMPAT_MINGW_COMPAT_UTIL_H
     +#define COMPAT_MINGW_COMPAT_UTIL_H
     +
    -+#include "compat/mingw/posix.h"
    ++#include "posix.h"
     +
     +struct config_context;
     +int mingw_core_config(const char *var, const char *value,
    @@ compat/mingw/posix.h: char *mingw_query_user_email(void);
     -#endif
     +#endif /* COMPAT_MINGW_POSIX_H */
     
    - ## compat/msvc.c ##
    + ## compat/msvc.c => compat/msvc/compat-util.c ##
     @@
      #include <conio.h>
    - #include "strbuf.h"
    + #include "../strbuf.h"
      
    --#include "compat/mingw.c"
    -+#include "compat/mingw/compat-util.c"
    +-#include "mingw.c"
    ++#include "mingw/compat-util.c"
     
    - ## compat/msvc.h ##
    -@@ compat/msvc.h: typedef int sigset_t;
    + ## compat/msvc/compat-util.h (new) ##
    +@@
    ++#ifndef COMPAT_MSVC_COMPAT_UTIL_H
    ++#define COMPAT_MSVC_COMPAT_UTIL_H
    ++
    ++#include "posix.h"
    ++#include "../mingw/compat-util.h"
    ++
    ++#endif /* COMPAT_MSVC_COMPAT_UTIL_H */
    +
    + ## compat/msvc.h => compat/msvc/posix.h ##
    +@@
    +-#ifndef __MSVC__HEAD
    +-#define __MSVC__HEAD
    ++#ifndef COMPAT_MSVC_POSIX_H
    ++#define COMPAT_MSVC_POSIX_H
    + 
    + #include <direct.h>
    + #include <process.h>
    +@@ compat/msvc/posix.h: typedef int sigset_t;
      /* open for reading, writing, or both (not in fcntl.h) */
      #define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
      
     -#include "compat/mingw.h"
    -+#include "compat/mingw/compat-util.h"
    ++#include "../mingw/posix.h"
      
    - #endif
    +-#endif
    ++#endif /* COMPAT_MSVC_POSIX_H */
     
      ## config.mak.uname ##
     @@ config.mak.uname: endif
    + 	AR = compat/vcbuild/scripts/lib.pl
    + 	CFLAGS =
    + 	BASIC_CFLAGS = -nologo -I. -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE
    +-	COMPAT_OBJS = compat/msvc.o compat/winansi.o \
    ++	COMPAT_OBJS = compat/msvc/compat-util.o compat/winansi.o \
    + 		compat/win32/flush.o \
    + 		compat/win32/path-utils.o \
    + 		compat/win32/pthread.o compat/win32/syslog.o \
    +@@ config.mak.uname: endif
      
      	EXTRA_PROGRAMS += headless-git$X
      
     -compat/msvc.o: compat/msvc.c compat/mingw.c GIT-CFLAGS
    -+compat/msvc.o: compat/msvc.c compat/mingw/compat-util.c GIT-CFLAGS
    ++compat/msvc/compat-util.o: compat/msvc/compat-util.c compat/mingw/compat-util.c GIT-CFLAGS
      endif
      ifeq ($(uname_S),Interix)
      	NO_INITGROUPS = YesPlease
     @@ config.mak.uname: ifeq ($(uname_S),MINGW)
      	BASIC_LDFLAGS += -municode
    - 	COMPAT_CFLAGS += -DNOGDI -Icompat/win32
    + 	COMPAT_CFLAGS += -DNOGDI -Icompat -Icompat/win32
      	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
     -	COMPAT_OBJS += compat/mingw.o compat/winansi.o \
     +	COMPAT_OBJS += compat/mingw/compat-util.o compat/winansi.o \
    @@ meson.build: if host_machine.system() == 'cygwin'
      elif host_machine.system() == 'windows'
        libgit_sources += [
     -    'compat/mingw.c',
    -+    'compat/mingw/compat-util.c',
          'compat/winansi.c',
          'compat/win32/flush.c',
          'compat/win32/path-utils.c',
    +@@ meson.build: elif host_machine.system() == 'windows'
    +   libgit_include_directories += 'compat/win32'
    +   if compiler.get_id() == 'msvc'
    +     libgit_include_directories += 'compat/vcbuild/include'
    ++    libgit_sources += 'compat/msvc/compat-util.c'
    ++  else
    ++    libgit_sources += 'compat/mingw/compat-util.c'
    +   endif
    + endif
    + 
     @@ meson.build: else
          error('Native regex support requested but not found')
      endif
17:  ae57e3f391 <  -:  ---------- compat/msvc: split out POSIX-related bits
18:  c32f39b777 ! 16:  141fd6a7cd git-compat-util.h: split out POSIX-emulating bits
    @@ compat/posix.h (new)
     +#endif
     +
     +#if defined(__MINGW32__)
    -+#include "compat/mingw/posix.h"
    ++#include "mingw/posix.h"
     +#elif defined(_MSC_VER)
    -+#include "compat/msvc/posix.h"
    ++#include "msvc/posix.h"
     +#else
     +#include <sys/utsname.h>
     +#include <sys/wait.h>
19:  496d30ee27 ! 17:  640f5e6376 reftable: decouple from Git codebase by pulling in "compat/posix.h"
    @@ reftable/system.h: license that can be found in the LICENSE file or at
      /* This header glues the reftable library to the rest of Git */
      
     -#include "git-compat-util.h"
    -+#include "compat/posix.h"
    ++#include "../compat/posix.h"
     +#include <zlib.h>
      
      /*
20:  d257172bf7 = 18:  2f17a1963d Makefile: skip reftable library for Coccinelle

---
base-commit: 8047765d092881ec4aef7dfc57772161eee7f0f5
change-id: 20241119-pks-reftable-drop-git-compat-util-470f2bfde562