{"thread":{"id":"66419","subject":"[PATCH 0/3] refs/reftable: fix on-disk representation of reflog timezones","startedAt":"2026-09-29T09:56:51Z","lastAt":"2026-10-01T17:11:05Z","messageCount":16,"participants":["Patrick Steinhardt","Karthik Nayak","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"553570","messageId":"20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im","threadId":"66419","inReplyTo":null,"subject":"[PATCH 0/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-29T09:56:28Z","receivedAt":"2026-09-29T09:56:51Z","isPatch":true,"body":"Hi,\n\nit was reported [1] that the way we store reflog timezones with the\nreftable format has a mismatch with the reftable specification. While\nthe spec says that reftables should be stored as a signed offset in\nminutes, we store them in the \"[+-]HHMM\" format that we typically use in\ncommit headers, for example.\n\nThis patch series fixes this bug by making our on-disk representation\nmatch the specification. This will of course make us reinterpret old\nreftables. But ultimately, the fallout caused by this change is somewhat\nlimited as we only ever use reflog timezones for display purposes. So\nyes, we'll display a wrong timezone. But it's not used as part of any\nkind of computations.\n\nThe series is built on top of v2.56.0.\n\nThanks!\n\nPatrick\n\n[1]: <85f7daa8-d60b-4348-ac2f-b1a68628af7b@app.fastmail.com>\n\n---\nPatrick Steinhardt (3):\n date: add helpers to convert between \"+HHMM\" timezones and minutes\n t/helper: fix segfault in \"dump-reftable -t\"\n refs/reftable: fix on-disk representation of reflog timezones\n\n apply.c | 3 ++-\n date.c | 25 +++++++++++++++++--------\n date.h | 9 +++++++++\n refs/reftable-backend.c | 7 ++++---\n strbuf.c | 3 +--\n t/helper/test-reftable.c | 13 +++++++++++--\n t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++\n 7 files changed, 79 insertions(+), 16 deletions(-)\n\n\n---\nbase-commit: a018953688f1b10bddf91bff8747068f5f4746a4\nchange-id: 20260929-pks-reftables-fix-timezone-format-93ecd0e7a1d1\n\n"},{"id":"553571","messageId":"20260929-pks-reftables-fix-timezone-format-v1-1-3df105a95ed1@pks.im","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im","subject":"[PATCH 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-29T09:56:29Z","receivedAt":"2026-09-29T09:56:52Z","isPatch":true,"body":"The timezones that we store in commits as part of the identity\ninformation are encoded in \"[+-]HHMM\", for example \"-0700\" for UTC-7.\nInternally we typically pass around this timezone either as string or as\na parsed integer (-700).\n\nSome sites want to convert between this format and minutes or vice\nversa, and that conversion is performed ad-hoc. We're about to introduce\nanother site though that wants to have access to this logic, and having\nit cluttered across our codebase is a bit awkward.\n\nIntroduce two new helpers `tz_to_minutes()` and `minutes_to_tz()` that\nperform the conversion for us and convert call sites to use them.\n\nNote that we used to perform a dance in `gm_time_t()` where we first\nconvert `tz` into a positive value, then calculate the minutes, and\nfinally turn the minutes into a negative value again. This dance is\nperformed because it is implementation-defined in C89 whether the\ndivision on negative values truncates towards zero or not [1]:\n\n If either operand is negative, whether the result of the / operator is\n the largest integer less than the algebraic quotient or the smallest\n integer greater than the algebraic quotient is implementation-defined,\n as is the sign of the result of the % operator.\n\nSo under C89, `-130 / 100` could legitimately result in -1 or -2, and\n`-130 % 100` could result in either -30 or 70. For us though, the result\nthat we want is the first one (-1 and -30), which is called truncation\ntoward zero.\n\nThis part of the C language has changed in C99, where this edge case is\nnow well-defined to always truncate towards zero [2]:\n\n When integers are divided, the result of the / operator is the\n algebraic quotient with any fractional part discarded.90) If the\n quotient a/b is representable, the expression (a/b)*b + a%b shall\n equal a.\n\n 90) This is often called ''truncation toward zero''.\n\nSo in theory it's unlikely that we still need this logic. In practice\nthough it feels safer to just retain it as we don't require a fully\nC99-compliant compiler in Git.\n\n[1]: https://port70.net/~nsz/c/c89/c89-draft.html#3.3.5\n[2]: https://port70.net/~nsz/c/c99/n1256.html#6.5.5p6\n\nSigned-off-by: Patrick Steinhardt \n---\n apply.c | 3 ++-\n date.c | 25 +++++++++++++++++--------\n date.h | 9 +++++++++\n strbuf.c | 3 +--\n 4 files changed, 29 insertions(+), 11 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex f00b7ba4d3..367271b8ac 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -14,6 +14,7 @@\n #include \"abspath.h\"\n #include \"base85.h\"\n #include \"config.h\"\n+#include \"date.h\"\n #include \"odb.h\"\n #include \"delta.h\"\n #include \"diff.h\"\n@@ -851,7 +852,7 @@ static int has_epoch_timestamp(const char *nameline)\n \tif (*colon == ':')\n \t\tzoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);\n \telse\n-\t\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n+\t\tzoneoffset = tz_to_minutes(zoneoffset);\n \tif (timestamp[m[3].rm_so] == '-')\n \t\tzoneoffset = -zoneoffset;\n \ndiff --git a/date.c b/date.c\nindex 014065b419..63ea9dbc76 100644\n--- a/date.c\n+++ b/date.c\n@@ -45,13 +45,23 @@ static const char *weekday_names[] = {\n \t\"Sundays\", \"Mondays\", \"Tuesdays\", \"Wednesdays\", \"Thursdays\", \"Fridays\", \"Saturdays\"\n };\n \n-static time_t gm_time_t(timestamp_t time, int tz)\n+int tz_to_minutes(int tz)\n {\n-\tint minutes;\n+\tint minutes = tz < 0 ? -tz : tz;\n+\tminutes = (minutes / 100) * 60 + (minutes % 100);\n+\treturn tz < 0 ? -minutes : minutes;\n+}\n \n-\tminutes = tz < 0 ? -tz : tz;\n-\tminutes = (minutes / 100)*60 + (minutes % 100);\n-\tminutes = tz < 0 ? -minutes : minutes;\n+int minutes_to_tz(int minutes)\n+{\n+\tint tz = minutes < 0 ? -minutes : minutes;\n+\ttz = (tz / 60) * 100 + (tz % 60);\n+\treturn minutes < 0 ? -tz : tz;\n+}\n+\n+static time_t gm_time_t(timestamp_t time, int tz)\n+{\n+\tint minutes = tz_to_minutes(tz);\n \n \tif (minutes > 0) {\n \t\tif (unsigned_add_overflows(time, minutes * 60))\n@@ -103,8 +113,7 @@ static int local_time_tzoffset(time_t t, struct tm *tm)\n \t\toffset = t_local - t;\n \t}\n \toffset /= 60; /* in minutes */\n-\toffset = (offset % 60) + ((offset / 60) * 100);\n-\treturn offset * eastwest;\n+\treturn minutes_to_tz(offset * eastwest);\n }\n \n /*\n@@ -862,7 +871,7 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in\n \tofs = strtol(date, &end, 10);\n \tif ((*end != '\\0' && (*end != '\\n')) || end != date + 4)\n \t\treturn -1;\n-\tofs = (ofs / 100) * 60 + (ofs % 100);\n+\tofs = tz_to_minutes(ofs);\n \tif (date[-1] == '-')\n \t\tofs = -ofs;\n \t*timestamp = stamp;\ndiff --git a/date.h b/date.h\nindex 0747864fd7..816df5b833 100644\n--- a/date.h\n+++ b/date.h\n@@ -70,4 +70,13 @@ void datestamp(struct strbuf *out);\n timestamp_t approxidate_careful(const char *, int *);\n int date_overflows(timestamp_t date);\n time_t tm_to_time_t(const struct tm *tm);\n+\n+/**\n+ * Convert between the \"[+-]HHMM\" timezone format and minutes. This format is\n+ * used for example as part of commit headers and reflogs. For example, the\n+ * timezone -0100 is converted to -60 minutes.\n+ */\n+int tz_to_minutes(int tz);\n+int minutes_to_tz(int minutes);\n+\n #endif\ndiff --git a/strbuf.c b/strbuf.c\nindex 44955669e8..c3baa47b3f 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1023,8 +1023,7 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n \t\telse if (skip_prefix(fmt, \"s\", &fmt))\n \t\t\tstrbuf_addf(&munged_fmt, \"%\"PRItime,\n \t\t\t\t (timestamp_t)tm_to_time_t(tm) -\n-\t\t\t\t 3600 * (tz_offset / 100) -\n-\t\t\t\t 60 * (tz_offset % 100));\n+\t\t\t\t 60 * tz_to_minutes(tz_offset));\n \t\telse if (skip_prefix(fmt, \"z\", &fmt))\n \t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n \t\telse if (suppress_tz_name && skip_prefix(fmt, \"Z\", &fmt))\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553572","messageId":"20260929-pks-reftables-fix-timezone-format-v1-2-3df105a95ed1@pks.im","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im","subject":"[PATCH 2/3] t/helper: fix segfault in \"dump-reftable -t\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-29T09:56:30Z","receivedAt":"2026-09-29T09:56:53Z","isPatch":true,"body":"The `test-tool dump-reftable` command can be used to dump the on-disk\ncontents of reftables. The \"-t\" subcommand specifically can be used to\ndump a single table from disk.\n\nWhen trying to use this subcommand though one will quickly realize that\nit is broken, as it always segfaults. The root cause of this segfault is\nthat we try to detect the hash algorithm via the merged table's hash ID.\nBut that hash ID is not the same as Git's understanding of a hash ID,\nand consequently we fail to look up the correct algorithm. This will\nthen lead to a segfault later on when we try to dereference a NULL\npointer.\n\nThis breakage went undetected until now because this particular\nsubcommand is not used anywhere in our test suite. So the obvious way to\nfix the bug is by just removing the code outright. But in the next\ncommit we're about to add a user.\n\nFix the issue by properly converting between the two hash IDs.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-reftable.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex fc49fafc34..57758936b0 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -103,7 +103,16 @@ static int dump_table(struct reftable_merged_table *mt)\n \tif (err < 0)\n \t\treturn err;\n \n-\talgop = &hash_algos[hash_algo_by_id(reftable_merged_table_hash_id(mt))];\n+\tswitch (reftable_merged_table_hash_id(mt)) {\n+\tcase REFTABLE_HASH_SHA1:\n+\t\talgop = &hash_algos[GIT_HASH_SHA1];\n+\t\tbreak;\n+\tcase REFTABLE_HASH_SHA256:\n+\t\talgop = &hash_algos[GIT_HASH_SHA256];\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"unknown reftable hash function: %d\", reftable_merged_table_hash_id(mt));\n+\t}\n \n \twhile (1) {\n \t\terr = reftable_iterator_next_ref(&it, &ref);\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553573","messageId":"20260929-pks-reftables-fix-timezone-format-v1-3-3df105a95ed1@pks.im","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im","subject":"[PATCH 3/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-29T09:56:31Z","receivedAt":"2026-09-29T09:56:56Z","isPatch":true,"body":"When writing reflog entries to disk we also record authorship\ninformation for the reflog. Besides the author name and mail address,\nit also contains the date and timezone at which the record has been\ncreated.\n\nThe timezone information is typically encoded in the \"[+-]HHMM\" format,\nand we often pass it around as parsed integer. For example, the timezone\n\"-0700\" would be passed around as -700. And this is also the value that\nwe eventually store in the reftable on disk.\n\nBut the specification in \"Documentation/technical/reftable.adoc\" notes\nthat the timezone is a \"2-byte timezone offset in minutes (signed)\". So\ninstead of storing -700 in the above example, we have to first convert\nthat value into minutes and then store -420. We don't though, so we have\na mismatch between specification and implementation.\n\nIdeally, we'd just adapt the specification to match the implementation.\nBut that's easier said than done, because the specification is 11 years\nold by now and reftables have already been implemented by JGit for a\nlong time. So if we now changed the specification, those libraries would\nhave to make a backwards-incompatible change.\n\nAnother alternative would be to bump the reftable format version, but\nthat feels suboptimal, too. Other libraries would all have to adapt, and\nit wouldn't really help us to fix the discrepancy between alternative\nimplementations and our implementation as older versions would still be\nmisinterpreted.\n\nThe only viable option seems to be that we simply treat this as a bug\nand fix it. This will of course make us misinterpret older reftables\nthat already exist on disk:\n\n ┌───────┬───────────────┬─────────────────┬────────────┐\n │ tz │ HHMM encoding │ correct minutes │ divergence │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +1400 │ 1400 │ 840 │ 560 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ -1200 │ -1200 │ -720 │ 480 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +0530 │ 530 │ 330 │ 200 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +0000 │ 0 │ 0 │ 0 │\n └───────┴───────────────┴─────────────────┴────────────┘\n\nBut this divergence ultimately doesn't matter much, as Git only uses the\ntimezone of reflog entries for display purposes anyway. We don't take\nthe timezone into account when parsing \"HEAD@{1.hour.ago}\" syntax, and\n`should_expire_reflog_ent()` doesn't use it either to decide whether\nreflog entries should be pruned.\n\nIn summary, the fallout from this change is quite contained. Adapt the\nreftable backend accordingly and simply reinterpret the timezones with\nthe specified meaning.\n\nAdd a test to verify that we properly encode the timezone as offset in\nminutes. Adapt the test helper accordingly to no longer zero-pad the\noffset with \"%04d\", as that can be easily misinterpreted as the \"HHMM\"\nencoding.\n\nReported-by: Josh McKinney \nSigned-off-by: Patrick Steinhardt \n---\n refs/reftable-backend.c | 7 ++++---\n t/helper/test-reftable.c | 2 +-\n t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++\n 3 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 10db03991e..d0de066355 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -2,6 +2,7 @@\n #include \"../abspath.h\"\n #include \"../chdir-notify.h\"\n #include \"../config.h\"\n+#include \"../date.h\"\n #include \"../dir.h\"\n #include \"../environment.h\"\n #include \"../fsck.h\"\n@@ -317,7 +318,7 @@ static void fill_reftable_log_record(struct reftable_log_record *log, const stru\n \t\ttz_begin++;\n \t}\n \n-\tlog->value.update.tz_offset = sign * atoi(tz_begin);\n+\tlog->value.update.tz_offset = tz_to_minutes(sign * atoi(tz_begin));\n }\n \n static int reftable_be_config(const char *var, const char *value,\n@@ -2186,7 +2187,7 @@ static int yield_log_record(struct reftable_ref_store *refs,\n \tfull_committer = fmt_ident(log->value.update.name, log->value.update.email,\n \t\t\t\t WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE);\n \treturn fn(log->refname, &old_oid, &new_oid, full_committer,\n-\t\t log->value.update.time, log->value.update.tz_offset,\n+\t\t log->value.update.time, minutes_to_tz(log->value.update.tz_offset),\n \t\t log->value.update.message, cb_data);\n }\n \n@@ -2690,7 +2691,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,\n \n \t\tif (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email,\n \t\t\t\t (timestamp_t)logs[i].value.update.time,\n-\t\t\t\t logs[i].value.update.tz_offset,\n+\t\t\t\t minutes_to_tz(logs[i].value.update.tz_offset),\n \t\t\t\t logs[i].value.update.message,\n \t\t\t\t policy_cb_data)) {\n \t\t\tdest->value_type = REFTABLE_LOG_DELETION;\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 57758936b0..d9f2ca1d0e 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -163,7 +163,7 @@ static int dump_table(struct reftable_merged_table *mt)\n \t\t\t log.update_index);\n \t\t\tbreak;\n \t\tcase REFTABLE_LOG_UPDATE:\n-\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %04d\\n\",\n+\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %d\\n\",\n \t\t\t log.refname, log.update_index,\n \t\t\t log.value.update.name ? log.value.update.name : \"\",\n \t\t\t log.value.update.email ? log.value.update.email : \"\",\ndiff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\nindex 35e98b43db..579657467d 100755\n--- a/t/t0610-reftable-basics.sh\n+++ b/t/t0610-reftable-basics.sh\n@@ -837,6 +837,41 @@ test_expect_success 'reflog: renaming branch writes reflog entry' '\n \t)\n '\n \n+test_expect_success 'reflog: timezone offset is stored in minutes' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 -1200\" git commit --allow-empty -m min &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 +0530\" git commit --allow-empty -m east &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 -0800\" git commit --allow-empty -m west &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 +1400\" git commit --allow-empty -m max &&\n+\n+\t\t# The reftable format specifies the timezone as the offset from\n+\t\t# UTC in minutes, whereas Git uses the parsed form of \"+HHMM\"\n+\t\t# internally. Verify that we do the conversion when writing.\n+\t\tfor table in .git/reftable/*.ref\n+\t\tdo\n+\t\t\ttest-tool dump-reftable -t \"$table\" || return 1\n+\t\tdone >dump &&\n+\t\tsed -n \"s/^log{refs\\/heads\\/main([0-9]*) .* 1234567890 //p\" dump >actual &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\t840\n+\t\t-480\n+\t\t330\n+\t\t-720\n+\t\tEOF\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# And verify that we convert back when reading.\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/main >entries &&\n+\t\ttest_grep \"1234567890 -1200\tcommit (initial): min\" entries &&\n+\t\ttest_grep \"1234567890 +0530\tcommit: east\" entries &&\n+\t\ttest_grep \"1234567890 -0800\tcommit: west\" entries &&\n+\t\ttest_grep \"1234567890 +1400\tcommit: max\" entries\n+\t)\n+'\n+\n test_expect_success 'reflog: can store empty logs' '\n \ttest_when_finished \"rm -rf repo\" &&\n \tgit init repo &&\n\n-- \n2.56.0.rc2.329.gd58861e689.dirty\n\n"},{"id":"553682","messageId":"CAOLa=ZQRDVL2Djh4du1zWGg_ABZTyzaWDYYb0PDg3EXAfpn7bA@mail.gmail.com","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-1-3df105a95ed1@pks.im","subject":"Re: [PATCH 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-30T11:47:02Z","receivedAt":"2026-09-30T11:47:08Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> The timezones that we store in commits as part of the identity\n> information are encoded in \"[+-]HHMM\", for example \"-0700\" for UTC-7.\n> Internally we typically pass around this timezone either as string or as\n> a parsed integer (-700).\n>\n> Some sites want to convert between this format and minutes or vice\n> versa, and that conversion is performed ad-hoc. We're about to introduce\n> another site though that wants to have access to this logic, and having\n> it cluttered across our codebase is a bit awkward.\n>\n> Introduce two new helpers `tz_to_minutes()` and `minutes_to_tz()` that\n> perform the conversion for us and convert call sites to use them.\n>\n> Note that we used to perform a dance in `gm_time_t()` where we first\n> convert `tz` into a positive value, then calculate the minutes, and\n> finally turn the minutes into a negative value again. This dance is\n> performed because it is implementation-defined in C89 whether the\n> division on negative values truncates towards zero or not [1]:\n>\n> If either operand is negative, whether the result of the / operator is\n> the largest integer less than the algebraic quotient or the smallest\n> integer greater than the algebraic quotient is implementation-defined,\n> as is the sign of the result of the % operator.\n>\n> So under C89, `-130 / 100` could legitimately result in -1 or -2, and\n> `-130 % 100` could result in either -30 or 70. For us though, the result\n> that we want is the first one (-1 and -30), which is called truncation\n> toward zero.\n>\n\nWe divide by '100' because we represent \"-0700\" as '-700' in integer. So\nwe need to separate out the 'HH' from 'MM'. Okay.\n\n> This part of the C language has changed in C99, where this edge case is\n> now well-defined to always truncate towards zero [2]:\n>\n> When integers are divided, the result of the / operator is the\n> algebraic quotient with any fractional part discarded.90) If the\n> quotient a/b is representable, the expression (a/b)*b + a%b shall\n> equal a.\n>\n> 90) This is often called ''truncation toward zero''.\n>\n> So in theory it's unlikely that we still need this logic. In practice\n> though it feels safer to just retain it as we don't require a fully\n> C99-compliant compiler in Git.\n>\n> [1]: https://port70.net/~nsz/c/c89/c89-draft.html#3.3.5\n> [2]: https://port70.net/~nsz/c/c99/n1256.html#6.5.5p6\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> apply.c | 3 ++-\n> date.c | 25 +++++++++++++++++--------\n> date.h | 9 +++++++++\n> strbuf.c | 3 +--\n> 4 files changed, 29 insertions(+), 11 deletions(-)\n>\n> diff --git a/apply.c b/apply.c\n> index f00b7ba4d3..367271b8ac 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -14,6 +14,7 @@\n> #include \"abspath.h\"\n> #include \"base85.h\"\n> #include \"config.h\"\n> +#include \"date.h\"\n> #include \"odb.h\"\n> #include \"delta.h\"\n> #include \"diff.h\"\n> @@ -851,7 +852,7 @@ static int has_epoch_timestamp(const char *nameline)\n> \tif (*colon == ':')\n> \t\tzoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);\n> \telse\n> -\t\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n> +\t\tzoneoffset = tz_to_minutes(zoneoffset);\n> \tif (timestamp[m[3].rm_so] == '-')\n> \t\tzoneoffset = -zoneoffset;\n>\n> diff --git a/date.c b/date.c\n> index 014065b419..63ea9dbc76 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -45,13 +45,23 @@ static const char *weekday_names[] = {\n> \t\"Sundays\", \"Mondays\", \"Tuesdays\", \"Wednesdays\", \"Thursdays\", \"Fridays\", \"Saturdays\"\n> };\n>\n> -static time_t gm_time_t(timestamp_t time, int tz)\n> +int tz_to_minutes(int tz)\n> {\n> -\tint minutes;\n> +\tint minutes = tz < 0 ? -tz : tz;\n\nThis is the part which we could skip as we're C99 compliant, but keeping\nto be on the safe side.\n\n> +\tminutes = (minutes / 100) * 60 + (minutes % 100);\n> +\treturn tz < 0 ? -minutes : minutes;\n> +}\n>\n> -\tminutes = tz < 0 ? -tz : tz;\n> -\tminutes = (minutes / 100)*60 + (minutes % 100);\n> -\tminutes = tz < 0 ? -minutes : minutes;\n> +int minutes_to_tz(int minutes)\n> +{\n> +\tint tz = minutes < 0 ? -minutes : minutes;\n> +\ttz = (tz / 60) * 100 + (tz % 60);\n> +\treturn minutes < 0 ? -tz : tz;\n> +}\n> +\n> +static time_t gm_time_t(timestamp_t time, int tz)\n> +{\n> +\tint minutes = tz_to_minutes(tz);\n>\n> \tif (minutes > 0) {\n> \t\tif (unsigned_add_overflows(time, minutes * 60))\n> @@ -103,8 +113,7 @@ static int local_time_tzoffset(time_t t, struct tm *tm)\n> \t\toffset = t_local - t;\n> \t}\n> \toffset /= 60; /* in minutes */\n> -\toffset = (offset % 60) + ((offset / 60) * 100);\n> -\treturn offset * eastwest;\n> +\treturn minutes_to_tz(offset * eastwest);\n\nWhile mathematically it's the same, but shouldn't this have been\n`minutes_to_tz(offset) * eastwest`?\n\n> }\n>\n> /*\n> @@ -862,7 +871,7 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in\n> \tofs = strtol(date, &end, 10);\n> \tif ((*end != '\\0' && (*end != '\\n')) || end != date + 4)\n> \t\treturn -1;\n> -\tofs = (ofs / 100) * 60 + (ofs % 100);\n> +\tofs = tz_to_minutes(ofs);\n> \tif (date[-1] == '-')\n> \t\tofs = -ofs;\n> \t*timestamp = stamp;\n> diff --git a/date.h b/date.h\n> index 0747864fd7..816df5b833 100644\n> --- a/date.h\n> +++ b/date.h\n> @@ -70,4 +70,13 @@ void datestamp(struct strbuf *out);\n> timestamp_t approxidate_careful(const char *, int *);\n> int date_overflows(timestamp_t date);\n> time_t tm_to_time_t(const struct tm *tm);\n> +\n> +/**\n> + * Convert between the \"[+-]HHMM\" timezone format and minutes. This format is\n> + * used for example as part of commit headers and reflogs. For example, the\n> + * timezone -0100 is converted to -60 minutes.\n> + */\n> +int tz_to_minutes(int tz);\n> +int minutes_to_tz(int minutes);\n> +\n> #endif\n> diff --git a/strbuf.c b/strbuf.c\n> index 44955669e8..c3baa47b3f 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -1023,8 +1023,7 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n> \t\telse if (skip_prefix(fmt, \"s\", &fmt))\n> \t\t\tstrbuf_addf(&munged_fmt, \"%\"PRItime,\n> \t\t\t\t (timestamp_t)tm_to_time_t(tm) -\n> -\t\t\t\t 3600 * (tz_offset / 100) -\n> -\t\t\t\t 60 * (tz_offset % 100));\n> +\t\t\t\t 60 * tz_to_minutes(tz_offset));\n> \t\telse if (skip_prefix(fmt, \"z\", &fmt))\n> \t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n> \t\telse if (suppress_tz_name && skip_prefix(fmt, \"Z\", &fmt))\n>\n> --\n> 2.56.0.rc2.329.gd58861e689.dirty\n\nThe rest looks good.\n"},{"id":"553683","messageId":"CAOLa=ZQorPk_Kkewkw5k-gdeh=VRVBMcaDA54S0ByJfAswcCWQ@mail.gmail.com","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-3-3df105a95ed1@pks.im","subject":"Re: [PATCH 3/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-09-30T11:51:15Z","receivedAt":"2026-09-30T11:51:18Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> When writing reflog entries to disk we also record authorship\n> information for the reflog. Besides the author name and mail address,\n> it also contains the date and timezone at which the record has been\n> created.\n>\n> The timezone information is typically encoded in the \"[+-]HHMM\" format,\n> and we often pass it around as parsed integer. For example, the timezone\n> \"-0700\" would be passed around as -700. And this is also the value that\n> we eventually store in the reftable on disk.\n>\n> But the specification in \"Documentation/technical/reftable.adoc\" notes\n> that the timezone is a \"2-byte timezone offset in minutes (signed)\". So\n> instead of storing -700 in the above example, we have to first convert\n> that value into minutes and then store -420. We don't though, so we have\n> a mismatch between specification and implementation.\n>\n> Ideally, we'd just adapt the specification to match the implementation.\n> But that's easier said than done, because the specification is 11 years\n> old by now and reftables have already been implemented by JGit for a\n> long time. So if we now changed the specification, those libraries would\n> have to make a backwards-incompatible change.\n>\n> Another alternative would be to bump the reftable format version, but\n> that feels suboptimal, too. Other libraries would all have to adapt, and\n> it wouldn't really help us to fix the discrepancy between alternative\n> implementations and our implementation as older versions would still be\n> misinterpreted.\n>\n> The only viable option seems to be that we simply treat this as a bug\n> and fix it. This will of course make us misinterpret older reftables\n> that already exist on disk:\n>\n> ┌───────┬───────────────┬─────────────────┬────────────┐\n> │ tz │ HHMM encoding │ correct minutes │ divergence │\n> ├───────┼───────────────┼─────────────────┼────────────┤\n> │ +1400 │ 1400 │ 840 │ 560 │\n> ├───────┼───────────────┼─────────────────┼────────────┤\n> │ -1200 │ -1200 │ -720 │ 480 │\n> ├───────┼───────────────┼─────────────────┼────────────┤\n> │ +0530 │ 530 │ 330 │ 200 │\n> ├───────┼───────────────┼─────────────────┼────────────┤\n> │ +0000 │ 0 │ 0 │ 0 │\n> └───────┴───────────────┴─────────────────┴────────────┘\n>\n> But this divergence ultimately doesn't matter much, as Git only uses the\n> timezone of reflog entries for display purposes anyway. We don't take\n> the timezone into account when parsing \"HEAD@{1.hour.ago}\" syntax, and\n> `should_expire_reflog_ent()` doesn't use it either to decide whether\n> reflog entries should be pruned.\n>\n> In summary, the fallout from this change is quite contained. Adapt the\n> reftable backend accordingly and simply reinterpret the timezones with\n> the specified meaning.\n>\n> Add a test to verify that we properly encode the timezone as offset in\n> minutes. Adapt the test helper accordingly to no longer zero-pad the\n> offset with \"%04d\", as that can be easily misinterpreted as the \"HHMM\"\n> encoding.\n>\n> Reported-by: Josh McKinney \n> Signed-off-by: Patrick Steinhardt \n> ---\n> refs/reftable-backend.c | 7 ++++---\n> t/helper/test-reftable.c | 2 +-\n> t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++\n> 3 files changed, 40 insertions(+), 4 deletions(-)\n>\n> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\n> index 10db03991e..d0de066355 100644\n> --- a/refs/reftable-backend.c\n> +++ b/refs/reftable-backend.c\n> @@ -2,6 +2,7 @@\n> #include \"../abspath.h\"\n> #include \"../chdir-notify.h\"\n> #include \"../config.h\"\n> +#include \"../date.h\"\n> #include \"../dir.h\"\n> #include \"../environment.h\"\n> #include \"../fsck.h\"\n> @@ -317,7 +318,7 @@ static void fill_reftable_log_record(struct reftable_log_record *log, const stru\n> \t\ttz_begin++;\n> \t}\n>\n> -\tlog->value.update.tz_offset = sign * atoi(tz_begin);\n> +\tlog->value.update.tz_offset = tz_to_minutes(sign * atoi(tz_begin));\n> }\n>\n> static int reftable_be_config(const char *var, const char *value,\n> @@ -2186,7 +2187,7 @@ static int yield_log_record(struct reftable_ref_store *refs,\n> \tfull_committer = fmt_ident(log->value.update.name, log->value.update.email,\n> \t\t\t\t WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE);\n> \treturn fn(log->refname, &old_oid, &new_oid, full_committer,\n> -\t\t log->value.update.time, log->value.update.tz_offset,\n> +\t\t log->value.update.time, minutes_to_tz(log->value.update.tz_offset),\n> \t\t log->value.update.message, cb_data);\n> }\n>\n> @@ -2690,7 +2691,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,\n>\n> \t\tif (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email,\n> \t\t\t\t (timestamp_t)logs[i].value.update.time,\n> -\t\t\t\t logs[i].value.update.tz_offset,\n> +\t\t\t\t minutes_to_tz(logs[i].value.update.tz_offset),\n> \t\t\t\t logs[i].value.update.message,\n> \t\t\t\t policy_cb_data)) {\n> \t\t\tdest->value_type = REFTABLE_LOG_DELETION;\n> diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\n> index 57758936b0..d9f2ca1d0e 100644\n> --- a/t/helper/test-reftable.c\n> +++ b/t/helper/test-reftable.c\n> @@ -163,7 +163,7 @@ static int dump_table(struct reftable_merged_table *mt)\n> \t\t\t log.update_index);\n> \t\t\tbreak;\n> \t\tcase REFTABLE_LOG_UPDATE:\n> -\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %04d\\n\",\n> +\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %d\\n\",\n> \t\t\t log.refname, log.update_index,\n> \t\t\t log.value.update.name ? log.value.update.name : \"\",\n> \t\t\t log.value.update.email ? log.value.update.email : \"\",\n> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\n> index 35e98b43db..579657467d 100755\n> --- a/t/t0610-reftable-basics.sh\n> +++ b/t/t0610-reftable-basics.sh\n> @@ -837,6 +837,41 @@ test_expect_success 'reflog: renaming branch writes reflog entry' '\n> \t)\n> '\n>\n> +test_expect_success 'reflog: timezone offset is stored in minutes' '\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\t\tGIT_COMMITTER_DATE=\"1234567890 -1200\" git commit --allow-empty -m min &&\n> +\t\tGIT_COMMITTER_DATE=\"1234567890 +0530\" git commit --allow-empty -m east &&\n> +\t\tGIT_COMMITTER_DATE=\"1234567890 -0800\" git commit --allow-empty -m west &&\n> +\t\tGIT_COMMITTER_DATE=\"1234567890 +1400\" git commit --allow-empty -m max &&\n\nNit: it would be nice to have a negative timezone with MM filled in too.\n\n> +\t\t# The reftable format specifies the timezone as the offset from\n> +\t\t# UTC in minutes, whereas Git uses the parsed form of \"+HHMM\"\n> +\t\t# internally. Verify that we do the conversion when writing.\n> +\t\tfor table in .git/reftable/*.ref\n> +\t\tdo\n> +\t\t\ttest-tool dump-reftable -t \"$table\" || return 1\n> +\t\tdone >dump &&\n> +\t\tsed -n \"s/^log{refs\\/heads\\/main([0-9]*) .* 1234567890 //p\" dump >actual &&\n> +\t\tcat >expect <<-\\EOF &&\n> +\t\t840\n> +\t\t-480\n> +\t\t330\n> +\t\t-720\n> +\t\tEOF\n> +\t\ttest_cmp expect actual &&\n> +\n> +\t\t# And verify that we convert back when reading.\n> +\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/main >entries &&\n> +\t\ttest_grep \"1234567890 -1200\tcommit (initial): min\" entries &&\n> +\t\ttest_grep \"1234567890 +0530\tcommit: east\" entries &&\n> +\t\ttest_grep \"1234567890 -0800\tcommit: west\" entries &&\n> +\t\ttest_grep \"1234567890 +1400\tcommit: max\" entries\n> +\t)\n> +'\n> +\n> test_expect_success 'reflog: can store empty logs' '\n> \ttest_when_finished \"rm -rf repo\" &&\n> \tgit init repo &&\n>\n> --\n> 2.56.0.rc2.329.gd58861e689.dirty\n\nApart from the nit, the changes look good.\n"},{"id":"553686","messageId":"arz7CNEhQSoDwJ77@pks.im","threadId":"66419","inReplyTo":"CAOLa=ZQRDVL2Djh4du1zWGg_ABZTyzaWDYYb0PDg3EXAfpn7bA@mail.gmail.com","subject":"Re: [PATCH 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-30T12:05:28Z","receivedAt":"2026-09-30T12:05:35Z","isPatch":true,"body":"On Wed, Sep 30, 2026 at 04:47:02AM -0700, Karthik Nayak wrote:\n\nOne suggestion: I'd recommend trimming the mails you're responding to a\nbit more aggressively. Otherwise one is hunting for responses in files\nand hunks that are not relevant to your remarks :)\n\n> Patrick Steinhardt writes:\n> > diff --git a/date.c b/date.c\n> > index 014065b419..63ea9dbc76 100644\n> > --- a/date.c\n> > +++ b/date.c\n> > @@ -103,8 +113,7 @@ static int local_time_tzoffset(time_t t, struct tm *tm)\n> > \t\toffset = t_local - t;\n> > \t}\n> > \toffset /= 60; /* in minutes */\n> > -\toffset = (offset % 60) + ((offset / 60) * 100);\n> > -\treturn offset * eastwest;\n> > +\treturn minutes_to_tz(offset * eastwest);\n> \n> While mathematically it's the same, but shouldn't this have been\n> `minutes_to_tz(offset) * eastwest`?\n\nI guess we can. It's probably less confusing if we do it this way\nindeed.\n\nPatrick\n"},{"id":"553687","messageId":"arz7DV44Au08oLad@pks.im","threadId":"66419","inReplyTo":"CAOLa=ZQorPk_Kkewkw5k-gdeh=VRVBMcaDA54S0ByJfAswcCWQ@mail.gmail.com","subject":"Re: [PATCH 3/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-30T12:05:33Z","receivedAt":"2026-09-30T12:05:45Z","isPatch":true,"body":"On Wed, Sep 30, 2026 at 04:51:15AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> > diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\n> > index 35e98b43db..579657467d 100755\n> > --- a/t/t0610-reftable-basics.sh\n> > +++ b/t/t0610-reftable-basics.sh\n> > @@ -837,6 +837,41 @@ test_expect_success 'reflog: renaming branch writes reflog entry' '\n> > \t)\n> > '\n> >\n> > +test_expect_success 'reflog: timezone offset is stored in minutes' '\n> > +\ttest_when_finished \"rm -rf repo\" &&\n> > +\tgit init repo &&\n> > +\t(\n> > +\t\tcd repo &&\n> > +\t\tGIT_COMMITTER_DATE=\"1234567890 -1200\" git commit --allow-empty -m min &&\n> > +\t\tGIT_COMMITTER_DATE=\"1234567890 +0530\" git commit --allow-empty -m east &&\n> > +\t\tGIT_COMMITTER_DATE=\"1234567890 -0800\" git commit --allow-empty -m west &&\n> > +\t\tGIT_COMMITTER_DATE=\"1234567890 +1400\" git commit --allow-empty -m max &&\n> \n> Nit: it would be nice to have a negative timezone with MM filled in too.\n\nSure, can do. I'll just change -0800 to -0830. Thanks!\n\nPatrick\n"},{"id":"553748","messageId":"xmqqa4oya51p.fsf@gitster.g","threadId":"66419","inReplyTo":"CAOLa=ZQRDVL2Djh4du1zWGg_ABZTyzaWDYYb0PDg3EXAfpn7bA@mail.gmail.com","subject":"Re: [PATCH 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-30T19:54:26Z","receivedAt":"2026-09-30T19:54:29Z","isPatch":true,"body":"Karthik Nayak writes:\n\n>> -\tint minutes;\n>> +\tint minutes = tz < 0 ? -tz : tz;\n>\n> This is the part which we could skip as we're C99 compliant, but keeping\n> to be on the safe side.\n>\n>> +\tminutes = (minutes / 100) * 60 + (minutes % 100);\n>> +\treturn tz < 0 ? -minutes : minutes;\n>> +}\n\nI was wondering exactly the same thing yesterday.\n\nAs written, it is clear even to those unfamiliar with the C89/C99\nsigned division rules, because we deal only with non-negative\nnumbers, which is a plus. The fewer things readers need to worry\nabout, the better.\n\n\n>> \toffset /= 60; /* in minutes */\n>> -\toffset = (offset % 60) + ((offset / 60) * 100);\n>> -\treturn offset * eastwest;\n>> +\treturn minutes_to_tz(offset * eastwest);\n>\n> While mathematically it's the same, but shouldn't this have been\n> `minutes_to_tz(offset) * eastwest`?\n\nThe way you suggest is more faithful rewrite of the original.\n\nThanks.\n"},{"id":"553806","messageId":"20261001-pks-reftables-fix-timezone-format-v2-0-a4fd1f7cd21a@pks.im","threadId":"66419","inReplyTo":"20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im","subject":"[PATCH v2 0/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-01T05:38:59Z","receivedAt":"2026-10-01T05:39:13Z","isPatch":true,"body":"Hi,\n\nit was reported [1] that the way we store reflog timezones with the\nreftable format has a mismatch with the reftable specification. While\nthe spec says that reftables should be stored as a signed offset in\nminutes, we store them in the \"[+-]HHMM\" format that we typically use in\ncommit headers, for example.\n\nThis patch series fixes this bug by making our on-disk representation\nmatch the specification. This will of course make us reinterpret old\nreftables. But ultimately, the fallout caused by this change is somewhat\nlimited as we only ever use reflog timezones for display purposes. So\nyes, we'll display a wrong timezone. But it's not used as part of any\nkind of computations.\n\nThe series is built on top of v2.56.0.\n\nChanges in v2:\n - Improve readability of one of the converted sites that now use\n `minutes_to_tz()`.\n - Improve test coverage.\n - Link to v1: https://patch.msgid.link/20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im\n\nThanks!\n\nPatrick\n\n[1]: <85f7daa8-d60b-4348-ac2f-b1a68628af7b@app.fastmail.com>\n\n---\nPatrick Steinhardt (3):\n date: add helpers to convert between \"+HHMM\" timezones and minutes\n t/helper: fix segfault in \"dump-reftable -t\"\n refs/reftable: fix on-disk representation of reflog timezones\n\n apply.c | 3 ++-\n date.c | 25 +++++++++++++++++--------\n date.h | 9 +++++++++\n refs/reftable-backend.c | 7 ++++---\n strbuf.c | 3 +--\n t/helper/test-reftable.c | 13 +++++++++++--\n t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++\n 7 files changed, 79 insertions(+), 16 deletions(-)\n\nRange-diff versus v1:\n\n1: 91b9739506 ! 1: 4f950cf7de date: add helpers to convert between \"+HHMM\" timezones and minutes\n @@ date.c: static int local_time_tzoffset(time_t t, struct tm *tm)\n \toffset /= 60; /* in minutes */\n -\toffset = (offset % 60) + ((offset / 60) * 100);\n -\treturn offset * eastwest;\n -+\treturn minutes_to_tz(offset * eastwest);\n ++\treturn minutes_to_tz(offset) * eastwest;\n }\n \n /*\n2: 112e414c34 = 2: 9ef28821a3 t/helper: fix segfault in \"dump-reftable -t\"\n3: 26eee10d6a ! 3: c66e467554 refs/reftable: fix on-disk representation of reflog timezones\n @@ t/t0610-reftable-basics.sh: test_expect_success 'reflog: renaming branch writes\n +\t\tcd repo &&\n +\t\tGIT_COMMITTER_DATE=\"1234567890 -1200\" git commit --allow-empty -m min &&\n +\t\tGIT_COMMITTER_DATE=\"1234567890 +0530\" git commit --allow-empty -m east &&\n -+\t\tGIT_COMMITTER_DATE=\"1234567890 -0800\" git commit --allow-empty -m west &&\n ++\t\tGIT_COMMITTER_DATE=\"1234567890 -0830\" git commit --allow-empty -m west &&\n +\t\tGIT_COMMITTER_DATE=\"1234567890 +1400\" git commit --allow-empty -m max &&\n +\n +\t\t# The reftable format specifies the timezone as the offset from\n @@ t/t0610-reftable-basics.sh: test_expect_success 'reflog: renaming branch writes\n +\t\tsed -n \"s/^log{refs\\/heads\\/main([0-9]*) .* 1234567890 //p\" dump >actual &&\n +\t\tcat >expect <<-\\EOF &&\n +\t\t840\n -+\t\t-480\n ++\t\t-510\n +\t\t330\n +\t\t-720\n +\t\tEOF\n @@ t/t0610-reftable-basics.sh: test_expect_success 'reflog: renaming branch writes\n +\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/main >entries &&\n +\t\ttest_grep \"1234567890 -1200\tcommit (initial): min\" entries &&\n +\t\ttest_grep \"1234567890 +0530\tcommit: east\" entries &&\n -+\t\ttest_grep \"1234567890 -0800\tcommit: west\" entries &&\n ++\t\ttest_grep \"1234567890 -0830\tcommit: west\" entries &&\n +\t\ttest_grep \"1234567890 +1400\tcommit: max\" entries\n +\t)\n +'\n\n---\nbase-commit: a018953688f1b10bddf91bff8747068f5f4746a4\nchange-id: 20260929-pks-reftables-fix-timezone-format-93ecd0e7a1d1\n\n"},{"id":"553808","messageId":"20261001-pks-reftables-fix-timezone-format-v2-1-a4fd1f7cd21a@pks.im","threadId":"66419","inReplyTo":"20261001-pks-reftables-fix-timezone-format-v2-0-a4fd1f7cd21a@pks.im","subject":"[PATCH v2 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-01T05:39:00Z","receivedAt":"2026-10-01T05:39:15Z","isPatch":true,"body":"The timezones that we store in commits as part of the identity\ninformation are encoded in \"[+-]HHMM\", for example \"-0700\" for UTC-7.\nInternally we typically pass around this timezone either as string or as\na parsed integer (-700).\n\nSome sites want to convert between this format and minutes or vice\nversa, and that conversion is performed ad-hoc. We're about to introduce\nanother site though that wants to have access to this logic, and having\nit cluttered across our codebase is a bit awkward.\n\nIntroduce two new helpers `tz_to_minutes()` and `minutes_to_tz()` that\nperform the conversion for us and convert call sites to use them.\n\nNote that we used to perform a dance in `gm_time_t()` where we first\nconvert `tz` into a positive value, then calculate the minutes, and\nfinally turn the minutes into a negative value again. This dance is\nperformed because it is implementation-defined in C89 whether the\ndivision on negative values truncates towards zero or not [1]:\n\n If either operand is negative, whether the result of the / operator is\n the largest integer less than the algebraic quotient or the smallest\n integer greater than the algebraic quotient is implementation-defined,\n as is the sign of the result of the % operator.\n\nSo under C89, `-130 / 100` could legitimately result in -1 or -2, and\n`-130 % 100` could result in either -30 or 70. For us though, the result\nthat we want is the first one (-1 and -30), which is called truncation\ntoward zero.\n\nThis part of the C language has changed in C99, where this edge case is\nnow well-defined to always truncate towards zero [2]:\n\n When integers are divided, the result of the / operator is the\n algebraic quotient with any fractional part discarded.90) If the\n quotient a/b is representable, the expression (a/b)*b + a%b shall\n equal a.\n\n 90) This is often called ''truncation toward zero''.\n\nSo in theory it's unlikely that we still need this logic. In practice\nthough it feels safer to just retain it as we don't require a fully\nC99-compliant compiler in Git.\n\n[1]: https://port70.net/~nsz/c/c89/c89-draft.html#3.3.5\n[2]: https://port70.net/~nsz/c/c99/n1256.html#6.5.5p6\n\nSigned-off-by: Patrick Steinhardt \n---\n apply.c | 3 ++-\n date.c | 25 +++++++++++++++++--------\n date.h | 9 +++++++++\n strbuf.c | 3 +--\n 4 files changed, 29 insertions(+), 11 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex f00b7ba4d3..367271b8ac 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -14,6 +14,7 @@\n #include \"abspath.h\"\n #include \"base85.h\"\n #include \"config.h\"\n+#include \"date.h\"\n #include \"odb.h\"\n #include \"delta.h\"\n #include \"diff.h\"\n@@ -851,7 +852,7 @@ static int has_epoch_timestamp(const char *nameline)\n \tif (*colon == ':')\n \t\tzoneoffset = zoneoffset * 60 + strtol(colon + 1, NULL, 10);\n \telse\n-\t\tzoneoffset = (zoneoffset / 100) * 60 + (zoneoffset % 100);\n+\t\tzoneoffset = tz_to_minutes(zoneoffset);\n \tif (timestamp[m[3].rm_so] == '-')\n \t\tzoneoffset = -zoneoffset;\n \ndiff --git a/date.c b/date.c\nindex 014065b419..c50f45d310 100644\n--- a/date.c\n+++ b/date.c\n@@ -45,13 +45,23 @@ static const char *weekday_names[] = {\n \t\"Sundays\", \"Mondays\", \"Tuesdays\", \"Wednesdays\", \"Thursdays\", \"Fridays\", \"Saturdays\"\n };\n \n-static time_t gm_time_t(timestamp_t time, int tz)\n+int tz_to_minutes(int tz)\n {\n-\tint minutes;\n+\tint minutes = tz < 0 ? -tz : tz;\n+\tminutes = (minutes / 100) * 60 + (minutes % 100);\n+\treturn tz < 0 ? -minutes : minutes;\n+}\n \n-\tminutes = tz < 0 ? -tz : tz;\n-\tminutes = (minutes / 100)*60 + (minutes % 100);\n-\tminutes = tz < 0 ? -minutes : minutes;\n+int minutes_to_tz(int minutes)\n+{\n+\tint tz = minutes < 0 ? -minutes : minutes;\n+\ttz = (tz / 60) * 100 + (tz % 60);\n+\treturn minutes < 0 ? -tz : tz;\n+}\n+\n+static time_t gm_time_t(timestamp_t time, int tz)\n+{\n+\tint minutes = tz_to_minutes(tz);\n \n \tif (minutes > 0) {\n \t\tif (unsigned_add_overflows(time, minutes * 60))\n@@ -103,8 +113,7 @@ static int local_time_tzoffset(time_t t, struct tm *tm)\n \t\toffset = t_local - t;\n \t}\n \toffset /= 60; /* in minutes */\n-\toffset = (offset % 60) + ((offset / 60) * 100);\n-\treturn offset * eastwest;\n+\treturn minutes_to_tz(offset) * eastwest;\n }\n \n /*\n@@ -862,7 +871,7 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in\n \tofs = strtol(date, &end, 10);\n \tif ((*end != '\\0' && (*end != '\\n')) || end != date + 4)\n \t\treturn -1;\n-\tofs = (ofs / 100) * 60 + (ofs % 100);\n+\tofs = tz_to_minutes(ofs);\n \tif (date[-1] == '-')\n \t\tofs = -ofs;\n \t*timestamp = stamp;\ndiff --git a/date.h b/date.h\nindex 0747864fd7..816df5b833 100644\n--- a/date.h\n+++ b/date.h\n@@ -70,4 +70,13 @@ void datestamp(struct strbuf *out);\n timestamp_t approxidate_careful(const char *, int *);\n int date_overflows(timestamp_t date);\n time_t tm_to_time_t(const struct tm *tm);\n+\n+/**\n+ * Convert between the \"[+-]HHMM\" timezone format and minutes. This format is\n+ * used for example as part of commit headers and reflogs. For example, the\n+ * timezone -0100 is converted to -60 minutes.\n+ */\n+int tz_to_minutes(int tz);\n+int minutes_to_tz(int minutes);\n+\n #endif\ndiff --git a/strbuf.c b/strbuf.c\nindex 44955669e8..c3baa47b3f 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1023,8 +1023,7 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n \t\telse if (skip_prefix(fmt, \"s\", &fmt))\n \t\t\tstrbuf_addf(&munged_fmt, \"%\"PRItime,\n \t\t\t\t (timestamp_t)tm_to_time_t(tm) -\n-\t\t\t\t 3600 * (tz_offset / 100) -\n-\t\t\t\t 60 * (tz_offset % 100));\n+\t\t\t\t 60 * tz_to_minutes(tz_offset));\n \t\telse if (skip_prefix(fmt, \"z\", &fmt))\n \t\t\tstrbuf_addf(&munged_fmt, \"%+05d\", tz_offset);\n \t\telse if (suppress_tz_name && skip_prefix(fmt, \"Z\", &fmt))\n\n-- \n2.56.0.353.g0856645cf6.dirty\n\n"},{"id":"553807","messageId":"20261001-pks-reftables-fix-timezone-format-v2-2-a4fd1f7cd21a@pks.im","threadId":"66419","inReplyTo":"20261001-pks-reftables-fix-timezone-format-v2-0-a4fd1f7cd21a@pks.im","subject":"[PATCH v2 2/3] t/helper: fix segfault in \"dump-reftable -t\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-01T05:39:01Z","receivedAt":"2026-10-01T05:39:17Z","isPatch":true,"body":"The `test-tool dump-reftable` command can be used to dump the on-disk\ncontents of reftables. The \"-t\" subcommand specifically can be used to\ndump a single table from disk.\n\nWhen trying to use this subcommand though one will quickly realize that\nit is broken, as it always segfaults. The root cause of this segfault is\nthat we try to detect the hash algorithm via the merged table's hash ID.\nBut that hash ID is not the same as Git's understanding of a hash ID,\nand consequently we fail to look up the correct algorithm. This will\nthen lead to a segfault later on when we try to dereference a NULL\npointer.\n\nThis breakage went undetected until now because this particular\nsubcommand is not used anywhere in our test suite. So the obvious way to\nfix the bug is by just removing the code outright. But in the next\ncommit we're about to add a user.\n\nFix the issue by properly converting between the two hash IDs.\n\nSigned-off-by: Patrick Steinhardt \n---\n t/helper/test-reftable.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex fc49fafc34..57758936b0 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -103,7 +103,16 @@ static int dump_table(struct reftable_merged_table *mt)\n \tif (err < 0)\n \t\treturn err;\n \n-\talgop = &hash_algos[hash_algo_by_id(reftable_merged_table_hash_id(mt))];\n+\tswitch (reftable_merged_table_hash_id(mt)) {\n+\tcase REFTABLE_HASH_SHA1:\n+\t\talgop = &hash_algos[GIT_HASH_SHA1];\n+\t\tbreak;\n+\tcase REFTABLE_HASH_SHA256:\n+\t\talgop = &hash_algos[GIT_HASH_SHA256];\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"unknown reftable hash function: %d\", reftable_merged_table_hash_id(mt));\n+\t}\n \n \twhile (1) {\n \t\terr = reftable_iterator_next_ref(&it, &ref);\n\n-- \n2.56.0.353.g0856645cf6.dirty\n\n"},{"id":"553809","messageId":"20261001-pks-reftables-fix-timezone-format-v2-3-a4fd1f7cd21a@pks.im","threadId":"66419","inReplyTo":"20261001-pks-reftables-fix-timezone-format-v2-0-a4fd1f7cd21a@pks.im","subject":"[PATCH v2 3/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-10-01T05:39:02Z","receivedAt":"2026-10-01T05:39:20Z","isPatch":true,"body":"When writing reflog entries to disk we also record authorship\ninformation for the reflog. Besides the author name and mail address,\nit also contains the date and timezone at which the record has been\ncreated.\n\nThe timezone information is typically encoded in the \"[+-]HHMM\" format,\nand we often pass it around as parsed integer. For example, the timezone\n\"-0700\" would be passed around as -700. And this is also the value that\nwe eventually store in the reftable on disk.\n\nBut the specification in \"Documentation/technical/reftable.adoc\" notes\nthat the timezone is a \"2-byte timezone offset in minutes (signed)\". So\ninstead of storing -700 in the above example, we have to first convert\nthat value into minutes and then store -420. We don't though, so we have\na mismatch between specification and implementation.\n\nIdeally, we'd just adapt the specification to match the implementation.\nBut that's easier said than done, because the specification is 11 years\nold by now and reftables have already been implemented by JGit for a\nlong time. So if we now changed the specification, those libraries would\nhave to make a backwards-incompatible change.\n\nAnother alternative would be to bump the reftable format version, but\nthat feels suboptimal, too. Other libraries would all have to adapt, and\nit wouldn't really help us to fix the discrepancy between alternative\nimplementations and our implementation as older versions would still be\nmisinterpreted.\n\nThe only viable option seems to be that we simply treat this as a bug\nand fix it. This will of course make us misinterpret older reftables\nthat already exist on disk:\n\n ┌───────┬───────────────┬─────────────────┬────────────┐\n │ tz │ HHMM encoding │ correct minutes │ divergence │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +1400 │ 1400 │ 840 │ 560 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ -1200 │ -1200 │ -720 │ 480 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +0530 │ 530 │ 330 │ 200 │\n ├───────┼───────────────┼─────────────────┼────────────┤\n │ +0000 │ 0 │ 0 │ 0 │\n └───────┴───────────────┴─────────────────┴────────────┘\n\nBut this divergence ultimately doesn't matter much, as Git only uses the\ntimezone of reflog entries for display purposes anyway. We don't take\nthe timezone into account when parsing \"HEAD@{1.hour.ago}\" syntax, and\n`should_expire_reflog_ent()` doesn't use it either to decide whether\nreflog entries should be pruned.\n\nIn summary, the fallout from this change is quite contained. Adapt the\nreftable backend accordingly and simply reinterpret the timezones with\nthe specified meaning.\n\nAdd a test to verify that we properly encode the timezone as offset in\nminutes. Adapt the test helper accordingly to no longer zero-pad the\noffset with \"%04d\", as that can be easily misinterpreted as the \"HHMM\"\nencoding.\n\nReported-by: Josh McKinney \nSigned-off-by: Patrick Steinhardt \n---\n refs/reftable-backend.c | 7 ++++---\n t/helper/test-reftable.c | 2 +-\n t/t0610-reftable-basics.sh | 35 +++++++++++++++++++++++++++++++++++\n 3 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 10db03991e..d0de066355 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -2,6 +2,7 @@\n #include \"../abspath.h\"\n #include \"../chdir-notify.h\"\n #include \"../config.h\"\n+#include \"../date.h\"\n #include \"../dir.h\"\n #include \"../environment.h\"\n #include \"../fsck.h\"\n@@ -317,7 +318,7 @@ static void fill_reftable_log_record(struct reftable_log_record *log, const stru\n \t\ttz_begin++;\n \t}\n \n-\tlog->value.update.tz_offset = sign * atoi(tz_begin);\n+\tlog->value.update.tz_offset = tz_to_minutes(sign * atoi(tz_begin));\n }\n \n static int reftable_be_config(const char *var, const char *value,\n@@ -2186,7 +2187,7 @@ static int yield_log_record(struct reftable_ref_store *refs,\n \tfull_committer = fmt_ident(log->value.update.name, log->value.update.email,\n \t\t\t\t WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE);\n \treturn fn(log->refname, &old_oid, &new_oid, full_committer,\n-\t\t log->value.update.time, log->value.update.tz_offset,\n+\t\t log->value.update.time, minutes_to_tz(log->value.update.tz_offset),\n \t\t log->value.update.message, cb_data);\n }\n \n@@ -2690,7 +2691,7 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,\n \n \t\tif (should_prune_fn(&old_oid, &new_oid, logs[i].value.update.email,\n \t\t\t\t (timestamp_t)logs[i].value.update.time,\n-\t\t\t\t logs[i].value.update.tz_offset,\n+\t\t\t\t minutes_to_tz(logs[i].value.update.tz_offset),\n \t\t\t\t logs[i].value.update.message,\n \t\t\t\t policy_cb_data)) {\n \t\t\tdest->value_type = REFTABLE_LOG_DELETION;\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 57758936b0..d9f2ca1d0e 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -163,7 +163,7 @@ static int dump_table(struct reftable_merged_table *mt)\n \t\t\t log.update_index);\n \t\t\tbreak;\n \t\tcase REFTABLE_LOG_UPDATE:\n-\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %04d\\n\",\n+\t\t\tprintf(\"log{%s(%\" PRIu64 \") %s <%s> %\" PRIu64 \" %d\\n\",\n \t\t\t log.refname, log.update_index,\n \t\t\t log.value.update.name ? log.value.update.name : \"\",\n \t\t\t log.value.update.email ? log.value.update.email : \"\",\ndiff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\nindex 35e98b43db..2253705a19 100755\n--- a/t/t0610-reftable-basics.sh\n+++ b/t/t0610-reftable-basics.sh\n@@ -837,6 +837,41 @@ test_expect_success 'reflog: renaming branch writes reflog entry' '\n \t)\n '\n \n+test_expect_success 'reflog: timezone offset is stored in minutes' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 -1200\" git commit --allow-empty -m min &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 +0530\" git commit --allow-empty -m east &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 -0830\" git commit --allow-empty -m west &&\n+\t\tGIT_COMMITTER_DATE=\"1234567890 +1400\" git commit --allow-empty -m max &&\n+\n+\t\t# The reftable format specifies the timezone as the offset from\n+\t\t# UTC in minutes, whereas Git uses the parsed form of \"+HHMM\"\n+\t\t# internally. Verify that we do the conversion when writing.\n+\t\tfor table in .git/reftable/*.ref\n+\t\tdo\n+\t\t\ttest-tool dump-reftable -t \"$table\" || return 1\n+\t\tdone >dump &&\n+\t\tsed -n \"s/^log{refs\\/heads\\/main([0-9]*) .* 1234567890 //p\" dump >actual &&\n+\t\tcat >expect <<-\\EOF &&\n+\t\t840\n+\t\t-510\n+\t\t330\n+\t\t-720\n+\t\tEOF\n+\t\ttest_cmp expect actual &&\n+\n+\t\t# And verify that we convert back when reading.\n+\t\ttest-tool ref-store main for-each-reflog-ent refs/heads/main >entries &&\n+\t\ttest_grep \"1234567890 -1200\tcommit (initial): min\" entries &&\n+\t\ttest_grep \"1234567890 +0530\tcommit: east\" entries &&\n+\t\ttest_grep \"1234567890 -0830\tcommit: west\" entries &&\n+\t\ttest_grep \"1234567890 +1400\tcommit: max\" entries\n+\t)\n+'\n+\n test_expect_success 'reflog: can store empty logs' '\n \ttest_when_finished \"rm -rf repo\" &&\n \tgit init repo &&\n\n-- \n2.56.0.353.g0856645cf6.dirty\n\n"},{"id":"553827","messageId":"CAOLa=ZQHhf2obR2OjvgmE5HqRpkWvn9WjE+-LeTx1WYgbQp0Dw@mail.gmail.com","threadId":"66419","inReplyTo":"arz7CNEhQSoDwJ77@pks.im","subject":"Re: [PATCH 1/3] date: add helpers to convert between \"+HHMM\" timezones and minutes","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-10-01T10:18:34Z","receivedAt":"2026-10-01T10:18:36Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> On Wed, Sep 30, 2026 at 04:47:02AM -0700, Karthik Nayak wrote:\n>\n> One suggestion: I'd recommend trimming the mails you're responding to a\n> bit more aggressively. Otherwise one is hunting for responses in files\n> and hunks that are not relevant to your remarks :)\n>\n\nSure, will do that more henceforth.\n\n>> Patrick Steinhardt writes:\n>> > diff --git a/date.c b/date.c\n>> > index 014065b419..63ea9dbc76 100644\n>> > --- a/date.c\n>> > +++ b/date.c\n>> > @@ -103,8 +113,7 @@ static int local_time_tzoffset(time_t t, struct tm *tm)\n>> > \t\toffset = t_local - t;\n>> > \t}\n>> > \toffset /= 60; /* in minutes */\n>> > -\toffset = (offset % 60) + ((offset / 60) * 100);\n>> > -\treturn offset * eastwest;\n>> > +\treturn minutes_to_tz(offset * eastwest);\n>>\n>> While mathematically it's the same, but shouldn't this have been\n>> `minutes_to_tz(offset) * eastwest`?\n>\n> I guess we can. It's probably less confusing if we do it this way\n> indeed.\n>\n> Patrick\n\nYup. Thanks\n"},{"id":"553829","messageId":"CAOLa=ZRVt=e3MqjLY=UkitfSg_YjpsfFjeDsEmqJQm-5YhopxA@mail.gmail.com","threadId":"66419","inReplyTo":"20261001-pks-reftables-fix-timezone-format-v2-0-a4fd1f7cd21a@pks.im","subject":"Re: [PATCH v2 0/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-10-01T10:19:29Z","receivedAt":"2026-10-01T10:19:31Z","isPatch":true,"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> it was reported [1] that the way we store reflog timezones with the\n> reftable format has a mismatch with the reftable specification. While\n> the spec says that reftables should be stored as a signed offset in\n> minutes, we store them in the \"[+-]HHMM\" format that we typically use in\n> commit headers, for example.\n>\n> This patch series fixes this bug by making our on-disk representation\n> match the specification. This will of course make us reinterpret old\n> reftables. But ultimately, the fallout caused by this change is somewhat\n> limited as we only ever use reflog timezones for display purposes. So\n> yes, we'll display a wrong timezone. But it's not used as part of any\n> kind of computations.\n>\n> The series is built on top of v2.56.0.\n>\n> Changes in v2:\n> - Improve readability of one of the converted sites that now use\n> `minutes_to_tz()`.\n> - Improve test coverage.\n> - Link to v1: https://patch.msgid.link/20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im\n>\n\nThe range-diff looks in order. This version looks good to me!\n\n[snip]\n"},{"id":"553850","messageId":"xmqq33up73dm.fsf@gitster.g","threadId":"66419","inReplyTo":"CAOLa=ZRVt=e3MqjLY=UkitfSg_YjpsfFjeDsEmqJQm-5YhopxA@mail.gmail.com","subject":"Re: [PATCH v2 0/3] refs/reftable: fix on-disk representation of reflog timezones","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-01T17:11:01Z","receivedAt":"2026-10-01T17:11:05Z","isPatch":true,"body":"Karthik Nayak writes:\n\n> Patrick Steinhardt writes:\n>\n>> Hi,\n>>\n>> it was reported [1] that the way we store reflog timezones with the\n>> reftable format has a mismatch with the reftable specification. While\n>> the spec says that reftables should be stored as a signed offset in\n>> minutes, we store them in the \"[+-]HHMM\" format that we typically use in\n>> commit headers, for example.\n>>\n>> This patch series fixes this bug by making our on-disk representation\n>> match the specification. This will of course make us reinterpret old\n>> reftables. But ultimately, the fallout caused by this change is somewhat\n>> limited as we only ever use reflog timezones for display purposes. So\n>> yes, we'll display a wrong timezone. But it's not used as part of any\n>> kind of computations.\n>>\n>> The series is built on top of v2.56.0.\n>>\n>> Changes in v2:\n>> - Improve readability of one of the converted sites that now use\n>> `minutes_to_tz()`.\n>> - Improve test coverage.\n>> - Link to v1: https://patch.msgid.link/20260929-pks-reftables-fix-timezone-format-v1-0-3df105a95ed1@pks.im\n>>\n>\n> The range-diff looks in order. This version looks good to me!\n\nThanks for writing and reviewing. The previous round was good\nenough already but with an extra polish, this looks really ready.\n\nWill mark the topic for 'next'.\n"}]}