[v2,0/3] refs: small followups to the migration corruption fix

Message

Karthik Nayak Jan. 21, 2025, 3:34 a.m. UTC

This is a follow up to the bug that was reported [1] around `git refs
migrate --ref-format=reftable` where the migration would fail for
repositories with reflogs with lots of entries. This was caused due to a
mismatch in the reftable's header and footer, specifically WRT the
'max_update_index'.

While there was a fix posted. This series is a small followup to fix
some of the topics discussed there:
1. To mark `ref_transaction_update_reflog()` as static since it is only
used internally within 'refs.c'.
2. To change the type of 'max_index' from 'unsigned int' to 'uint64_t'.
This would be much safer for large repositories with millions of files
and on 32bit systems.
3. To add a safeguard to prevent 'update_index' changes post any record
addition. This is a preventive measure to ensure such bugs don't arise
in the future.

This is based on top of master 757161efcc (Sync with Git 2.48.1,
2025-01-13) with 'kn/reflog-migration-fix' merged in.

[1]: https://lore.kernel.org/r/[e-mail]

Signed-off-by: Karthik Nayak <[e-mail]>
---
Changes in v2:
- Commit 1: Keep the function comments.
- Commit 3: Modify the requirement to be for any record writes instead
of the first block write. This ensures that the limits need to be set 
before any records being added. Instead of using `BUG()`, return 
`REFTABLE_API_ERROR`. Handle all callers as needed.
- Link to v1: https://lore.kernel.org/r/[e-mail]

---
Karthik Nayak (3):
      refs: mark `ref_transaction_update_reflog()` as static
      refs: use 'uint64_t' for 'ref_update.index'
      reftable: prevent 'update_index' changes after adding records

 refs.c                          | 24 ++++++++++++++++--------
 refs.h                          | 14 --------------
 refs/refs-internal.h            |  4 ++--
 refs/reftable-backend.c         | 22 ++++++++++++++++------
 reftable/reftable-error.h       |  1 +
 reftable/reftable-writer.h      | 24 ++++++++++++++----------
 reftable/stack.c                |  6 ++++--
 reftable/writer.c               | 13 +++++++++++--
 t/unit-tests/t-reftable-stack.c |  8 +++++---
 9 files changed, 69 insertions(+), 47 deletions(-)
---

Range-diff versus v1:

1:  82a4f950dd ! 1:  5821ddfbe2 refs: mark `ref_transaction_update_reflog()` as static
    @@ refs.c: int ref_transaction_update(struct ref_transaction *transaction,
     -				  const char *committer_info, unsigned int flags,
     -				  const char *msg, unsigned int index,
     -				  struct strbuf *err)
    ++/*
    ++ * Similar to`ref_transaction_update`, but this function is only for adding
    ++ * a reflog update. Supports providing custom committer information. The index
    ++ * field can be utiltized to order updates as desired. When not used, the
    ++ * updates default to being ordered by refname.
    ++ */
     +static int ref_transaction_update_reflog(struct ref_transaction *transaction,
     +					 const char *refname,
     +					 const struct object_id *new_oid,
2:  51ed04e95d = 2:  8025cfadf2 refs: use 'uint64_t' for 'ref_update.index'
3:  34e3001e29 < -:  ---------- reftable: prevent 'update_index' changes after header write
-:  ---------- > 3:  4b0416ce20 reftable: prevent 'update_index' changes after adding records


---

base-commit: a5aa44e7930761cb900813d971b4105f901818fb
change-id: 20250117-461-corrupted-reftable-followup-eb0e4fd1a723

Thanks
- Karthik