git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [WIP v2 1/2] Adding a record-time-zone command option for commit

From
SLShengfa Lin <shengfa@google.com>
Date
Oct 26, 2020, 04:14 UTC
Message-ID
<20201026041443.1362933-1-shengfa@google.com>
In-Reply-To
<xmqqd01ans4h.fsf@gitster.c.googlers.com>
Show 16 quoted lines
>> Yes, we could check it in datestamp(), but ... 
>>
>>> Initially, I thought this would be sufficient to show "-0000" in commit log
>>> message. However, I found that the show_date function is used for "decoding";
>>> converting timestamp and tz to more readable format. Then I realize the
>>> function won't distinguish between +0 and -0 as it only takes in a tz as
>>> argument. As a result,...
>>
>> ... I would have imagined that you do not have to deal with all
>> those complications if you don't hook this to such a low level of
>> the call graph.  That is why I wondered:
>> ...
>
> Let me answer some of my puzzlement myself; that is, I would have
> understood the change well if it were explained to me this way, and
> if that explanation matched what the patches did ;-)
Yes, I agree.
Show 8 quoted lines
> The topic has two major parts.
> 
> The code that prepares the timestamp to be recorded for the current
> user, who wants to record an anonymous timezone "-0000", is one (and
> the easier) part.  And this part could be done all inside
> ident_default_date() without touching anything in date.c; when we
> need to call datestamp(), we are getting the current time for the
> current user, so we can mask the timezone.

So for this part, there is no need to modify datestamp in dates.c. We could modify ident_default_date buf after datestamp to set the last 5 bytes to "-0000" using strcpy.

Show 8 quoted lines
> The other part is that we need to read the timestamp from existing
> records, and if we choose to distinguish between timestamp in UTC
> and timestamp with anonymous timezone, we'd need to devise a way to
> encode the anonymous timezone differently.  It is where the extra
> bit that says "this bit does not usually mean anything but only when
> the offset (which is a signed integer whose valid range is set to
> between -2400 to +2400 by date.c::match_tz()) is zero, and this bit
> is set, the zone is anonymous" comes in.
Yes, that's correct.
Show 5 quoted lines
> 	Side note.  I suspect the damage to the callchain can be
> 	limited much narrower if we didn't add this bit throughout
> 	the API.  What if we instead pick a number outside the valid
> 	range of offsets, say -10000, as a sentinel value and passed
> 	that throughout the code when we want an anonymous zone?
Good idea.
Show 9 quoted lines
> 	The functions in the callchain that care about the timezone
> 	must understand how anonymous zone is encoded anyway, so to
> 	them it's a matter of using an int plus one bit or using an
> 	int that can have a special value.  But other functions in
> 	the callchain whose sole purpose (with respect to the
> 	timezone information) is to pass it between their caller and
> 	their callee as an opaque piece of data, using just a single
> 	integer is much less error prone---the patch does not have
> 	to touch them at all.
That would be easier to follow and requires less changes as well.
> Thanks.

Thanks for the clarification. Now I think we have a much better understanding. I will try to do a better job describing patches next time.

Previous: Junio C HamanoNext: Junio C Hamano
Message 46 of 47 in “[ISSUE] Stop accessing, storing, and sharing the user's time zone”
  1. Nathaniel ManistaDec 5, 2019
  2. Junio C HamanoDec 5, 2019
  3. Randall S. BeckerDec 5, 2019
  4. Junio C HamanoDec 5, 2019
  5. Randall S. BeckerDec 5, 2019
  6. Santiago Torres AriasDec 5, 2019
  7. 0/1 adding user.hideTimezone for setting UTC timezoneShengfa Lin, Sep 30, 2020
  8. 1/1 hideTimezone: add a user.hideTimezone configShengfa Lin, Sep 30, 2020
  9. Junio C HamanoSep 30, 2020
  10. Junio C HamanoOct 1, 2020
  11. Shengfa LinOct 2, 2020
  12. Junio C HamanoOct 1, 2020
  13. Junio C HamanoOct 1, 2020
  14. Shengfa LinOct 2, 2020
  15. Shengfa LinOct 2, 2020
  16. Shengfa LinOct 2, 2020
  17. Shengfa LinOct 2, 2020
  18. Jonathan NiederOct 2, 2020
  19. Shengfa LinOct 2, 2020
  20. Junio C HamanoOct 3, 2020
  21. Junio C HamanoSep 30, 2020
  22. Shengfa LinOct 2, 2020
  23. Junio C HamanoOct 1, 2020
  24. Jonathan NiederOct 1, 2020
  25. Shengfa LinOct 2, 2020
  26. Junio C HamanoSep 30, 2020
  27. Junio C HamanoOct 1, 2020
  28. Jonathan NiederOct 1, 2020
  29. Junio C HamanoOct 1, 2020
  30. Junio C HamanoOct 8, 2020
  31. Shengfa LinOct 2, 2020
  32. Junio C HamanoOct 2, 2020
  33. Shengfa LinOct 3, 2020
  34. Junio C HamanoOct 3, 2020
  35. brian m. carlsonOct 3, 2020
  36. Junio C HamanoOct 3, 2020
  37. Shengfa LinOct 2, 2020
  38. Shengfa LinOct 2, 2020
  39. 0/2 experiment with commit option record-time-zoneShengfa Lin, Oct 13, 2020
  40. 2/2 Demonstrate failing and passing testsShengfa Lin, Oct 13, 2020
  41. 1/2 Adding a record-time-zone command option for commitShengfa Lin, Oct 13, 2020
  42. Junio C HamanoOct 13, 2020
  43. Shengfa LinOct 21, 2020
  44. Junio C HamanoOct 21, 2020
  45. Junio C HamanoOct 22, 2020
  46. Shengfa LinOct 26, 2020
  47. Junio C HamanoOct 9, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.