Re: Reftable reflog timezone encoding differs from specification

["Josh McKinney" <git-bugs@lists.joshka.net>]

I think my main concern here is mostly around using multiple tools on the same repo and how they should interpret on disk formats (my clanker picked up the problem when comparing git's output with a library it's writing).

Anyway, nothing urgent on the problem from me because I noticed it purely in a development context.
Thanks for filling in the bits about the real world impact on this too.

Josh

-- 
Josh McKinney
joshka.net

On Mon, Sep 28, 2026, at 7:42 AM, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
>> We should use the one that we have in our specification, so in my
>> opinion we should fix Git itself. This is also because JGit, which had a
>> reftable implementation for far longer compared to us, implements the
>> specification correctly:
>>
>>
>> 	private PersonIdent readPersonIdent() {
>> 		String name = readValueString();
>> 		String email = readValueString();
>> 		long epochSeconds = readVarint64();
>> 		ZoneOffset tz = ZoneOffset.ofTotalSeconds(readInt16() * 60);
>> 		return new PersonIdent(name, email, Instant.ofEpochSecond(epochSeconds), tz);
>> 	}
>
> Thanks for checking.  I (unfortunately) agree with the (unfortunate)
> conclusion.
>
> We do not ship reftable files over networks and reflogs at the
> conceptual level is not shared across repositories, so the issue,
> other than the trivial part of updating the implementation, is how
> to migrate the data in a local repository that uses reftable.  One
> time offline conversion may be the simplest but I do not know if it
> is worth it, given ...
>
>> We could of course retroactively declare that version 2 of the format
>> uses the syntax that Git uses right now. After all, JGit only knows to
>> read version 1 of it anyway, so that could kind of fix it. But for any
>> repository that uses SHA1 we used to write version 1 anyway, so this
>> does not really buy us anything, I'd claim.
>>
>> In summary:
>>
>>   - We have an upper limit in divergence of <10h.
>
> ... this.
>
>>
>>   - This only matters in the context of reflogs, we don't use these
>>     anywhere else.
>>
>>   - The risk for data loss by a change is limited as our default grace
>>     period for garbage collecting reflog entries is 30 days.
>>
>> With these points I'm inclined to call it a bug and just fix it, without
>> handling backwards compatibility.