On 25/03/31 10:41AM, Patrick Steinhardt wrote:
> The logic to print individual blocks in a table is hosted in the
> reftable library. This is only the case due to historical reasons though
> because users of the library had no interfaces to read blocks one by
> one. Otherwise, printing individual blocks has no place in the reftable
> library given that the format will not be generic in the first place.
>
> We have now grown a public interface to iterate through blocks contained
> in a table, and thus we can finally move the logic to print them into
> the test helper.
>
> Move over the logic and refactor it accordingly. Note that the iterator
> also trivially allows us to access index sections, which we previously
> didn't print at all. This omission wasn't intentional though, so start
> dumping those sections as well so that we can assert that indices are
> written as expected.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> reftable/reftable-table.h | 3 --
> reftable/table.c | 65 ------------------------------------
> t/helper/test-reftable.c | 69 ++++++++++++++++++++++++++++++++++++++-
> t/t0613-reftable-write-options.sh | 9 +++++
> 4 files changed, 77 insertions(+), 69 deletions(-)
>
> diff --git a/reftable/reftable-table.h b/reftable/reftable-table.h
> index f0f1784c664..293fffbddc6 100644
> --- a/reftable/reftable-table.h
> +++ b/reftable/reftable-table.h
> @@ -97,9 +97,6 @@ uint64_t reftable_table_max_update_index(struct reftable_table *t);
> /* return the min_update_index for a table */
> uint64_t reftable_table_min_update_index(struct reftable_table *t);
>
> -/* print blocks onto stdout for debugging. */
> -int reftable_table_print_blocks(const char *tablename);
> -
> /*
> * An iterator that iterates through the blocks contained in a given table.
> */
> diff --git a/reftable/table.c b/reftable/table.c
> index 48f0cdfd42b..8a7581b9800 100644
> --- a/reftable/table.c
> +++ b/reftable/table.c
> @@ -740,71 +740,6 @@ uint64_t reftable_table_min_update_index(struct reftable_table *t)
> return t->min_update_index;
> }
>
> -int reftable_table_print_blocks(const char *tablename)
> -{
> - struct {
> - const char *name;
> - int type;
> - } sections[] = {
> - {
> - .name = "ref",
> - .type = REFTABLE_BLOCK_TYPE_REF,
> - },
> - {
> - .name = "obj",
> - .type = REFTABLE_BLOCK_TYPE_OBJ,
> - },
> - {
> - .name = "log",
> - .type = REFTABLE_BLOCK_TYPE_LOG,
> - },
> - };
> - struct reftable_block_source src = { 0 };
> - struct reftable_table *table = NULL;
> - struct table_iter ti = { 0 };
> - size_t i;
> - int err;
> -
> - err = reftable_block_source_from_file(&src, tablename);
> - if (err < 0)
> - goto done;
> -
> - err = reftable_table_new(&table, &src, tablename);
> - if (err < 0)
> - goto done;
> -
> - table_iter_init(&ti, table);
> -
> - printf("header:\n");
> - printf(" block_size: %d\n", table->block_size);
> -
> - for (i = 0; i < sizeof(sections) / sizeof(*sections); i++) {
> - err = table_iter_seek_start(&ti, sections[i].type, 0);
> - if (err < 0)
> - goto done;
> - if (err > 0)
> - continue;
> -
> - printf("%s:\n", sections[i].name);
> -
> - while (1) {
> - printf(" - length: %u\n", ti.block.restart_off);
> - printf(" restarts: %u\n", ti.block.restart_count);
> -
> - err = table_iter_next_block(&ti);
> - if (err < 0)
> - goto done;
> - if (err > 0)
> - break;
> - }
> - }
> -
> -done:
> - reftable_table_decref(table);
> - table_iter_close(&ti);
> - return err;
> -}
How that we have a table iterator exposed to iterate over the records we
can remove `reftable_table_print_blocks()` from the reftable library.
Make sense.
> int reftable_table_init_table_iterator(struct reftable_table *t,
> struct reftable_table_iterator *it)
> {
> diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
> index f8f1956f4f3..c465137826c 100644
> --- a/t/helper/test-reftable.c
> +++ b/t/helper/test-reftable.c
> @@ -2,6 +2,7 @@
> #include "hash.h"
> #include "hex.h"
> #include "reftable/system.h"
> +#include "reftable/reftable-constants.h"
> #include "reftable/reftable-error.h"
> #include "reftable/reftable-merged.h"
> #include "reftable/reftable-stack.h"
> @@ -20,6 +21,72 @@ static void print_help(void)
> "\n");
> }
>
> +static int dump_blocks(const char *tablename)
> +{
> + struct reftable_table_iterator ti = { 0 };
> + struct reftable_block_source src = { 0 };
> + struct reftable_table *table = NULL;
> + uint8_t section_type = 0;
> + int err;
> +
> + err = reftable_block_source_from_file(&src, tablename);
> + if (err < 0)
> + goto done;
> +
> + err = reftable_table_new(&table, &src, tablename);
> + if (err < 0)
> + goto done;
> +
> + err = reftable_table_init_table_iterator(table, &ti);
> + if (err < 0)
> + goto done;
> +
> + printf("header:\n");
> + printf(" block_size: %d\n", table->block_size);
> +
> + while (1) {
> + const struct reftable_block *block;
> +
> + err = reftable_table_iterator_next(&ti, &block);
> + if (err < 0)
> + goto done;
> + if (err > 0)
> + break;
> +
> + if (block->block_type != section_type) {
> + const char *section;
> + switch (block->block_type) {
> + case REFTABLE_BLOCK_TYPE_LOG:
> + section = "log";
> + break;
> + case REFTABLE_BLOCK_TYPE_REF:
> + section = "ref";
> + break;
> + case REFTABLE_BLOCK_TYPE_OBJ:
> + section = "obj";
> + break;
> + case REFTABLE_BLOCK_TYPE_INDEX:
> + section = "idx";
> + break;
> + default:
> + err = -1;
> + goto done;
> + }
> +
> + section_type = block->block_type;
> + printf("%s:\n", section);
> + }
> +
> + printf(" - length: %u\n", block->restart_off);
> + printf(" restarts: %u\n", block->restart_count);
> + }
> +
> +done:
> + reftable_table_iterator_release(&ti);
> + reftable_table_decref(table);
> + return err;
> +}
> +
> static int dump_table(struct reftable_merged_table *mt)
> {
> struct reftable_iterator it = { NULL };
> @@ -184,7 +251,7 @@ int cmd__dump_reftable(int argc, const char **argv)
> arg = argv[1];
>
> if (opt_dump_blocks) {
> - err = reftable_table_print_blocks(arg);
> + err = dump_blocks(arg);
Nice!
> } else if (opt_dump_table) {
> err = dump_reftable(arg);
> } else if (opt_dump_stack) {
> diff --git a/t/t0613-reftable-write-options.sh b/t/t0613-reftable-write-options.sh
> index e2708e11d5b..e4c7461ce9e 100755
> --- a/t/t0613-reftable-write-options.sh
> +++ b/t/t0613-reftable-write-options.sh
> @@ -93,6 +93,9 @@ test_expect_success 'many refs results in multiple blocks' '
> restarts: 3
> - length: 3289
> restarts: 3
> + idx:
> + - length: 103
> + restarts: 1
> EOF
> test-tool dump-reftable -b .git/reftable/*.ref >actual &&
> test_cmp expect actual
> @@ -241,6 +244,9 @@ test_expect_success 'object index gets written by default with ref index' '
> restarts: 1
> - length: 80
> restarts: 1
> + idx:
> + - length: 55
> + restarts: 2
> obj:
> - length: 11
> restarts: 1
> @@ -277,6 +283,9 @@ test_expect_success 'object index can be disabled' '
> restarts: 1
> - length: 80
> restarts: 1
> + idx:
> + - length: 55
> + restarts: 2
> EOF
> test-tool dump-reftable -b .git/reftable/*.ref >actual &&
> test_cmp expect actual
>
> --
> 2.49.0.604.gff1f9ca942.dirty
>
>
