git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/5] refs/reftable: add fsck checks

From
Karthik Nayak <karthik.188@gmail.com>
Date
Sep 2, 2025, 07:05 UTC
Message-ID
<20250902-228-reftable-introduce-consistency-checks-v2-0-4f96b3834779@gmail.com>
In-Reply-To
<20250819-228-reftable-introduce-consistency-checks-v1-0-8b8f6879fa9e@gmail.com>

This series adds the required infrastructure and also some fsck checks for the reftable backend.

Since the reftable backend is treated as a library within the Git codebase, we don't want to spillover our internal fsck implementation into the library. At the same time, the fsck checks need to access internal structures of the reftable library which aren't exposed outside the library.

So we solve this by adding a 'reftable/fsck.[ch]' which implements and exposes a checker for the reftable library and returns specific errors as defined by the library. We then add glue code within 'refs/reftable-backend.c' to map these errors to errors which Git's fsck implementation would understand. This allows us to separate concerns.

This series then adds some checks on the stack ('reftable/tables.list')
level of reftable, namely:
1. The table name is as per the spec
2. The number of tables are consistent
3. The tables.list has a newline at the end of file
4. The table names follow correct index sequences

I also plan to send in follow up series's which will implement further checks and go into deeper layers (tables, block, references).

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v2:
- Ensured that 'struct reftable_fsck_info' is passed around as a
  pointer, this provides a smaller footprint (pointer size vs struct
  size).
- Run FSCK checks for other worktrees too, even if one of them fails.
- Separate messaging for table name vs table check and add additional
  test.
- Use the relative path in messages used.
- Small style and typo fixes.
- Link to v1: https://lore.kernel.org/r/20250819-228-reftable-introduce-consistency-checks-v1-0-8b8f6879fa9e@gmail.com
---
 Documentation/fsck-msgids.adoc |  15 +++-
 Makefile                       |   1 +
 fsck.h                         | 154 ++++++++++++++++++++-------------------
 meson.build                    |   1 +
 refs/reftable-backend.c        |  66 +++++++++++++++--
 reftable/fsck.c                | 134 ++++++++++++++++++++++++++++++++++
 reftable/reftable-fsck.h       |  44 +++++++++++
 t/meson.build                  |   3 +-
 t/t0614-reftable-fsck.sh       | 161 +++++++++++++++++++++++++++++++++++++++++
 9 files changed, 494 insertions(+), 85 deletions(-)
Karthik Nayak (5):
      fsck: order 'fsck_msg_type' alphabetically
      refs/reftable: add fsck check for checking the table name
      refs/reftable: add fsck check for number of tables
      refs/reftable: add fsck check for trailing newline
      refs/reftable: add fsck check for incorrect update index
Range-diff versus v1:
1:  d1875fbbc7 = 1:  c049cd428a fsck: order 'fsck_msg_type' alphabetically
2:  b63799aad1 ! 2:  1e46786745 refs/reftable: add fsck check for checking the table name
    @@ Commit message
         Also add 'badReftableTableName' as a corresponding error within Git. Add
         a test to check for this behavior.
     
    +    While here, remove a unused header `#include "../lockfile.h"` from
    +    'refs/reftable-backend.c'.
    +
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## Documentation/fsck-msgids.adoc ##
    @@ refs/reftable-backend.c: static int reftable_be_reflog_expire(struct ref_store *
     +		fprintf_ln(stderr, "%s", _(msg));
     +}
     +
    -+static int reftable_fsck_error_handler(struct reftable_fsck_info info,
    ++static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
     +				       void *cb_data)
     +{
    ++	struct fsck_ref_report report = { .path = info->path };
     +	struct fsck_options *o = cb_data;
    -+	struct fsck_ref_report report = { .path = info.path };
     +	enum fsck_msg_id msg_id;
     +
    -+	switch (info.error) {
    ++	switch (info->error) {
     +	case REFTABLE_FSCK_ERROR_TABLE_NAME:
     +		msg_id = FSCK_MSG_BAD_REFTABLE_TABLE_NAME;
     +		break;
     +	default:
    -+		BUG("unknown fsck error: %d", info.error);
    ++		BUG("unknown fsck error: %d", info->error);
     +	}
     +
    -+	return fsck_report_ref(o, &report, msg_id, "%s", info.msg);
    ++	return fsck_report_ref(o, &report, msg_id, "%s", info->msg);
     +}
     +
     +static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,
    @@ refs/reftable-backend.c: static int reftable_be_reflog_expire(struct ref_store *
     +	if (o->verbose)
     +		fprintf_ln(stderr, _("Checking references consistency"));
     +
    -+	ret = reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,
    ++	ret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,
     +				  reftable_fsck_verbose_handler, o);
    -+	if (!ret)
    -+		return ret;
     +
     +	strmap_for_each_entry(&refs->worktree_backends, &iter, entry) {
     +		struct reftable_backend *b = (struct reftable_backend *)entry->value;
    -+		ret = reftable_fsck_check(b->stack, reftable_fsck_error_handler,
    ++		ret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler,
     +					  reftable_fsck_verbose_handler, o);
    -+		if (!ret)
    -+			return ret;
     +	}
     +
     +	return ret;
    @@ reftable/fsck.c (new)
     +			reftable_fsck_verbose_fn verbose_fn,
     +			void *cb_data)
     +{
    ++
     +	char **names = NULL;
     +	uint64_t min, max;
     +	int err = 0;
    @@ reftable/fsck.c (new)
     +		struct reftable_fsck_info info = {
     +			.error = REFTABLE_FSCK_ERROR_TABLE_NAME,
     +			.path = names[i],
    -+			.msg = "invalid reftable name"
     +		};
     +		uint32_t rnd;
     +		/*
    @@ reftable/fsck.c (new)
     +
     +		if (sscanf(names[i], "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x%5s",
     +			   &min, &max, &rnd, tail) != 4) {
    -+			err = report_fn(info, cb_data);
    ++			info.msg = "invalid reftable table name";
    ++			err = report_fn(&info, cb_data);
    ++			continue;
     +		}
     +
     +		if (strcmp(tail, ".ref")) {
    -+			err = report_fn(info, cb_data);
    ++			info.msg = "invalid reftable table extension";
    ++			err = report_fn(&info, cb_data);
     +		}
     +	}
     +
    @@ reftable/reftable-fsck.h (new)
     +	REFTABLE_FSCK_ERROR_TABLE_NAME = -1,
     +};
     +
    -+/* Represents an individual error encounctered during the FSCK checks. */
    ++/* Represents an individual error encountered during the FSCK checks. */
     +struct reftable_fsck_info {
     +	enum reftable_fsck_error error;
     +	const char *msg;
     +	const char *path;
     +};
     +
    -+typedef int reftable_fsck_report_fn(struct reftable_fsck_info info,
    ++typedef int reftable_fsck_report_fn(struct reftable_fsck_info *info,
     +				    void *cb_data);
     +typedef void reftable_fsck_verbose_fn(const char *msg, void *cb_data);
     +
    @@ reftable/reftable-fsck.h (new)
     + *
     + * If an issue is encountered, the issue is reported to the callee via the
     + * provided 'report_fn'. If the issue is non-recoverable the flow will not
    -+ * conitnue. If it is recoverable, the flow will continue and further issues
    ++ * continue. If it is recoverable, the flow will continue and further issues
     + * will be reported as identified.
     + *
     + * The 'verbose_fn' will be invoked to provide verbose information about
    @@ t/t0614-reftable-fsck.sh (new)
     +		test_must_be_empty err &&
     +
     +		TABLE_NAME=$(cat .git/reftable/tables.list | head -n1) &&
    ++		sed "1s/^/extra/" .git/reftable/tables.list >.git/reftable/tables.list.tmp &&
    ++		mv .git/reftable/tables.list.tmp .git/reftable/tables.list &&
    ++		mv .git/reftable/${TABLE_NAME} .git/reftable/extra${TABLE_NAME} &&
    ++
    ++		test_must_fail git refs verify 2>err &&
    ++		cat >expect <<-EOF &&
    ++		error: extra${TABLE_NAME}: badReftableTableName: invalid reftable table name
    ++		EOF
    ++		test_cmp expect err
    ++	)
    ++'
    ++
    ++test_expect_success 'table name should be checked' '
    ++	test_when_finished "rm -rf repo" &&
    ++	git init repo &&
    ++	(
    ++		cd repo &&
    ++		git commit --allow-empty -m initial &&
    ++
    ++		git refs verify 2>err &&
    ++		test_must_be_empty err &&
    ++
    ++		TABLE_NAME=$(cat .git/reftable/tables.list | head -n1) &&
     +		sed "1s/$/extra/" .git/reftable/tables.list >.git/reftable/tables.list.tmp &&
     +		mv .git/reftable/tables.list.tmp .git/reftable/tables.list &&
     +		mv .git/reftable/${TABLE_NAME} .git/reftable/${TABLE_NAME}extra &&
     +
     +		test_must_fail git refs verify 2>err &&
     +		cat >expect <<-EOF &&
    -+		error: ${TABLE_NAME}extra: badReftableTableName: invalid reftable name
    ++		error: ${TABLE_NAME}extra: badReftableTableName: invalid reftable table extension
     +		EOF
     +		test_cmp expect err
     +	)
3:  4c6c99ded3 ! 3:  52fc14fdeb refs/reftable: add fsck check for number of tables
    @@ fsck.h: enum fsck_msg_type {
      	FUNC(BAD_REF_FILETYPE, ERROR)                              \
     
      ## refs/reftable-backend.c ##
    -@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info info,
    +@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
      	case REFTABLE_FSCK_ERROR_TABLE_NAME:
      		msg_id = FSCK_MSG_BAD_REFTABLE_TABLE_NAME;
      		break;
    @@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_
     +		msg_id = FSCK_MSG_BAD_REFTABLE_STACK_COUNT;
     +		break;
      	default:
    - 		BUG("unknown fsck error: %d", info.error);
    + 		BUG("unknown fsck error: %d", info->error);
      	}
     
      ## reftable/fsck.c ##
    @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
     +	if (!reftable_fsck_valid_stack_count(stack)) {
     +		struct reftable_fsck_info info = {
     +			.error = REFTABLE_FSCK_ERROR_STACK_COUNT,
    -+			.path = stack->list_file,
    ++			.path = "reftable/tables.list",
     +			.msg = "mismatch in number of tables"
     +		};
     +
    -+		err = report_fn(info, cb_data);
    ++		err = report_fn(&info, cb_data);
     +	}
     +
      out:
    @@ reftable/reftable-fsck.h
     +	REFTABLE_FSCK_ERROR_STACK_COUNT = -2,
      };
      
    - /* Represents an individual error encounctered during the FSCK checks. */
    + /* Represents an individual error encountered during the FSCK checks. */
     
      ## t/t0614-reftable-fsck.sh ##
     @@ t/t0614-reftable-fsck.sh: test_expect_success 'table name should be checked' '
    @@ t/t0614-reftable-fsck.sh: test_expect_success 'table name should be checked' '
     +
     +		test_must_fail git refs verify 2>err &&
     +		cat >expect <<-EOF &&
    -+		error: $(pwd)/.git/reftable/tables.list: badReftableStackCount: mismatch in number of tables
    ++		error: reftable/tables.list: badReftableStackCount: mismatch in number of tables
     +		EOF
     +		test_cmp expect err
     +	)
4:  7e8a14c77e ! 4:  4099878ceb refs/reftable: add fsck check for trailing newline
    @@ fsck.h: enum fsck_msg_type {
      	FUNC(BAD_REF_FILETYPE, ERROR)                              \
     
      ## refs/reftable-backend.c ##
    -@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info info,
    +@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
      	case REFTABLE_FSCK_ERROR_STACK_COUNT:
      		msg_id = FSCK_MSG_BAD_REFTABLE_STACK_COUNT;
      		break;
    @@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_
     +		msg_id = FSCK_MSG_BAD_REFTABLE_STACK_LIST_NEWLINE;
     +		break;
      	default:
    - 		BUG("unknown fsck error: %d", info.error);
    + 		BUG("unknown fsck error: %d", info->error);
      	}
     
      ## reftable/fsck.c ##
    @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
     +	if (!reftable_fsck_stack_contains_newline(stack->list_file)) {
     +		struct reftable_fsck_info info = {
     +			.error = REFTABLE_FSCK_ERROR_STACK_LIST_MISSING_NEWLINE,
    -+			.path = stack->list_file,
    ++			.path = "reftable/tables.list",
     +			.msg = "trailing newline missing in stack list"
     +		};
     +
    -+		err = report_fn(info, cb_data);
    ++		err = report_fn(&info, cb_data);
     +	}
     +
      	verbose_fn("Checking reftable tables count", cb_data);
    @@ reftable/reftable-fsck.h: enum reftable_fsck_error {
     +	REFTABLE_FSCK_ERROR_STACK_LIST_MISSING_NEWLINE = -3,
      };
      
    - /* Represents an individual error encounctered during the FSCK checks. */
    + /* Represents an individual error encountered during the FSCK checks. */
     
      ## t/t0614-reftable-fsck.sh ##
     @@ t/t0614-reftable-fsck.sh: test_expect_success 'table count should be checked' '
    @@ t/t0614-reftable-fsck.sh: test_expect_success 'table count should be checked' '
     +
     +		test_must_fail git refs verify 2>err &&
     +		cat >expect <<-EOF &&
    -+		error: $(pwd)/.git/reftable/tables.list: badReftableStackListNewline: trailing newline missing in stack list
    ++		error: reftable/tables.list: badReftableStackListNewline: trailing newline missing in stack list
     +		EOF
     +		test_cmp expect err
     +	)
5:  56ee4348d5 ! 5:  e33345088b refs/reftable: add fsck check for incorrect update index
    @@ fsck.h: enum fsck_msg_type {
      	FUNC(BAD_REF_NAME, ERROR)                                  \
     
      ## refs/reftable-backend.c ##
    -@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info info,
    +@@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
      	case REFTABLE_FSCK_ERROR_STACK_LIST_MISSING_NEWLINE:
      		msg_id = FSCK_MSG_BAD_REFTABLE_STACK_LIST_NEWLINE;
      		break;
    @@ refs/reftable-backend.c: static int reftable_fsck_error_handler(struct reftable_
     +		msg_id = FSCK_MSG_BAD_REFTABLE_UPDATE_INDEX;
     +		break;
      	default:
    - 		BUG("unknown fsck error: %d", info.error);
    + 		BUG("unknown fsck error: %d", info->error);
      	}
     
      ## reftable/fsck.c ##
    @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
      			reftable_fsck_verbose_fn verbose_fn,
      			void *cb_data)
      {
    +-
     +	uint64_t min, max, prev_max = 0;
      	char **names = NULL;
     -	uint64_t min, max;
    @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
      
      	if (stack == NULL)
     @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
    - 			err = report_fn(info, cb_data);
    + 			continue;
      		}
      
     +		if (min != (prev_max + 1) || max < min) {
    @@ reftable/fsck.c: int reftable_fsck_check(struct reftable_stack *stack,
     +				.msg = "incorrect update index in table name"
     +			};
     +
    -+			err = report_fn(info, cb_data);
    ++			err = report_fn(&info, cb_data);
     +		}
     +
      		if (strcmp(tail, ".ref")) {
    - 			err = report_fn(info, cb_data);
    + 			info.msg = "invalid reftable table extension";
    + 			err = report_fn(&info, cb_data);
      		}
     +
     +		prev_max = max;
    @@ reftable/reftable-fsck.h: enum reftable_fsck_error {
     +	REFTABLE_FSCK_ERROR_UPDATE_INDEX = -4,
      };
      
    - /* Represents an individual error encounctered during the FSCK checks. */
    + /* Represents an individual error encountered during the FSCK checks. */
     
      ## t/t0614-reftable-fsck.sh ##
     @@ t/t0614-reftable-fsck.sh: test_expect_success 'stack list must contain trailing newline' '

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0 change-id: 20250714-228-reftable-introduce-consistency-checks-379ded93c544

Thanks
- Karthik
Next: Karthik Nayak
Message 1 of 18 in “refs/reftable: add fsck checks”
  1. 0/5 refs/reftable: add fsck checksKarthik Nayak, Sep 2, 2025
  2. 1/5 fsck: order 'fsck_msg_type' alphabeticallyKarthik Nayak, Sep 2, 2025
  3. Junio C HamanoSep 2, 2025
  4. Karthik NayakSep 8, 2025
  5. 2/5 refs/reftable: add fsck check for checking the table nameKarthik Nayak, Sep 2, 2025
  6. Patrick SteinhardtSep 3, 2025
  7. shejialuoSep 3, 2025
  8. Karthik NayakSep 9, 2025
  9. Karthik NayakSep 9, 2025
  10. 3/5 refs/reftable: add fsck check for number of tablesKarthik Nayak, Sep 2, 2025
  11. Patrick SteinhardtSep 3, 2025
  12. Karthik NayakSep 15, 2025
  13. 4/5 refs/reftable: add fsck check for trailing newlineKarthik Nayak, Sep 2, 2025
  14. Junio C HamanoSep 2, 2025
  15. Patrick SteinhardtSep 3, 2025
  16. 5/5 refs/reftable: add fsck check for incorrect update indexKarthik Nayak, Sep 2, 2025
  17. Junio C HamanoSep 2, 2025
  18. Karthik NayakSep 18, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.