[PATCH v2 3/3] refs/reftable: fix on-disk representation of reflog timezones
From: Patrick Steinhardt <hidden>
Date: 2026-10-01 05:39:20
Subsystem:
the rest · Maintainer:
Linus Torvalds
When writing reflog entries to disk we also record authorship
information for the reflog. Besides the author name and mail address,
it also contains the date and timezone at which the record has been
created.
The timezone information is typically encoded in the "[+-]HHMM" format,
and we often pass it around as parsed integer. For example, the timezone
"-0700" would be passed around as -700. And this is also the value that
we eventually store in the reftable on disk.
But the specification in "Documentation/technical/reftable.adoc" notes
that the timezone is a "2-byte timezone offset in minutes (signed)". So
instead of storing -700 in the above example, we have to first convert
that value into minutes and then store -420. We don't though, so we have
a mismatch between specification and implementation.
Ideally, we'd just adapt the specification to match the implementation.
But that's easier said than done, because the specification is 11 years
old by now and reftables have already been implemented by JGit for a
long time. So if we now changed the specification, those libraries would
have to make a backwards-incompatible change.
Another alternative would be to bump the reftable format version, but
that feels suboptimal, too. Other libraries would all have to adapt, and
it wouldn't really help us to fix the discrepancy between alternative
implementations and our implementation as older versions would still be
misinterpreted.
The only viable option seems to be that we simply treat this as a bug
and fix it. This will of course make us misinterpret older reftables
that already exist on disk:
┌───────┬───────────────┬─────────────────┬────────────┐
│ tz │ HHMM encoding │ correct minutes │ divergence │
├───────┼───────────────┼─────────────────┼────────────┤
│ +1400 │ 1400 │ 840 │ 560 │
├───────┼───────────────┼─────────────────┼────────────┤
│ -1200 │ -1200 │ -720 │ 480 │
├───────┼───────────────┼─────────────────┼────────────┤
│ +0530 │ 530 │ 330 │ 200 │
├───────┼───────────────┼─────────────────┼────────────┤
│ +0000 │ 0 │ 0 │ 0 │
└───────┴───────────────┴─────────────────┴────────────┘
But this divergence ultimately doesn't matter much, as Git only uses the
timezone of reflog entries for display purposes anyway. We don't take
the timezone into account when parsing "HEAD@{1.hour.ago}" syntax, and
`should_expire_reflog_ent()` doesn't use it either to decide whether
reflog entries should be pruned.
In summary, the fallout from this change is quite contained. Adapt the
reftable backend accordingly and simply reinterpret the timezones with
the specified meaning.
Add a test to verify that we properly encode the timezone as offset in
minutes. Adapt the test helper accordingly to no longer zero-pad the
offset with "%04d", as that can be easily misinterpreted as the "HHMM"
encoding.
Reported-by: Josh McKinney <redacted>
Signed-off-by: Patrick Steinhardt <redacted>
---
refs/reftable-backend.c | 7 ++++---
t/helper/test-reftable.c | 2 +-
t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 10db03991e..d0de066355 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c@@ -2,6 +2,7 @@ #include "../abspath.h" #include "../chdir-notify.h" #include "../config.h" +#include "../date.h" #include "../dir.h" #include "../environment.h" #include "../fsck.h"
@@ -317,7 +318,7 @@ static void fill_reftable_log_record(struct reftable_log_record *log, const stru tz_begin++; } - log->value.update.tz_offset = sign * atoi(tz_begin); + log->value.update.tz_offset = tz_to_minutes(sign * atoi(tz_begin)); } static int reftable_be_config(const char *var, const char *value,
@@ -2186,7 +2187,7 @@ static int yield_log_record(struct reftable_ref_store *refs, full_committer = fmt_ident(log->value.update.name, log->value.update.email, WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE); return fn(log->refname, &old_oid, &new_oid, full_committer, - log->value.update.time, log->value.update.tz_offset, + log->value.update.time, minutes_to_tz(log->value.update.tz_offset), log->value.update.message, cb_data); }
@@ -2690,7 +2691,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store, if (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email, (timestamp_t)logs[i].value.update.time, - logs[i].value.update.tz_offset, + minutes_to_tz(logs[i].value.update.tz_offset), logs[i].value.update.message, policy_cb_data)) { dest->value_type = REFTABLE_LOG_DELETION;
diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index 57758936b0..d9f2ca1d0e 100644
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c@@ -163,7 +163,7 @@ static int dump_table(struct reftable_merged_table *mt) log.update_index); break; case REFTABLE_LOG_UPDATE: - printf("log{%s(%" PRIu64 ") %s <%s> %" PRIu64 " %04d\n", + printf("log{%s(%" PRIu64 ") %s <%s> %" PRIu64 " %d\n", log.refname, log.update_index, log.value.update.name ? log.value.update.name : "", log.value.update.email ? log.value.update.email : "",
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 35e98b43db..2253705a19 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh@@ -837,6 +837,41 @@ test_expect_success 'reflog: renaming branch writes reflog entry' ' ) ' +test_expect_success 'reflog: timezone offset is stored in minutes' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + GIT_COMMITTER_DATE="1234567890 -1200" git commit --allow-empty -m min && + GIT_COMMITTER_DATE="1234567890 +0530" git commit --allow-empty -m east && + GIT_COMMITTER_DATE="1234567890 -0830" git commit --allow-empty -m west && + GIT_COMMITTER_DATE="1234567890 +1400" git commit --allow-empty -m max && + + # The reftable format specifies the timezone as the offset from + # UTC in minutes, whereas Git uses the parsed form of "+HHMM" + # internally. Verify that we do the conversion when writing. + for table in .git/reftable/*.ref + do + test-tool dump-reftable -t "$table" || return 1 + done >dump && + sed -n "s/^log{refs\/heads\/main([0-9]*) .* 1234567890 //p" dump >actual && + cat >expect <<-\EOF && + 840 + -510 + 330 + -720 + EOF + test_cmp expect actual && + + # And verify that we convert back when reading. + test-tool ref-store main for-each-reflog-ent refs/heads/main >entries && + test_grep "1234567890 -1200 commit (initial): min" entries && + test_grep "1234567890 +0530 commit: east" entries && + test_grep "1234567890 -0830 commit: west" entries && + test_grep "1234567890 +1400 commit: max" entries + ) +' + test_expect_success 'reflog: can store empty logs' ' test_when_finished "rm -rf repo" && git init repo &&
--
2.56.0.353.g0856645cf6.dirty