git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Shengfa Lin <shengfa@google.com>
Cc: git@vger.kernel.org, nathaniel@google.com,
	rsbecker@nexbridge.com, santiago@nyu.edu, gitster@pobox.com
Subject: Re: [RFC PATCH 1/1] hideTimezone: add a user.hideTimezone config
Date: Wed, 30 Sep 2020 19:44:52 -0700	[thread overview]
Message-ID: <20201001024452.GA2930867@google.com> (raw)
In-Reply-To: <20200930232138.3656304-2-shengfa@google.com>

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@gmail.com> 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

  parent reply	other threads:[~2020-10-01  2:45 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-05 17:14 [ISSUE] Stop accessing, storing, and sharing the user's time zone Nathaniel Manista
2019-12-05 17:31 ` Junio C Hamano
2019-12-05 17:33 ` Randall S. Becker
2019-12-05 17:43   ` Junio C Hamano
2019-12-05 17:53     ` Santiago Torres Arias
2019-12-05 18:00     ` Randall S. Becker
2020-09-30 23:21 ` [RFC PATCH 0/1] adding user.hideTimezone for setting UTC timezone Shengfa Lin
2020-09-30 23:21   ` [RFC PATCH 1/1] hideTimezone: add a user.hideTimezone config Shengfa Lin
2020-09-30 23:41     ` Junio C Hamano
2020-10-01  0:17       ` Junio C Hamano
2020-10-02  6:07         ` Shengfa Lin
2020-10-01  0:31       ` Junio C Hamano
2020-10-01  0:35         ` Junio C Hamano
2020-10-02  6:41           ` Shengfa Lin
2020-10-02  6:46             ` Shengfa Lin
2020-10-02  6:37         ` Shengfa Lin
2020-10-02  6:02       ` Shengfa Lin
2020-10-02  6:15         ` Jonathan Nieder
2020-10-02 22:32           ` Shengfa Lin
2020-10-03  4:57             ` Junio C Hamano
2020-09-30 23:55     ` Junio C Hamano
2020-10-02  6:51       ` Shengfa Lin
2020-10-01  0:05     ` Junio C Hamano
2020-10-01  2:44     ` Jonathan Nieder [this message]
2020-10-02 21:17       ` Shengfa Lin
2020-09-30 23:53   ` [RFC PATCH 0/1] adding user.hideTimezone for setting UTC timezone Junio C Hamano
2020-10-01  2:17     ` Junio C Hamano
2020-10-01  3:43       ` Jonathan Nieder
2020-10-01 15:48         ` Junio C Hamano
2020-10-08 19:49           ` Junio C Hamano
     [not found]             ` <CAEOYnASgxCE5NjhoSgDwyQyAmdLhw5UyFq_Fu==8q7y6uXGz6w@mail.gmail.com>
2020-10-09 16:48               ` Junio C Hamano
2020-10-02 21:56         ` Shengfa Lin
2020-10-02 22:06           ` Junio C Hamano
2020-10-03  3:50             ` Shengfa Lin
2020-10-03  4:42               ` Junio C Hamano
2020-10-03 19:53         ` brian m. carlson
2020-10-03 22:14           ` Junio C Hamano
2020-10-02 21:42       ` Shengfa Lin
2020-10-02 21:23     ` Shengfa Lin
2020-10-13  5:28 ` [WIP v2 0/2] experiment with commit option record-time-zone Shengfa Lin
2020-10-13  5:28   ` [WIP v2 1/2] Adding a record-time-zone command option for commit Shengfa Lin
2020-10-13 20:03     ` Junio C Hamano
2020-10-21  5:01       ` Shengfa Lin
2020-10-21 18:55         ` Junio C Hamano
2020-10-22 16:27           ` Junio C Hamano
2020-10-26  4:14             ` Shengfa Lin
2020-10-13  5:28   ` [WIP v2 2/2] Demonstrate failing and passing tests Shengfa Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201001024452.GA2930867@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nathaniel@google.com \
    --cc=rsbecker@nexbridge.com \
    --cc=santiago@nyu.edu \
    --cc=shengfa@google.com \
    --subject='Re: [RFC PATCH 1/1] hideTimezone: add a user.hideTimezone config' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).