[ISSUE] Stop accessing, storing, and sharing the user's time zone

> Affected Version: All? This has been bothering me at least a year.
> 
> What steps will reproduce the problem?
> 1. Author a commit.
> 2. "git log --pretty=fuller"
> 
> What is the expected output?
> The log will display that the timestamps of the commit have both the
> author time and committer time in UTC. Internally no part of the
> commit will have stored any time zone information and when the commit
> is shared with others no information about where the user was in the
> world at the time of the commit will be obtainable from it.
> 
> What do you see instead?
> Authoring and sharing a commit by default exposes the user's time zone.
> 
> Additional context:
> "commit --date=YYYY-MM-DDThh:mm:ss+0000" suffices to put the author
> time in UTC but not the commit time in UTC. But the user shouldn't
> have to pass a flag at all.
> 
> Where the user is in the world is PII that git ought not to record and
> make available as part of the user's software engineering (make
> available to colleagues, in the case of proprietary development, and
> make available to the world, in the case of open source). Git should
> entirely stop accessing, recording, and sharing the user's time zone,
> full stop. Failing that, git should by default stop accessing,
> recording, and sharing the user's time zone, but if individual users
> want to have their time zones on their commits, they can opt into it.
> Failing that, users should be able to add a .gitconfig line to ensure
> that all author timestamps, all committer timestamps, and any other
> information are in UTC.

> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> > .... I currently am
> > running a repository located at UTC+2, with developers at UTC-5. It is
> > driving us a bit wonky. I would rather see only UTC.
> 
> If "seeing" is the primary reason (i.e. you want to compare times your

> committer name.  Those who want to hide can hide but in order to keep
> track of provenance who did what when, we do record them.
> 
> As you can guess from the above reasoning, I am not fundamentally opposed
> to introducing user.tz to complement existing user.name and user.email
> configuration variables.

> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> > .... I currently am
> > running a repository located at UTC+2, with developers at
> > UTC-5. It is driving us a bit wonky. I would rather see only UTC.
>
> I also feel that TZ being PII is not particularly a brilliant
> argument against recording TZ---of course it is PII, so are the
> committer e-mail address and the committer name.  Those who want to
> hide can hide but in order to keep track of provenance who did what
> when, we do record them.

> Users requested hiding location in the world from source control
> trail. This is an implementation to read user.hideTimezone in
> cmd_commit and set timezone to UTC if it's true.
>
> Added a brief explanation of the new field in Documentation
> and added tests for true/false and reset-author
>
> Signed-off-by: Shengfa Lin <shengfa@google.com>
> ---
>  Documentation/config/user.txt   |  4 ++++
>  builtin/commit.c                |  5 +++++

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 42b964e0ca..fb1cbb8a39 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>  	s.colopts = 0;
>  
> +  git_config(git_default_config, NULL);

> +    setenv("TZ", "UTC", 1);
> +
>  	if (get_oid("HEAD", &oid))
>  		current_head = NULL;
>  	else {
> diff --git a/t/t7527-commit-hide-timezone.sh b/t/t7527-commit-hide-timezone.sh
> new file mode 100755
> index 0000000000..41ed9c27da

> --- /dev/null
> +++ b/t/t7527-commit-hide-timezone.sh
> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +
> +test_description='git-commit can override local timezone setting by reading user.hideTimezone from config'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'commit date shows timezone offset +0300 when user.hideTimezone is false' '
> +        git config user.hideTimezone false &&
> +        echo test1 >> file &&

> +        git add file &&
> +        # unset GIT_AUTHOR_DATE from test_tick
> +        unset GIT_AUTHOR_DATE &&
> +        TZ=Europe/Istanbul git commit -m initial &&
> +        git log -1 > output &&
> +        grep "Date: .* +0300" output

> +test_expect_success 'commit date shows timezone offset +0000 when user.hideTimezone is true and reset' '
> +        git config user.hideTimezone true &&
> +        git commit --amend --reset-author &&
> +        git log -1 > output &&
> +        grep "Date: .* +0000" output

> +'
> +
> +test_expect_success 'commit date shows timezone offset +0000 even TZ setting says otherwise' '
> +        git config user.hideTimezone true &&
> +        echo test2 >> file &&
> +        git add file &&
> +        # TZ setting corresponding to -0600 or -0500 depending on DST
> +        # unset GIT_AUTHOR_DATE from test_tick
> +        unset GIT_AUTHOR_DATE &&
> +        TZ=America/Chicago git commit -m test2 &&

> +        git log -1 > output &&
> +        grep "Date: .* +0000" output
> +'
> +
> +test_done

>> new file mode 100755
>> index 0000000000..41ed9c27da
>
> Let's not waste a test number for just a single test or two.  Can't
> we roll this into 

>>> new file mode 100755
>>> index 0000000000..41ed9c27da
>>
>> Let's not waste a test number for just a single test or two.  Can't
>> we roll this into 
>
> ... an existing test for "git commit"?

>> +test_expect_success 'commit date shows timezone offset +0000 when user.hideTimezone is true and reset' '
>> +        git config user.hideTimezone true &&
>> +        git commit --amend --reset-author &&
>> +        git log -1 > output &&
>> +        grep "Date: .* +0000" output
>
> This one IS interesting, but keep the GIT_AUTHOR_DATE set and
> exported.  As long as that is from a timezone different from UTC, we
> are testing what we want to test here.

> 	test_config user.hideTimeZone true &&
> 	(
> 		export GIT_AUTHOR_DATE=@1600000000 TZ=UTC-09 &&
> 		git commit ... &&
> 		git show -s --format='%aI' >output &&
> 		echo 2020-09-13T15:26:40+03:00 >expect &&

>> 	test_config user.hideTimeZone true &&
>> 	(
>> 		export GIT_AUTHOR_DATE=@1600000000 TZ=UTC-09 &&
>> 		git commit ... &&
>> 		git show -s --format='%aI' >output &&
>> 		echo 2020-09-13T15:26:40+03:00 >expect &&
>
> Oops.  The sample date and zone must be adjusted for the values
> exported above.  I expect that we'd need to do
> 
> 		...
> 		echo 2020-09-13T12:26:40+00:00 >expect &&
> 		test_cmp expect output

>>> 	test_config user.hideTimeZone true &&
>>> 	(
>>> 		export GIT_AUTHOR_DATE=@1600000000 TZ=UTC-09 &&
>>> 		git commit ... &&
>>> 		git show -s --format='%aI' >output &&
>>> 		echo 2020-09-13T15:26:40+03:00 >expect &&
>>
>> Oops.  The sample date and zone must be adjusted for the values
>> exported above.  I expect that we'd need to do
>> 
>> 		...
>> 		echo 2020-09-13T12:26:40+00:00 >expect &&
>> 		test_cmp expect output
>
> Tested the following:
> 
> test_expect_success 'commit date shows timezone offset +0000 when user.hideTimezone is true' '
>         test_config user.hideTimezone true &&
>         (
>                 echo test2 >file &&
>                 git add file &&
>                 export GIT_AUTHOR_DATE=@1600000000 TZ=UTC-09 &&
>                 git commit -m "test2" &&
>                 git show -s --format='%aI' >output &&
>                 echo 2020-09-13T12:26:40+00:00 >expect &&
>                 test_cmp output expect
>         )
> '

> 
> I think (haven't actually tested) "git commit --date=<datestring>" option
> is handled the same way, i.e. comparing these two would be a way not
> to touch the environment variable.
> 
>     TZ=UTC-09 git commit --date=@1600000000 ... &&
>     TZ=UTC-09 git -c user.hideTimeZone=true commit --date=@1600000000 ... &&
>     git show -s --format='%aI' HEAD~1 >output0 &&
>     git show -s --format='%aI' HEAD~0 >output1

>> Users requested hiding location in the world from source control
>> trail. This is an implementation to read user.hideTimezone in
>> cmd_commit and set timezone to UTC if it's true.
>>
>> Added a brief explanation of the new field in Documentation
>> and added tests for true/false and reset-author
>>
>> Signed-off-by: Shengfa Lin <shengfa@google.com>
>> ---
>>  Documentation/config/user.txt   |  4 ++++
>>  builtin/commit.c                |  5 +++++
>
> There are many ways other than running "git commit" for a commit to
> be created, including but not limited to "git merge", "git rebase",
> "git pull" (with or without "--rebase").

>> +user.hideTimezone::
>> +  Override TZ to UTC for Git commits to hide user's timezone in commit
>> +  date
>
> One level of indentation in this codebase is a single HT.
>
> Unterminated sentence.

>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 42b964e0ca..fb1cbb8a39 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>>  	s.colopts = 0;
>>  
>> +  git_config(git_default_config, NULL);
>
>Declaration after statement is not tolerated in this codebase.

>> +  int hide_timezone = 0;
>
> Unnecessary initialization.
>> +  if (!git_config_get_bool("user.hideTimezone", &hide_timezone)  && hide_timezone)
>

>> +        git add file &&
>> +        # unset GIT_AUTHOR_DATE from test_tick
>> +        unset GIT_AUTHOR_DATE &&
>> +        TZ=Europe/Istanbul git commit -m initial &&
>> +        git log -1 > output &&
>> +        grep "Date: .* +0300" output
>
> Do they not have DST over there, and is it guaranteed that they will
> never have one?  Would we see this test fail about half of the year,
> when timezone rules change over there in some future year?  After
> all, they changed in 2016 last time, which is fairly recent.

> This test attempts to establish the baseline, but I do not think it
> is a good idea.  I think it is better *not* to unset GIT_AUTHOR_DATE
> like this.  Instead, make sure it is set to some timestamp in some
> timezone that is not UTC, and the timezone of the resulting commit
> author date is in that timezone.  But that must have already been
> done in basic tests on "git commit" that we honor the environment
> variable, no?  Which means there is no need to add yet another extra
> baseline test here.
>

>> +'
>> +
>> +test_expect_success 'commit date shows timezone offset +0000 even TZ setting says otherwise' '
>> +        git config user.hideTimezone true &&
>> +        echo test2 >> file &&
>> +        git add file &&
>> +        # TZ setting corresponding to -0600 or -0500 depending on DST
>> +        # unset GIT_AUTHOR_DATE from test_tick
>> +        unset GIT_AUTHOR_DATE &&
>> +        TZ=America/Chicago git commit -m test2 &&
>
> This one is a borderline meh, compared to a test with explicit
> GIT_AUTHOR_DATE getting overridden by the configuration.  It is not
> all that wrong, but I do not see a point in adding cycles to the
> already big testsuite.
>

>>> +user.hideTimezone::
>>> +  Override TZ to UTC for Git commits to hide user's timezone in commit
>>> +  date
>>
>> One level of indentation in this codebase is a single HT.
>>
>> Unterminated sentence.
>
> What does HT stands for? I will change the indentation to 8 spaces.

>>> --- a/builtin/commit.c
>>> +++ b/builtin/commit.c
>>> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>>>  	s.colopts = 0;
>>>  
>>> +  git_config(git_default_config, NULL);
>>
>> Declaration after statement is not tolerated in this codebase.
>
> If I use the DEVELOPER=1 flag in config.mak and call make again, would the compiler
> catches this as an error?

>>> +  int hide_timezone = 0;
>>
>> Unnecessary initialization.
>>
>>> +  if (!git_config_get_bool("user.hideTimezone", &hide_timezone)  && hide_timezone)
>
> Is it unnecessary because I am checking the return value from git_config_get_bool so
> that the uninitialized value won't be used?

>>             Instead, make sure it is set to some timestamp in some
>> timezone that is not UTC, and the timezone of the resulting commit
>> author date is in that timezone.  But that must have already been
>> done in basic tests on "git commit" that we honor the environment
>> variable, no?  Which means there is no need to add yet another extra
>> baseline test here.
>
> I am not sure if this test has already been done in commit basic tests.
> Will remove this test.

>>> [...]
>>
>> What does HT stands for? I will change the indentation to 8 spaces.
>
> HT means "horizontal tab", like might be shown with "man ascii".
> 
> Git uses tabs for indentation.  This file is documentation instead of
> source so clang-format doesn't know about it, but I might as well
> mention anyway: if you run "make style", then clang-format will give
> some suggestions around formatting.  The configuration for that is not
> yet perfect so you can take its suggestions with a grain of salt, but
> they should get you in the right direction.
>

>[...]
>>>> --- a/builtin/commit.c
>>>> +++ b/builtin/commit.c
>>>> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>>>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>>>>  	s.colopts = 0;
>>>>  
>>>> +  git_config(git_default_config, NULL);
>>>
>>> Declaration after statement is not tolerated in this codebase.
>>
>> If I use the DEVELOPER=1 flag in config.mak and call make again, would the compiler
>> catches this as an error?
>
> Yes, DEVELOPER_CFLAGS includes -Wdeclaration-after-statement.
>

>>>> +  int hide_timezone = 0;
>>>
>>> Unnecessary initialization.
>>>
>>>> +  if (!git_config_get_bool("user.hideTimezone", &hide_timezone)  && hide_timezone)
>>
>> Is it unnecessary because I am checking the return value from git_config_get_bool so
>> that the uninitialized value won't be used?
>
> By leaving it uninitialized, you can help avoid the reader wondering
> whether there is some code path where the default value is used.
>

>[...]
>>>             Instead, make sure it is set to some timestamp in some
>>> timezone that is not UTC, and the timezone of the resulting commit
>>> author date is in that timezone.  But that must have already been
>>> done in basic tests on "git commit" that we honor the environment
>>> variable, no?  Which means there is no need to add yet another extra
>>> baseline test here.
>>
>> I am not sure if this test has already been done in commit basic tests.
>> Will remove this test.
>
> Let's see: *checks with "git grep -e TZ -- t"*.
> 
> Looks like t0006 tests various aspects of TZ handling pretty well and
> t1100 includes of test using TZ with commit-tree (good).
> 
> Thanks and hope that helps,
> Jonathan

>>>>> +  git_config(git_default_config, NULL);
>>>>
>>>> Declaration after statement is not tolerated in this codebase.
>>>
>>> If I use the DEVELOPER=1 flag in config.mak and call make again, would the compiler
>>> catches this as an error?
>>
>> Yes, DEVELOPER_CFLAGS includes -Wdeclaration-after-statement.
>
> Got the error from int hide_timezone = 0; but not
> git_config(git_default_config, NULL);.
> ...
>>>>> +  int hide_timezone = 0;

>
> Added a brief explanation of the new field in Documentation
> and added tests for true/false and reset-author
>
> Signed-off-by: Shengfa Lin <shengfa@google.com>
> ---
>  Documentation/config/user.txt   |  4 ++++
>  builtin/commit.c                |  5 +++++
>  t/t7527-commit-hide-timezone.sh | 37 +++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>  create mode 100755 t/t7527-commit-hide-timezone.sh
>
> diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt
> index 59aec7c3ae..bd6813f527 100644
> --- a/Documentation/config/user.txt
> +++ b/Documentation/config/user.txt
> @@ -36,3 +36,7 @@ user.signingKey::
>  	commit, you can override the default selection with this variable.
>  	This option is passed unchanged to gpg's --local-user parameter,
>  	so you may specify a key using any method that gpg supports.
> +
> +user.hideTimezone::
> +  Override TZ to UTC for Git commits to hide user's timezone in commit
> +  date
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 42b964e0ca..fb1cbb8a39 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>  	s.colopts = 0;
>  
> +  git_config(git_default_config, NULL);
> +  int hide_timezone = 0;
> +  if (!git_config_get_bool("user.hideTimezone", &hide_timezone)  && hide_timezone)
> +    setenv("TZ", "UTC", 1);
> +
>  	if (get_oid("HEAD", &oid))
>  		current_head = NULL;
>  	else {
> diff --git a/t/t7527-commit-hide-timezone.sh b/t/t7527-commit-hide-timezone.sh
> new file mode 100755
> index 0000000000..41ed9c27da
> --- /dev/null
> +++ b/t/t7527-commit-hide-timezone.sh
> @@ -0,0 +1,37 @@
> +#!/bin/sh
> +
> +test_description='git-commit can override local timezone setting by reading user.hideTimezone from config'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'commit date shows timezone offset +0300 when user.hideTimezone is false' '
> +        git config user.hideTimezone false &&
> +        echo test1 >> file &&
> +        git add file &&
> +        # unset GIT_AUTHOR_DATE from test_tick
> +        unset GIT_AUTHOR_DATE &&
> +        TZ=Europe/Istanbul git commit -m initial &&
> +        git log -1 > output &&
> +        grep "Date: .* +0300" output
> +'
> +
> +test_expect_success 'commit date shows timezone offset +0000 when user.hideTimezone is true and reset' '
> +        git config user.hideTimezone true &&
> +        git commit --amend --reset-author &&
> +        git log -1 > output &&
> +        grep "Date: .* +0000" output
> +'
> +
> +test_expect_success 'commit date shows timezone offset +0000 even TZ setting says otherwise' '
> +        git config user.hideTimezone true &&
> +        echo test2 >> file &&
> +        git add file &&
> +        # TZ setting corresponding to -0600 or -0500 depending on DST
> +        # unset GIT_AUTHOR_DATE from test_tick
> +        unset GIT_AUTHOR_DATE &&
> +        TZ=America/Chicago git commit -m test2 &&
> +        git log -1 > output &&
> +        grep "Date: .* +0000" output
> +'
> +
> +test_done

>> Users requested hiding location in the world from source control
>> trail. This is an implementation to read user.hideTimezone in
>> cmd_commit and set timezone to UTC if it's true.
>
> Not a very good proposed log message, that sounds as if "it is
> what 'Users' requested, so it must be a worthwhile thing to do",
> which is not the line of thinking to go by.

> Users requested hiding location in the world from source control
> trail. This is an implementation to read user.hideTimezone in
> cmd_commit and set timezone to UTC if it's true.
>
> Added a brief explanation of the new field in Documentation
> and added tests for true/false and reset-author

>  Documentation/config/user.txt   |  4 ++++
>  builtin/commit.c                |  5 +++++
>  t/t7527-commit-hide-timezone.sh | 37 +++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>  create mode 100755 t/t7527-commit-hide-timezone.sh

> --- a/Documentation/config/user.txt
> +++ b/Documentation/config/user.txt
> @@ -36,3 +36,7 @@ user.signingKey::
>  	commit, you can override the default selection with this variable.
>  	This option is passed unchanged to gpg's --local-user parameter,
>  	so you may specify a key using any method that gpg supports.
> +
> +user.hideTimezone::
> +  Override TZ to UTC for Git commits to hide user's timezone in commit
> +  date

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 42b964e0ca..fb1cbb8a39 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1569,6 +1569,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
>  	s.colopts = 0;
>  
> +  git_config(git_default_config, NULL);
> +  int hide_timezone = 0;
> +  if (!git_config_get_bool("user.hideTimezone", &hide_timezone)  && hide_timezone)
> +    setenv("TZ", "UTC", 1);

> Like Junio mentioned, this affects "git commit" but not other commands
> that record the current date with the local timezone.
> 
> The fundamental tool to exercise that machinery is
> 
> 	$ git var GIT_AUTHOR_IDENT
> 	Jonathan Nieder <jrnieder@gmail.com> 1601517809 -0700
> 
> so I suppose I'd be interested in seeing that exercised in tests.

> [...]
> 
> The unfortunate thing about these APIs is that there's no way to pass
> in a timezone from a string instead of from the environment.  This
> means that passing through the environment as above is the only
> reasonable way to do it, but that would have the unfortunate result
> of changing the output of commands like "git log --date=local" that
> are about writing dates to the terminal instead of storing them.
> 

> [...]
> Looking over callers, who would this affect?  There are three callers:
> 
>  fast-import.c::parse_ident:
> 	Used to handle ident string "now".  That seems in keeping with
> 	the intent here, and fast-import does respect some other
> 	configuration though only affecting storage.  Seems fine.sensible.
> 
>  ident.c::ident_default_date:
> 	Used to produce author and committer timestamps and timestamps for
> 	reflog entries.  That's the goal; good.
> 
>  send-pack.c::generate_push_cert:
> 	Used for the timestamp sent to the server in a signed push
> 	certificate.  Also good.
> 

> So I think this does the right thing, plus it retains the
> user-friendly feature of being able to *display* timestamps in their
> local timezone.
> 
> Now let's talk through the downsides:
> 
> It's complex.  The performance isn't likely to be bad when
> user.timezone is not set, which is nice, but it still is messier than
> I'd like to see.
> 

> What I said is that git () { TZ=UTC command git "$@" } is simple
> enough that I doubt it is worth the engineering effort to either
> read configuration file early (which is tricky) so that setenv() can
> be done early in cmd_main() and/or audit the codebase widely enough
> (which is time consuming) to make sure that we pay attention to the
> configuration variable in all codepaths that matter to do the
> setenv().

>                                                Our setenv() may not
> be early enough---before the code that decides to call a setenv()
> is run, there are many things that are outside your control (like
> the tracing subsystem, repository discovery, etc.) will run, and if
> any of them does something that triggers tzset() to be called, it
> will be done with the value of TZ the process started with, and our
> setenv() that happens much later won't have any effect to it.

> You can let the existing code produce its natural result and then
> when the "force UTC" flag is set, override the offset part to +0000
> if and only if the timezone was obtained from the current
> environment (this if-and-only-if is necessary, because you do not
> want to rewrite and force UTC when you run "git commit --amend"
> without the "--reset-author" option to update a commit that was
> created somewhere else to UTC).  That way, we do not have to futz
> with TZ environment or tzset.

> In addition to not having to futz with TZ, I think I like the
> semantics better.  The motivation that started this thread was not so
> much "I want to set a custom timezone to blend in" but rather "why are
> we recording the timezone at all here?"  In that context, it makes
> sense to me to have a setting such as
>
> 	core.recordTimeZone
>
> that I can turn *off* to say that I don't think datestamp() callers
> should consider the timezone to be information worth recording (and
> instead they should write +0000).  To me that seems a little simpler
> to understand than user.hideTimezone since this focuses on turning
> some functionality off (recording of the time zone) instead of turning
> on a new stealth mode.

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> In addition to not having to futz with TZ, I think I like the
>> semantics better.  The motivation that started this thread was not so
>> much "I want to set a custom timezone to blend in" but rather "why are
>> we recording the timezone at all here?"  In that context, it makes
>> sense to me to have a setting such as
>>
>> 	core.recordTimeZone
>>
>> that I can turn *off* to say that I don't think datestamp() callers
>> should consider the timezone to be information worth recording (and
>> instead they should write +0000).  To me that seems a little simpler
>> to understand than user.hideTimezone since this focuses on turning
>> some functionality off (recording of the time zone) instead of turning
>> on a new stealth mode.
>
> Hmph.  It is a valid way to look at the issue, I guess.
>
> Thanks for an input.

>>                                                Our setenv() may not
>> be early enough---before the code that decides to call a setenv()
>> is run, there are many things that are outside your control (like
>> the tracing subsystem, repository discovery, etc.) will run, and if
>> any of them does something that triggers tzset() to be called, it
>> will be done with the value of TZ the process started with, and our
>> setenv() that happens much later won't have any effect to it.
>
> I thought about this before, but in fact it's okay: when calling a
> function like localtime() (though not localetime_r() --- see my other
> reply), tzset() is called each time so it is able to reflect any
> updates to the TZ envvar from the interim.
> 

> [....]
> 
> In addition to not having to futz with TZ, I think I like the
> semantics better.  The motivation that started this thread was not so
> much "I want to set a custom timezone to blend in" but rather "why are
> we recording the timezone at all here?"  In that context, it makes
> sense to me to have a setting such as
> 
> 	core.recordTimeZone
> 
> that I can turn *off* to say that I don't think datestamp() callers
> should consider the timezone to be information worth recording (and
> instead they should write +0000).  To me that seems a little simpler
> to understand than user.hideTimezone since this focuses on turning
> some functionality off (recording of the time zone) instead of turning
> on a new stealth mode.
> 
> Thanks,
> Jonathan

>> In addition to not having to futz with TZ, I think I like the
>> semantics better.  The motivation that started this thread was not so
>> much "I want to set a custom timezone to blend in" but rather "why are
>> we recording the timezone at all here?"  In that context, it makes
>> sense to me to have a setting such as
>> 
>> 	core.recordTimeZone
>> 
>> that I can turn *off* to say that I don't think datestamp() callers
>> should consider the timezone to be information worth recording (and
>> instead they should write +0000).  To me that seems a little simpler
>> to understand than user.hideTimezone since this focuses on turning
>> some functionality off (recording of the time zone) instead of turning
>> on a new stealth mode.
>> 
>> Thanks,
>> Jonathan
>
> +1, simpler to understand.

>> If we have a setting of "core.recordTimeZone", do we need to make it
>> as a command option as Junio suggested earlier?
>
> Usually we add command line option --[no-]record-time-zone first
> without configuration option when introducing a new features like
> this one.  Once the feature proves useful, we'd add a matching
> configuration variable for convenience, but leave the command line
> option (negative form in this case) so that a configured per-user
> default can be overridden as needed.

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> If we have a setting of "core.recordTimeZone", do we need to make it
>>> as a command option as Junio suggested earlier?
>>
>> Usually we add command line option --[no-]record-time-zone first
>> without configuration option when introducing a new features like
>> this one.  Once the feature proves useful, we'd add a matching
>> configuration variable for convenience, but leave the command line
>> option (negative form in this case) so that a configured per-user
>> default can be overridden as needed.
>
> Any recommendation as to where to add this command line option?
> Should the option be added separately in "git merge", "git rebase",
> "git commit", "git pull" and other places as OPT_BOOL?

> Junio C Hamano wrote:
> > You can let the existing code produce its natural result and then
> > when the "force UTC" flag is set, override the offset part to +0000
> > if and only if the timezone was obtained from the current
> > environment (this if-and-only-if is necessary, because you do not
> > want to rewrite and force UTC when you run "git commit --amend"
> > without the "--reset-author" option to update a commit that was
> > created somewhere else to UTC).  That way, we do not have to futz
> > with TZ environment or tzset.
> 
> Yes, I think this is simpler and nicer than the proposal in my other
> reply.

> In addition to not having to futz with TZ, I think I like the
> semantics better.  The motivation that started this thread was not so
> much "I want to set a custom timezone to blend in" but rather "why are
> we recording the timezone at all here?"  In that context, it makes
> sense to me to have a setting such as
> 
> 	core.recordTimeZone
> 
> that I can turn *off* to say that I don't think datestamp() callers
> should consider the timezone to be information worth recording (and
> instead they should write +0000).  To me that seems a little simpler
> to understand than user.hideTimezone since this focuses on turning
> some functionality off (recording of the time zone) instead of turning
> on a new stealth mode.

> I'd like to make one suggestion here, and that's that instead of writing
> "+0000" in this case, we write "-0000".  As far as I'm aware, it should
> be parsed equivalently but it mirrors RFC 5322:
>
>   Though "-0000" also indicates Universal Time, it is used to indicate
>   that the time was generated on a system that may be in a local time
>   zone other than Universal Time and that the date-time contains no
>   information about the local time zone.
>
> This is exactly my case.  As you can tell from my emails, I'm not
> physically located in a UTC timezone, but my system is in UTC and uses
> that for timestamps.

> Regarding the implementation, the posted patch to "git commit" uses
> the "futz with TZ early inside the git process" approach, but I
> think we should not do so for another reason.  Our setenv() may not
> be early enough---before the code that decides to call a setenv()
> is run, there are many things that are outside your control (like
> the tracing subsystem, repository discovery, etc.) will run, and if
> any of them does something that triggers tzset() to be called, it
> will be done with the value of TZ the process started with, and our
> setenv() that happens much later won't have any effect to it.
> 

> All code that create object headers like committer, author and
> tagger lines use the same git_author_info() and git_committer_info()
> API, I think, which eventually goes to fmt_ident(), and they produce
> a pair <number of seconds since epoch, offset>.  This gives us an
> entirely different opening to tackle this issue from, because the
> "number of seconds since epoch" part won't change the value no
> matter what timezone you are in.
> 

> want to rewrite and force UTC when you run "git commit --amend"
> without the "--reset-author" option to update a commit that was
> created somewhere else to UTC).  That way, we do not have to futz
> with TZ environment or tzset.
> 
> Just something to think about.

>> Discussed with Jonathan and Junio regarding whether we should support
>> letting user specify timezone or restricted to UTC only. There was no
>> defnite conclusion.
>
> I do not think was involved in that part of the discussion to
> consider UTC vs custom zone, though.
> 
> What I said is that git () { TZ=UTC command git "$@" } is simple
> enough that I doubt it is worth the engineering effort to either
> read configuration file early (which is tricky) so that setenv() can
> be done early in cmd_main() and/or audit the codebase widely enough
> (which is time consuming) to make sure that we pay attention to the
> configuration variable in all codepaths that matter to do the
> setenv().

> Many places in Git record the timezone of actor when a
> timestamp is recorded, including the commiter and author
> timestamps in a commit object and the tagger timestamp in a tag
> object. Some people however prefer not to share where they
> actually are.
>
> They _could_ just say "export TZ=UTC" and be done with it,
> but the method would not easily allow them to pretend to be
> in the UTC timezone only with Git, while revealing their true
> timezone to other activities (e.g. sending e-emails?).
>
> Introduce --[no-]record-time-zone for commit command, which can be
> specified to disallow Git to record time zone. Timezone will be
> recorded by default.
>
> Note that this is a WIP and the test is failing.

> diff --git a/builtin/am.c b/builtin/am.c
> index 2c7673f74e..059cc5fce7 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -884,7 +884,7 @@ static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
>  			if (tz > 0)
>  				tz2 = -tz2;
>  
> -			fprintf(out, "Date: %s\n", show_date(timestamp, tz2, DATE_MODE(RFC2822)));
> +			fprintf(out, "Date: %s\n", show_date(timestamp, tz2, NULL, DATE_MODE(RFC2822)));

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 1dfd799ec5..ee3ca2c7ac 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1547,7 +1547,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  				N_("ok to record an empty change")),
>  		OPT_HIDDEN_BOOL(0, "allow-empty-message", &allow_empty_message,
>  				N_("ok to record a change with an empty message")),
> -
> +                OPT_BOOL(0, "record-time-zone", &record_time_zone, N_("record user timezone")),

>  		OPT_END()
>  	};
>  
> @@ -1580,6 +1580,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>  					  builtin_commit_usage,
>  					  prefix, current_head, &s);
> +
>  	if (verbose == -1)
>  		verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
>  

> +static int negative_zero(int tz, int *sign)
> +{
> +        return !tz && sign && (*sign) == '-';
> +}
> +
> +static const char *tz_fmt(int tz, int *sign)
> +{
> +        if (!negative_zero(tz, sign))
> +                return " %+05d";
> +        else
> +                return " -%04d";
> +}
> +
> +
> +static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm, int tz, int *sign, struct tm *human_tm, int human_tz, int local)
>  {
>  	struct {
>  		unsigned int	year:1,
> @@ -277,10 +293,10 @@ static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm
>  		strbuf_addf(buf, " %d", tm->tm_year + 1900);
>  
>  	if (!hide.tz)
> -		strbuf_addf(buf, " %+05d", tz);
> +                strbuf_addf(buf, tz_fmt(tz, sign), tz);
>  }
>  
> -const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
> +const char *show_date(timestamp_t time, int tz, int *signp, const struct date_mode *mode)

> @@ -826,17 +849,21 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in
>  
>  /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
>     (i.e. English) day/month names, and it doesn't work correctly with %z. */
> -int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
> +int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset, int *zero_offset_negative_sign)
>  {
>  	struct tm tm;
>  	int tm_gmt;
>  	timestamp_t dummy_timestamp;
>  	int dummy_offset;
> +        int dummy_zero_offset_negative_sign;
> +        int negative_sign;
>  	if (!timestamp)
>  		timestamp = &dummy_timestamp;
>  	if (!offset)
>  		offset = &dummy_offset;

>  	memset(&tm, 0, sizeof(tm));
>  	tm.tm_year = -1;
> @@ -848,6 +875,7 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>  	tm.tm_sec = -1;
>  	*offset = -1;
>  	tm_gmt = 0;
> +        negative_sign = 0;
>  
>  	if (*date == '@' &&
>  	    !match_object_header_date(date + 1, timestamp, offset))
> @@ -864,9 +892,11 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>  			match = match_alpha(date, &tm, offset);
>  		else if (isdigit(c))
>  			match = match_digit(date, &tm, offset, &tm_gmt);
> -		else if ((c == '-' || c == '+') && isdigit(date[1]))
> +		else if ((c == '-' || c == '+') && isdigit(date[1])) {
>  			match = match_tz(date, offset);
> -
> +                        if (c == '-')
> +                                negative_sign = 1;
> +                }

>  		if (!match) {
>  			/* BAD CRAP */
>  			match = 1;
> @@ -895,6 +925,9 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>  
>  	if (!tm_gmt)
>  		*timestamp -= *offset * 60;
> +
> +        *zero_offset_negative_sign = (!(*offset) && negative_sign);
> +

> @@ -996,15 +1030,16 @@ void parse_date_format(const char *format, struct date_mode *mode)
>  void datestamp(struct strbuf *out)
>  {
>  	time_t now;
> -	int offset;
> -	struct tm tm = { 0 };
> +	int offset = 0;
>  
> -	time(&now);
> +        time(&now);
> +        if (record_time_zone) {
> +          struct tm tm = { 0 };
> +          offset = tm_to_time_t(localtime_r(&now, &tm)) - now;
> +          offset /= 60;
> +        }
>  
> -	offset = tm_to_time_t(localtime_r(&now, &tm)) - now;
> -	offset /= 60;
> -
> -	date_string(now, offset, out);
> +	date_string(now, offset, !record_time_zone, out);
>  }
>  
>  /*
> @@ -1330,7 +1365,7 @@ timestamp_t approxidate_relative(const char *date)
>  	int offset;
>  	int errors = 0;
>  
> -	if (!parse_date_basic(date, &timestamp, &offset))
> +	if (!parse_date_basic(date, &timestamp, &offset, NULL))
>  		return timestamp;
>  
>  	get_time(&tv);
> @@ -1346,7 +1381,7 @@ timestamp_t approxidate_careful(const char *date, int *error_ret)
>  	if (!error_ret)
>  		error_ret = &dummy;
>  
> -	if (!parse_date_basic(date, &timestamp, &offset)) {
> +	if (!parse_date_basic(date, &timestamp, &offset, NULL)) {
>  		*error_ret = 0;
>  		return timestamp;
>  	}
> @@ -1371,3 +1406,4 @@ int date_overflows(timestamp_t t)
>  	sys = t;
>  	return t != sys || (t < 1) != (sys < 1);
>  }
> +

>> Many places in Git record the timezone of actor when a
>> timestamp is recorded, including the commiter and author
>> timestamps in a commit object and the tagger timestamp in a tag
>> object. Some people however prefer not to share where they
>> actually are.
>>
>> They _could_ just say "export TZ=UTC" and be done with it,
>> but the method would not easily allow them to pretend to be
>> in the UTC timezone only with Git, while revealing their true
>> timezone to other activities (e.g. sending e-emails?).
>>
>> Introduce --[no-]record-time-zone for commit command, which can be
>> specified to disallow Git to record time zone. Timezone will be
>> recorded by default.
>>
>> Note that this is a WIP and the test is failing.
>
> And there is no outline of the design decision made so far, so it is
> hard to give useful review comments.

>> diff --git a/builtin/am.c b/builtin/am.c
>> index 2c7673f74e..059cc5fce7 100644
>> --- a/builtin/am.c
>> +++ b/builtin/am.c
>> @@ -884,7 +884,7 @@ static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
>>  			if (tz > 0)
>>  				tz2 = -tz2;
>>  
>> -			fprintf(out, "Date: %s\n", show_date(timestamp, tz2, DATE_MODE(RFC2822)));
>> +			fprintf(out, "Date: %s\n", show_date(timestamp, tz2, NULL, DATE_MODE(RFC2822)));
>
> For example, it appears that tweaking "show_date()" API function
> seems to be a central part of the design, as there are so many
> instances like this change in the patch.  If the proposed log
> message mentioned, even briefly, what the extra parameter added to
> the API function meant (especially what NULL means there) upfront,
> then readers can coast the part that added NULL there, like these
> ones, and concentrate on the parts of this patch that matter, which
> presumably uses something more interesting than NULL instead.

>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 1dfd799ec5..ee3ca2c7ac 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1547,7 +1547,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>  				N_("ok to record an empty change")),
>>  		OPT_HIDDEN_BOOL(0, "allow-empty-message", &allow_empty_message,
>>  				N_("ok to record a change with an empty message")),
>> -
>> +                OPT_BOOL(0, "record-time-zone", &record_time_zone, N_("record user timezone")),
>
> Our code indents with HT; get used to the style early and your
> patches won't distract reviewers as much, leading to more quality
> reviews and suggestions.

> Likewise.  The record_time_zone global variable seems to play a
> crucial role in this change, but without preparing readers by
> mentioning where it is defined, what normal/default value it takes,
> and who potentially touches it, in the proposed log message, it
> makes reading the change harder than necessary.
> 
> A system-wide global like this is usually defined in environment.c,
> by the way.  Look for say trust_executable_bit and mimic where it 
> is defined and declared.

>>  		OPT_END()
>>  	};
>>  
>> @@ -1580,6 +1580,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>>  	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
>>  					  builtin_commit_usage,
>>  					  prefix, current_head, &s);
>> +
>>  	if (verbose == -1)
>>  		verbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;
>>  
>
> Distraction.

>> +static int negative_zero(int tz, int *sign)
>> +{
>> +        return !tz && sign && (*sign) == '-';
>> +}
>> +
>> +static const char *tz_fmt(int tz, int *sign)
>> +{
>> +        if (!negative_zero(tz, sign))
>> +                return " %+05d";
>> +        else
>> +                return " -%04d";
>> +}
>> +
>> +
>> +static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm, int tz, int *sign, struct tm *human_tm, int human_tz, int local)
>>  {
>>  	struct {
>>  		unsigned int	year:1,
>> @@ -277,10 +293,10 @@ static void show_date_normal(struct strbuf *buf, timestamp_t time, struct tm *tm
>>  		strbuf_addf(buf, " %d", tm->tm_year + 1900);
>>  
>>  	if (!hide.tz)
>> -		strbuf_addf(buf, " %+05d", tz);
>> +                strbuf_addf(buf, tz_fmt(tz, sign), tz);
>>  }
>>  
>> -const char *show_date(timestamp_t time, int tz, const struct date_mode *mode)
>> +const char *show_date(timestamp_t time, int tz, int *signp, const struct date_mode *mode)
>
> With its type, we can tell easily that sign is a pointer, so no need
> for 'p' (we do not have modep, either, next door).  More important
> is if 'sign' is a good name that conveys what the parameter (which
> is almost always NULL) means.  Without any introductory text, it is
> hard to tell and offer recommendations.

>> @@ -826,17 +849,21 @@ static int match_object_header_date(const char *date, timestamp_t *timestamp, in
>>  
>>  /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
>>     (i.e. English) day/month names, and it doesn't work correctly with %z. */
>> -int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>> +int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset, int *zero_offset_negative_sign)
>>  {
>>  	struct tm tm;
>>  	int tm_gmt;
>>  	timestamp_t dummy_timestamp;
>>  	int dummy_offset;
>> +        int dummy_zero_offset_negative_sign;
>> +        int negative_sign;
>>  	if (!timestamp)
>>  		timestamp = &dummy_timestamp;
>>  	if (!offset)
>>  		offset = &dummy_offset;
>
> I see no need for the extra dummy_zero_offset_negative_sign variable.
> I can guess this mimics "if (!offset) offset = &dummy_offset" without
> much thought---a big difference is that after calling match_tz() to
> fill *offset, the code needs to obtain the value match_tz() parsed
> to decide if it needs to do the mktime() dance to guess the current
> zone offset, and also needs to read *offset to adjust *timestamp
> the function returns.
> 
> The zero_offset_negative_sign pointer that specifies the location to
> optionally return a bit of info is *ONLY* used once at the very end
> of the function, so 
>> +        if (!zero_offset_negative_sign)
>> +                zero_offset_negative_sign = &dummy_zero_offset_negative_sign;
>
> there is absolutely no need for the dummy variable or this
> assignment, especially since the patch adds negative_sign variable
> that always exists, and ...
>
>>  	memset(&tm, 0, sizeof(tm));
>>  	tm.tm_year = -1;
>> @@ -848,6 +875,7 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>>  	tm.tm_sec = -1;
>>  	*offset = -1;
>>  	tm_gmt = 0;
>> +        negative_sign = 0;
>>  
>>  	if (*date == '@' &&
>>  	    !match_object_header_date(date + 1, timestamp, offset))
>> @@ -864,9 +892,11 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>>  			match = match_alpha(date, &tm, offset);
>>  		else if (isdigit(c))
>>  			match = match_digit(date, &tm, offset, &tm_gmt);
>> -		else if ((c == '-' || c == '+') && isdigit(date[1]))
>> +		else if ((c == '-' || c == '+') && isdigit(date[1])) {
>>  			match = match_tz(date, offset);
>> -
>> +                        if (c == '-')
>> +                                negative_sign = 1;
>> +                }
>
>... is usable.

>>  		if (!match) {
>>  			/* BAD CRAP */
>>  			match = 1;
>> @@ -895,6 +925,9 @@ int parse_date_basic(const char *date, timestamp_t *timestamp, int *offset)
>>  
>>  	if (!tm_gmt)
>>  		*timestamp -= *offset * 60;
>> +
>> +        *zero_offset_negative_sign = (!(*offset) && negative_sign);
>> +
>
> The only change needed for this optional extra bit return is to
> make sure that the assignment happens only when it was requested by
> the caller, i.e.
>
> 	if (zero_offset_negative_sign)
> 		*zero_offset_negative_sign = ...
>
> Having said all that, quite honestly, I do not know if this is going
> in the right direction.  Specifically, I do not offhand see why we
> need such a huge surgery to date.c just to touch datestamp() and
> date_string().
> 
> I may be totally off, partly due to lack of explanation of how the
> patch attempts to achieve its goal, but wouldn't it be just the
> matter of touching the single callsite of datestamp() in ident.c, so
> that after it gets git_default_date string filled, null out the last
> 5 bytes in it with "-0000" if record_tz is off?
>

> First, I would like to use a "global" variable to keep track of whether
> record-time-zone is set and default to true. Then in various places such
> as commit, pull, merge and rebase; we can add command option that can
> modify this value.
>
> Then in datestamp in date.c, we can check this value; offset would be
> initialized to 0 and only be set if record_time_zone is true. Additionally,
> date_string from the same file would take an extra argument to indicate if
> we want to use nagative sign for zero offset. Then the timestamp along with
> sign and 4 digits offset would be stored in "git_default_date" as buf
> "1603255519 -0000". I think of this as the "encoding" step.

> 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,...

> 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:
> ...

>> 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 ;-)

> 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.

> 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.

> 	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?

> 	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.

> If we say that we would like to eventually live in a world in which a
> user's time zones are not recorded until after that user deliberately opts
> into git recording zir time zones, does that point us toward an eventual
> destination of "users who don't wish to record their time zones don't have
> to do anything to their .gitconfigs, and users who do wish to record their
> time zones only have to write one line in their .gitconfigs"? If that's the
> case, ought that one line that some users choose to write be targeted to be
> "recordTimeZone = true" rather than anything else (particularly rather than
> "hideTimeZone = false")? If all that holds, does
>
> today: introduce recordTimeZone with a default of true and advertise its
> existence so that those users who have feelings one way or the other on the
> matter may explicitly set it to one value or the other
> six months from today: flip recordTimeZone's default from true to false
>
> look like a plausible sketch of how to get to the desired future state?
>
> What am I missing (and 🤞 that my "if"s hold too...)?