[v2,0/7] reftable: stop using Git subsystems

Message

Patrick Steinhardt Nov. 8, 2024, 8:17 a.m. UTC

Hi,

this is the second version of my patch series that continues to detangle
the reftable library from the Git codebase.

Changes compared to v1:

  - Fix a commit message typo.

  - Document the values of the newly introduced reftable format IDs.

  - Include "reftable-basics.h" instead of "basics.h".

  - Adapt `stack_fsync()` to take write options as input instead of the
    whole stack.

Thanks!

Patrick

Patrick Steinhardt (7):
  reftable/system: move "dir.h" to its only user
  reftable: explicitly handle hash format IDs
  reftable/system: stop depending on "hash.h"
  reftable/stack: stop using `fsync_component()` directly
  reftable/system: provide thin wrapper for tempfile subsystem
  reftable/stack: drop only use of `get_locked_file_path()`
  reftable/system: provide thin wrapper for lockfile subsystem

 Makefile                            |   1 +
 refs/reftable-backend.c             |  19 +++-
 reftable/basics.c                   |  13 ++-
 reftable/basics.h                   |  10 +-
 reftable/merged.c                   |   4 +-
 reftable/merged.h                   |   3 +-
 reftable/reader.c                   |  14 ++-
 reftable/reader.h                   |   4 +-
 reftable/reftable-basics.h          |  13 +++
 reftable/reftable-merged.h          |   4 +-
 reftable/reftable-reader.h          |   2 +-
 reftable/reftable-record.h          |  12 +-
 reftable/reftable-writer.h          |   8 +-
 reftable/stack.c                    | 171 ++++++++++++++--------------
 reftable/system.c                   | 126 ++++++++++++++++++++
 reftable/system.h                   |  88 +++++++++++++-
 reftable/writer.c                   |  20 +++-
 t/helper/test-reftable.c            |   4 +-
 t/unit-tests/lib-reftable.c         |   5 +-
 t/unit-tests/lib-reftable.h         |   2 +-
 t/unit-tests/t-reftable-block.c     |  41 +++----
 t/unit-tests/t-reftable-merged.c    |  26 ++---
 t/unit-tests/t-reftable-pq.c        |   3 +-
 t/unit-tests/t-reftable-reader.c    |   4 +-
 t/unit-tests/t-reftable-readwrite.c |  41 +++----
 t/unit-tests/t-reftable-record.c    |  59 +++++-----
 t/unit-tests/t-reftable-stack.c     |  37 +++---
 27 files changed, 505 insertions(+), 229 deletions(-)
 create mode 100644 reftable/system.c

Range-diff against v1:
1:  036cc8f9d60 ! 1:  2b7d4e28529 reftable/system: move "dir.h" to its only user
    @@ Metadata
      ## Commit message ##
         reftable/system: move "dir.h" to its only user
     
    -    We still include "dir.h" in "reftable/system.h" evne though it is not
    +    We still include "dir.h" in "reftable/system.h" even though it is not
         used by anything but by a single unit test. Move it over into that unit
         test so that we don't accidentally use any functionality provided by it
         in the reftable codebase.
2:  c1bd8e2b3c4 ! 2:  38cfe85bf5b reftable: explicitly handle hash format IDs
    @@ reftable/basics.h: int common_prefix_size(struct reftable_buf *a, struct reftabl
      
     +/*
     + * Format IDs that identify the hash function used by a reftable. Note that
    -+ * these constants end up on disk and thus mustn't change.
    ++ * these constants end up on disk and thus mustn't change. The format IDs are
    ++ * "sha1" and "s256" in big endian, respectively.
     + */
     +#define REFTABLE_FORMAT_ID_SHA1   ((uint32_t) 0x73686131)
     +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536)
3:  b595668a5cd ! 3:  745c1a070dd reftable/system: stop depending on "hash.h"
    @@ reftable/merged.h: license that can be found in the LICENSE file or at
      #define MERGED_H
      
      #include "system.h"
    -+#include "basics.h"
    ++#include "reftable-basics.h"
      
      struct reftable_merged_table {
      	struct reftable_reader **readers;
4:  86269fc4fca ! 4:  7782652b975 reftable/stack: stop using `fsync_component()` directly
    @@ reftable/stack.c: static int stack_filename(struct reftable_buf *dest, struct re
      }
      
     -static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
    -+static int stack_fsync(struct reftable_stack *st, int fd)
    ++static int stack_fsync(const struct reftable_write_options *opts, int fd)
      {
     -	int *fdp = (int *)arg;
     -	return write_in_full(*fdp, data, sz);
    -+	if (st->opts.fsync)
    -+		return st->opts.fsync(fd);
    ++	if (opts->fsync)
    ++		return opts->fsync(fd);
     +	return fsync(fd);
      }
      
     -static int reftable_fd_flush(void *arg)
     +struct fd_writer {
    -+	struct reftable_stack *stack;
    ++	const struct reftable_write_options *opts;
     +	int fd;
     +};
     +
    @@ reftable/stack.c: static int stack_filename(struct reftable_buf *dest, struct re
     +static int fd_writer_flush(void *arg)
     +{
     +	struct fd_writer *writer = arg;
    -+	return stack_fsync(writer->stack, writer->fd);
    ++	return stack_fsync(writer->opts, writer->fd);
      }
      
      int reftable_new_stack(struct reftable_stack **dest, const char *dir,
    @@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
      	}
      
     -	err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd);
    -+	err = stack_fsync(add->stack, lock_file_fd);
    ++	err = stack_fsync(&add->stack->opts, lock_file_fd);
      	if (err < 0) {
      		err = REFTABLE_IO_ERROR;
      		goto done;
    @@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
      	struct reftable_writer *wr = NULL;
      	struct tempfile *tab_file = NULL;
     +	struct fd_writer writer = {
    -+		.stack = add->stack,
    ++		.opts = &add->stack->opts,
     +	};
      	int err = 0;
     -	int tab_fd;
    @@ reftable/stack.c: static int stack_compact_locked(struct reftable_stack *st,
      	struct reftable_buf tab_file_path = REFTABLE_BUF_INIT;
      	struct reftable_writer *wr = NULL;
     +	struct fd_writer writer=  {
    -+		.stack = st,
    ++		.opts = &st->opts,
     +	};
      	struct tempfile *tab_file;
     -	int tab_fd, err = 0;
    @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st,
      	}
      
     -	err = fsync_component(FSYNC_COMPONENT_REFERENCE, get_lock_file_fd(&tables_list_lock));
    -+	err = stack_fsync(st, get_lock_file_fd(&tables_list_lock));
    ++	err = stack_fsync(&st->opts, get_lock_file_fd(&tables_list_lock));
      	if (err < 0) {
      		err = REFTABLE_IO_ERROR;
      		unlink(new_table_path.buf);
5:  aca19955560 ! 5:  b15daefbc83 reftable/system: provide thin wrapper for tempfile subsystem
    @@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
     -	struct tempfile *tab_file = NULL;
     +	struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
      	struct fd_writer writer = {
    - 		.stack = add->stack,
    + 		.opts = &add->stack->opts,
      	};
     @@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
      	if (err < 0)
    @@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac
      	struct reftable_buf tab_file_path = REFTABLE_BUF_INIT;
     @@ reftable/stack.c: static int stack_compact_locked(struct reftable_stack *st,
      	struct fd_writer writer=  {
    - 		.stack = st,
    + 		.opts = &st->opts,
      	};
     -	struct tempfile *tab_file;
     +	struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
6:  74afe30974d = 6:  83949837a29 reftable/stack: drop only use of `get_locked_file_path()`
7:  71b213d6f8a ! 7:  80fe5bc5e10 reftable/system: provide thin wrapper for lockfile subsystem
    @@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
      		goto done;
      	}
      
    --	err = stack_fsync(add->stack, lock_file_fd);
    -+	err = stack_fsync(add->stack, add->tables_list_lock.fd);
    +-	err = stack_fsync(&add->stack->opts, lock_file_fd);
    ++	err = stack_fsync(&add->stack->opts, add->tables_list_lock.fd);
      	if (err < 0) {
      		err = REFTABLE_IO_ERROR;
      		goto done;
    @@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st,
      		goto done;
      	}
      
    --	err = stack_fsync(st, get_lock_file_fd(&tables_list_lock));
    -+	err = stack_fsync(st, tables_list_lock.fd);
    +-	err = stack_fsync(&st->opts, get_lock_file_fd(&tables_list_lock));
    ++	err = stack_fsync(&st->opts, tables_list_lock.fd);
      	if (err < 0) {
      		err = REFTABLE_IO_ERROR;
      		unlink(new_table_path.buf);