Hi,
Shengfa Lin wrote:
> 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
Thanks for this discussion starter. I'm interested to see what we end
up with.
To summarize the thread so far:
Nathaniel Manista, who we can credit with a `Reported-by` line,
reports that (mildly paraphrased):
Authoring and sharing a commit by default exposes the user's
time zone.
"gt commit --date=YYYY-MM-DDThh:mm:ss+0000" suffices to put
the author time in UTC (though 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 private information 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.
On the other hand, various others have mentioned some beneficial
aspects of recording the timezone --- for example, it makes it easier
to make sense of the chronology in a program's development. What
seems uncontroversial is that users should have control over the time
zone used (as they already do, via the TZ environment variable).
In response to the suggestion of a "[user] timeZone" setting,
Nathaniel suggests
That sounds like a great first step and like something that
wouldn't ruffle anyone's feathers while proving the value of
ignoring time zone information. 🙂
There is more to discuss in this design space --- let's see where the
patch leads us.
> --- 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
To me, this seems less appealing than being able to set the time zone.
By setting the time zone to match others in the same project, I would
be able to blend in without sharing information about my travels. If
I use the hideTimezone setting instead, then I may stick out as the
only contributor using UTC.
UTC is the default timezone in various development environments so
this is not a big deal, but it seems enough reason to prefer the
fuller-control approach.
> 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@xxxxxxxxx> 1601517809 -0700
so I suppose I'd be interested in seeing that exercised in tests.
Looking at the implementation, I find fmt_ident in ident.c, which
calls ident_default_date(), which calls date.c's datestamp, which uses
localtime_r to get the timezone. localtime_r on all platforms I know
of calls tzset, though apparently[*] it is not required to.
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.
So I'd be tempted to do something targetted like this:
-- 8< --
diff --git i/date.c w/date.c
index f9ea807b3a9..658ba1a9a45 100644
--- i/date.c
+++ w/date.c
@@ -5,6 +5,7 @@
*/
#include "cache.h"
+#include "config.h"
/*
* This is like mktime, but without normalization of tm_wday and tm_yday.
@@ -998,6 +999,20 @@ void datestamp(struct strbuf *out)
time_t now;
int offset;
struct tm tm = { 0 };
+ const char *env_tz;
+ char *config_tz = NULL;
+ int restore_tz = 0;
+
+ if (!git_config_get_string("user.timezone", &config_tz)) {
+ env_tz = getenv("TZ");
+ if (env_tz)
+ env_tz = xstrdup(env_tz);
+ if (!env_tz || strcmp(env_tz, config_tz)) {
+ restore_tz = 1;
+ setenv("TZ", config_tz, 1);
+ tzset();
+ }
+ }
time(&now);
@@ -1005,6 +1020,14 @@ void datestamp(struct strbuf *out)
offset /= 60;
date_string(now, offset, out);
+
+ if (restore_tz) {
+ if (env_tz)
+ setenv("TZ", env_tz, 1);
+ else
+ unsetenv("TZ");
+ }
+ free(config_tz);
}
/*
-- >8 --
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.
It's specific to Git, but a user might want to disable recording their
timezone in *all* collaboration tools (mail clients, newsreaders,
etc), not just Git. So I would find it more compelling if there were
a common convention shared across tools for setting your exposed
timezone, just like the EMAIL envvar for setting your exposed email
address.
Thoughts?
Thanks,
Jonathan
[*] https://pubs.opengroup.org/onlinepubs/9699919799/functions/localtime.html
