WIP: gix-date: handling bad offsets

[elmarco]

This commit from edk2 has an invalid author date offset: tianocore/edk2@630cb85

$ gix cat 630cb8507b2f1d7d7af3ac0f992d40f209dc1cee ...
author Ruiyu Ni <[email protected]> 1516956202 -3407

But git show can still display it:
commit 630cb8507b2f1d7d7af3ac0f992d40f209dc1cee
Author: Ray Ni <[email protected]>
Date:   Wed Jan 24 22:36:22 2018 -3407

How to handle it with gix-date?

[Byron]

Thanks for contributing a test!

I see now that to_time() can panic, and I think that's to be fixed right away by making it fallible.
Is this something you want to contribute?

Besides that, do you have something in particular that is failing in gitoxide thanks to that that Git handles better?
Particularly, round-tripping should be supported by gix as well as it stores the original bytes for the author and time (

gitoxide/gix-actor/src/lib.rs

Lines 90 to 94 in aff23d6

	/// The timestamp at which the signature was performed,
	/// potentially malformed due to lenient parsing.
	///
	/// Use [`SignatureRef::time()`] to decode.
	pub time: &'a str,

) so it won't fail when decoding invalid times or authors.

[elmarco]

Hi @Byron and cool project! I would be happy to contribute to if I had more time ;) I am a bit unsure how to handle this issue. Decoding &str to Time is not the issue, it's the formatting back to human string that is problematic, and I understand it's not really gix business at this point. At the same time, handling badly formatted time representations may also be not jiff responsability.

Yet, imho gitoxide should provide a way to display the same result a git show in this case. Do you think we should work on solving this issue in jiff by having some support for bad representations? Or do you disagree with the above?

[Byron]

I think I understand now what you mean - it's to be able to decode the date at least, and leave the offset alone. Git can decode just the date string, and displays the original offset.
My feeling is that it just has no limits in terms of offset, so for it there is no problem.

As jiff is used for formatting, and as jiff is requiring correct inputs, I don't think Time::to_time() can ever be used. What I can imagine is to have a lenient version of the function that catches such an error and instead retries without an offset, possibly providing an extra method to format the offset just like Git does. Then the caller can explicitly handle this case.

Looking at the code…

gitoxide/gix-date/src/time/format.rs

Lines 53 to 58 in 57366d3

	fn to_time(self) -> jiff::Zoned {
	let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset");
	jiff::Timestamp::from_second(self.seconds)
	.expect("always valid unix time")
	.to_zoned(offset.to_time_zone())
	}

…it would already be trivial to do as a workaround, so maybe what gitoxide should provide is a lenient function that does it all.

Maybe Time::to_lenient_time() -> (jiff::Zoned, Option<gix_time::parse::TimeBuf>), where the TimeBuf holds the offset part in case it couldn't be handled by jiff.

Application code still has to handle more, but I see no other way. Maybe you do though.