Re: Reftable reflog timezone encoding differs from specification

Patrick Steinhardt [off-list ref] 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.