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); -- 2.47.0.229.g8f8d6eee53.dirty