mbox series

[v3,00/15] reftable: drop generic `reftable_table` interface

Message ID cover.1724308389.git.ps@pks.im (mailing list archive)
Headers show
Series reftable: drop generic `reftable_table` interface | expand

Message

Patrick Steinhardt Aug. 22, 2024, 6:34 a.m. UTC
Hi,

this is the third version of my patch series that gets rid of the
generic `reftable_table` interface. It made it way harder to understand
the reftable code base and is not really required nowadays anymore where
we have generic re-seekable reftable iterators.

There is only a single change compared to v2, which updates one of the
commit messages to explain why it is fine to drop tests for the printing
functionality.

This patch series continues to build on top of 25673b1c47 (The third
batch, 2024-08-07) with Junio's ps/reftable-stack-compaction at
f234df07f6 (reftable/stack: handle locked tables during auto-compaction,
2024-08-08) merged into it.

Thanks!

Patrick

Patrick Steinhardt (15):
  reftable/merged: expose functions to initialize iterators
  reftable/merged: rename `reftable_new_merged_table()`
  reftable/merged: stop using generic tables in the merged table
  reftable/stack: open-code reading refs
  reftable/iter: drop double-checking logic
  reftable/generic: move generic iterator code into iterator interface
  reftable/dump: drop unused `compact_stack()`
  t/helper: inline `reftable_dump_main()`
  t/helper: inline `reftable_reader_print_file()`
  t/helper: inline `reftable_stack_print_directory()`
  t/helper: inline `reftable_table_print()`
  t/helper: inline printing of reftable records
  t/helper: use `hash_to_hex_algop()` to print hashes
  t/helper: refactor to not use `struct reftable_table`
  reftable/generic: drop interface

 Makefile                         |   2 -
 reftable/dump.c                  | 111 ---------------
 reftable/generic.c               | 229 -------------------------------
 reftable/generic.h               |  37 -----
 reftable/iter.c                  | 126 ++++++++++++++---
 reftable/iter.h                  |  30 +++-
 reftable/merged.c                |  72 ++++------
 reftable/merged.h                |   4 +-
 reftable/reader.c                |  70 +---------
 reftable/reader.h                |   4 +
 reftable/record.c                | 127 -----------------
 reftable/record.h                |   1 -
 reftable/reftable-generic.h      |  47 -------
 reftable/reftable-merged.h       |  26 ++--
 reftable/reftable-reader.h       |   9 --
 reftable/reftable-record.h       |   8 --
 reftable/reftable-stack.h        |   3 -
 reftable/reftable-tests.h        |   1 -
 reftable/stack.c                 |  94 ++++++-------
 reftable/stack_test.c            |  29 ++--
 t/helper/test-reftable.c         | 189 ++++++++++++++++++++++++-
 t/unit-tests/t-reftable-merged.c |  17 +--
 22 files changed, 422 insertions(+), 814 deletions(-)
 delete mode 100644 reftable/dump.c
 delete mode 100644 reftable/generic.c
 delete mode 100644 reftable/generic.h
 delete mode 100644 reftable/reftable-generic.h

Range-diff against v2:
 1:  472c169b501 =  1:  472c169b501 reftable/merged: expose functions to initialize iterators
 2:  bc6f1cd8c1b =  2:  bc6f1cd8c1b reftable/merged: rename `reftable_new_merged_table()`
 3:  58e91ab4b34 =  3:  58e91ab4b34 reftable/merged: stop using generic tables in the merged table
 4:  6ba3fcee411 =  4:  6ba3fcee411 reftable/stack: open-code reading refs
 5:  cac08a934c5 =  5:  cac08a934c5 reftable/iter: drop double-checking logic
 6:  103262dc79c =  6:  103262dc79c reftable/generic: move generic iterator code into iterator interface
 7:  4011fa65d81 =  7:  4011fa65d81 reftable/dump: drop unused `compact_stack()`
 8:  ceaa296bfd4 =  8:  ceaa296bfd4 t/helper: inline `reftable_dump_main()`
 9:  a62e4612e97 =  9:  a62e4612e97 t/helper: inline `reftable_reader_print_file()`
10:  7acfe4fecc5 ! 10:  242c179df5f t/helper: inline `reftable_stack_print_directory()`
    @@ Commit message
         Move `reftable_stack_print_directory()` into the "dump-reftable" helper.
         This follows the same reasoning as the preceding commit.
     
    +    Note that this requires us to remove the tests for this functionality in
    +    `reftable/stack_test.c`. The test does not really add much anyway,
    +    because all it verifies is that we do not crash or run into an error,
    +    and it specifically doesn't check the outputted data. Also, as the code
    +    is now part of the test helper, it doesn't make much sense to have a
    +    unit test for it in the first place.
    +
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## reftable/reftable-stack.h ##
11:  8bd53a1a656 = 11:  a05e2060996 t/helper: inline `reftable_table_print()`
12:  c50aabbb804 = 12:  ee22a08e11e t/helper: inline printing of reftable records
13:  5498395872c = 13:  0a3c619e842 t/helper: use `hash_to_hex_algop()` to print hashes
14:  5390be75c37 = 14:  8eab399dfc6 t/helper: refactor to not use `struct reftable_table`
15:  5aeab8ee077 = 15:  b5d7b5679b5 reftable/generic: drop interface

Comments

karthik nayak Aug. 22, 2024, 8:03 a.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

[snip]

> Range-diff against v2:
>  1:  472c169b501 =  1:  472c169b501 reftable/merged: expose functions to initialize iterators
>  2:  bc6f1cd8c1b =  2:  bc6f1cd8c1b reftable/merged: rename `reftable_new_merged_table()`
>  3:  58e91ab4b34 =  3:  58e91ab4b34 reftable/merged: stop using generic tables in the merged table
>  4:  6ba3fcee411 =  4:  6ba3fcee411 reftable/stack: open-code reading refs
>  5:  cac08a934c5 =  5:  cac08a934c5 reftable/iter: drop double-checking logic
>  6:  103262dc79c =  6:  103262dc79c reftable/generic: move generic iterator code into iterator interface
>  7:  4011fa65d81 =  7:  4011fa65d81 reftable/dump: drop unused `compact_stack()`
>  8:  ceaa296bfd4 =  8:  ceaa296bfd4 t/helper: inline `reftable_dump_main()`
>  9:  a62e4612e97 =  9:  a62e4612e97 t/helper: inline `reftable_reader_print_file()`
> 10:  7acfe4fecc5 ! 10:  242c179df5f t/helper: inline `reftable_stack_print_directory()`
>     @@ Commit message
>          Move `reftable_stack_print_directory()` into the "dump-reftable" helper.
>          This follows the same reasoning as the preceding commit.
>
>     +    Note that this requires us to remove the tests for this functionality in
>     +    `reftable/stack_test.c`. The test does not really add much anyway,
>     +    because all it verifies is that we do not crash or run into an error,
>     +    and it specifically doesn't check the outputted data. Also, as the code
>     +    is now part of the test helper, it doesn't make much sense to have a
>     +    unit test for it in the first place.
>     +
>          Signed-off-by: Patrick Steinhardt <ps@pks.im>
>
>       ## reftable/reftable-stack.h ##
> 11:  8bd53a1a656 = 11:  a05e2060996 t/helper: inline `reftable_table_print()`
> 12:  c50aabbb804 = 12:  ee22a08e11e t/helper: inline printing of reftable records
> 13:  5498395872c = 13:  0a3c619e842 t/helper: use `hash_to_hex_algop()` to print hashes
> 14:  5390be75c37 = 14:  8eab399dfc6 t/helper: refactor to not use `struct reftable_table`
> 15:  5aeab8ee077 = 15:  b5d7b5679b5 reftable/generic: drop interface
> --
> 2.46.0.164.g477ce5ccd6.dirty

This was the only change I requested in the previous version. So looks
good to me now!

Thanks
Karthik
Junio C Hamano Aug. 22, 2024, 5:11 p.m. UTC | #2
Patrick Steinhardt <ps@pks.im> writes:

> this is the third version of my patch series that gets rid of the
> generic `reftable_table` interface. It made it way harder to understand
> the reftable code base and is not really required nowadays anymore where
> we have generic re-seekable reftable iterators.

Looking good.  Let me mark it for 'next'.  Thanks.