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/Z4UbkcmJAU1MT-Rs@xxxxxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> --- 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/20250117-461-corrupted-reftable-followup-v1-0-70ee605ae3fe@xxxxxxxxx --- 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
