git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Torsten Bögershausen" <tboegi@web.de>
Subject: [PATCH v4 0/9] Introduce timestamp_t for timestamps
Date: Fri, 21 Apr 2017 12:45:00 +0200 (CEST)	[thread overview]
Message-ID: <cover.1492771484.git.johannes.schindelin@gmx.de> (raw)
In-Reply-To: <cover.1492721487.git.johannes.schindelin@gmx.de>

Git v2.9.2 was released in a hurry to accomodate for platforms like
Windows, where the `unsigned long` data type is 32-bit even for 64-bit
setups.

The quick fix was to simply disable all the testing with "absurd" future
dates.

However, we can do much better than that, as we already make use of
64-bit data types internally. There is no good reason why we should not
use the same for timestamps. Hence, let's use uintmax_t for timestamps.

Note: while the `time_t` data type exists and is meant to be used for
timestamps, on 32-bit Linux it is *still* 32-bit. An earlier iteration
used `time_t` for that reason, but it came with a few serious downsides:
as `time_t` can be signed (and indeed, on Windows it is an int64_t),
Git's expectation that 0 is the minimal value does no longer hold true,
introducing its own set of interesting challenges. Besides, if we *can*
handle far in the future timestamps (except for formatting them using
the system libraries), it is more consistent to do so.

The upside of using `uintmax_t` for timestamps is that we do a much
better job to support far in the future timestamps across all platforms,
including 32-bit ones. The downside is that those platforms that use a
32-bit `time_t` will barf when parsing or formatting those timestamps.

This iteration has two fixes for issues reported by Travis CI (thank
deity for Continuous Testing, eh?), one by the MacOSX nodes (but not by
the Linux nor the Windows nodes), the other by the windows node (which
reminds me to run tests after rebasing to `master` not only in a Linux
VM but also on Windows, even if it takes ages there due to the heavy
shell scripting).

After reading Junio's comments on the cover letter of v2, I had another
look at the usage of date_overflows() and was able to relax two of them.

Changes since v3:

- fixed the fix in archive-zip.c that tried to report a too large
  timestamp (and would have reported the uninitialized time_t instead)

- adjusted the so-far forgotten each_reflog() function (that was
  introduced after v1, in 80f2a6097c4 (t/helper: add test-ref-store to
  test ref-store functions, 2017-03-26)) to use timestamp_t and PRItime,
  too

- removed the date_overflows() check from time_to_tm(), as it calls
  gm_time_t() which already performs that check

- the date_overflows() check in show_ident_date() was removed, as we do
  not know at that point yet whether we use the system functions to
  render the date or not (and there would not be a problem in the latter
  case)


Johannes Schindelin (9):
  ref-filter: avoid using `unsigned long` for catch-all data type
  t0006 & t5000: prepare for 64-bit timestamps
  t0006 & t5000: skip "far in the future" test when time_t is too
    limited
  Specify explicitly where we parse timestamps
  Introduce a new "printf format" for timestamps
  Introduce a new data type for timestamps
  Abort if the system time cannot handle one of our timestamps
  Use uintmax_t for timestamps
  show_date_ident(): defer date overflow check

 Documentation/technical/api-parse-options.txt |   8 +-
 archive-tar.c                                 |   5 +-
 archive-zip.c                                 |  12 ++-
 archive.h                                     |   2 +-
 builtin/am.c                                  |   4 +-
 builtin/blame.c                               |  14 ++--
 builtin/fsck.c                                |   6 +-
 builtin/gc.c                                  |   2 +-
 builtin/log.c                                 |   4 +-
 builtin/merge-base.c                          |   2 +-
 builtin/name-rev.c                            |   6 +-
 builtin/pack-objects.c                        |   4 +-
 builtin/prune.c                               |   4 +-
 builtin/receive-pack.c                        |  14 ++--
 builtin/reflog.c                              |  24 +++---
 builtin/rev-list.c                            |   2 +-
 builtin/rev-parse.c                           |   2 +-
 builtin/show-branch.c                         |   4 +-
 builtin/worktree.c                            |   4 +-
 bundle.c                                      |   4 +-
 cache.h                                       |  14 ++--
 commit.c                                      |  18 ++---
 commit.h                                      |   2 +-
 config.c                                      |   2 +-
 credential-cache--daemon.c                    |  12 +--
 date.c                                        | 106 ++++++++++++++------------
 fetch-pack.c                                  |   8 +-
 fsck.c                                        |   2 +-
 git-compat-util.h                             |   9 +++
 http-backend.c                                |   4 +-
 parse-options-cb.c                            |   4 +-
 pretty.c                                      |  16 ++--
 reachable.c                                   |   9 +--
 reachable.h                                   |   4 +-
 ref-filter.c                                  |  22 +++---
 reflog-walk.c                                 |   8 +-
 refs.c                                        |  14 ++--
 refs.h                                        |   8 +-
 refs/files-backend.c                          |   8 +-
 revision.c                                    |   6 +-
 revision.h                                    |   4 +-
 sha1_name.c                                   |   6 +-
 t/helper/test-date.c                          |  18 +++--
 t/helper/test-parse-options.c                 |   4 +-
 t/helper/test-ref-store.c                     |   4 +-
 t/t0006-date.sh                               |   4 +-
 t/t5000-tar-tree.sh                           |   6 +-
 t/test-lib.sh                                 |   3 +
 tag.c                                         |   6 +-
 tag.h                                         |   2 +-
 upload-pack.c                                 |   8 +-
 vcs-svn/fast_export.c                         |   8 +-
 vcs-svn/fast_export.h                         |   4 +-
 vcs-svn/svndump.c                             |   2 +-
 wt-status.c                                   |   2 +-
 55 files changed, 257 insertions(+), 227 deletions(-)


base-commit: 6a2c2f8d34fa1e8f3bb85d159d354810ed63692e
Published-As: https://github.com/dscho/git/releases/tag/time_t-may-be-int64-v4
Fetch-It-Via: git fetch https://github.com/dscho/git time_t-may-be-int64-v4

Interdiff vs v3:

 diff --git a/archive-zip.c b/archive-zip.c
 index bb117a74d45..68df3d64402 100644
 --- a/archive-zip.c
 +++ b/archive-zip.c
 @@ -551,7 +551,8 @@ static void dos_time(timestamp_t *timestamp, int *dos_date, int *dos_time)
  	struct tm *t;
  
  	if (date_overflows(*timestamp))
 -		die("Timestamp too large for this system: %"PRItime, time);
 +		die("timestamp too large for this system: %"PRItime,
 +		    *timestamp);
  	time = (time_t)*timestamp;
  	t = localtime(&time);
  	*timestamp = time;
 diff --git a/date.c b/date.c
 index db3435df3e4..75f6335cd09 100644
 --- a/date.c
 +++ b/date.c
 @@ -43,12 +43,12 @@ static time_t gm_time_t(timestamp_t time, int tz)
  {
  	int minutes;
  
 -	if (date_overflows(time))
 -		die("Timestamp too large for this system: %"PRItime, time);
 -
  	minutes = tz < 0 ? -tz : tz;
  	minutes = (minutes / 100)*60 + (minutes % 100);
  	minutes = tz < 0 ? -minutes : minutes;
 +
 +	if (date_overflows(time + minutes * 60))
 +		die("Timestamp too large for this system: %"PRItime, time);
  	return (time_t)time + minutes * 60;
  }
  
 @@ -59,12 +59,7 @@ static time_t gm_time_t(timestamp_t time, int tz)
   */
  static struct tm *time_to_tm(timestamp_t time, int tz)
  {
 -	time_t t;
 -
 -	if (date_overflows(time))
 -		die("Timestamp too large for this system: %"PRItime, time);
 -
 -	t = gm_time_t((time_t)time, tz);
 +	time_t t = gm_time_t(time, tz);
  	return gmtime(&t);
  }
  
 diff --git a/pretty.c b/pretty.c
 index 587d48371b0..35fd290096a 100644
 --- a/pretty.c
 +++ b/pretty.c
 @@ -410,14 +410,10 @@ const char *show_ident_date(const struct ident_split *ident,
  
  	if (ident->date_begin && ident->date_end)
  		date = parse_timestamp(ident->date_begin, NULL, 10);
 -	if (date_overflows(date))
 -		date = 0;
 -	else {
 -		if (ident->tz_begin && ident->tz_end)
 -			tz = strtol(ident->tz_begin, NULL, 10);
 -		if (tz >= INT_MAX || tz <= INT_MIN)
 -			tz = 0;
 -	}
 +	if (ident->tz_begin && ident->tz_end)
 +		tz = strtol(ident->tz_begin, NULL, 10);
 +	if (tz >= INT_MAX || tz <= INT_MIN)
 +		tz = 0;
  	return show_date(date, tz, mode);
  }
  
 diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
 index 2d84c45ffe9..9077ec2c330 100644
 --- a/t/helper/test-ref-store.c
 +++ b/t/helper/test-ref-store.c
 @@ -138,10 +138,10 @@ static int cmd_for_each_reflog(struct ref_store *refs, const char **argv)
  }
  
  static int each_reflog(struct object_id *old_oid, struct object_id *new_oid,
 -		       const char *committer, unsigned long timestamp,
 +		       const char *committer, timestamp_t timestamp,
  		       int tz, const char *msg, void *cb_data)
  {
 -	printf("%s %s %s %lu %d %s\n",
 +	printf("%s %s %s %"PRItime" %d %s\n",
  	       oid_to_hex(old_oid), oid_to_hex(new_oid),
  	       committer, timestamp, tz, msg);
  	return 0;

-- 
2.12.2.windows.2.406.gd14a8f8640f


  parent reply	other threads:[~2017-04-21 10:45 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27 21:30 [PATCH 0/6] Use time_t Johannes Schindelin
2017-02-27 21:30 ` [PATCH 1/6] t0006 & t5000: prepare for 64-bit time_t Johannes Schindelin
2017-02-27 22:55   ` Junio C Hamano
2017-02-27 21:30 ` [PATCH 2/6] Specify explicitly where we parse timestamps Johannes Schindelin
2017-02-27 22:37   ` Junio C Hamano
2017-02-27 22:51     ` Junio C Hamano
2017-02-28 10:49       ` Johannes Schindelin
2017-02-27 21:31 ` [PATCH 3/6] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-03-01 18:20   ` Junio C Hamano
2017-03-01 19:53     ` Junio C Hamano
2017-02-27 21:31 ` [PATCH 4/6] Prepare for timestamps to use 64-bit signed types Johannes Schindelin
2017-02-27 21:31 ` [PATCH 5/6] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-02-27 21:31 ` [PATCH 6/6] Use time_t where appropriate Johannes Schindelin
2017-02-27 22:48 ` [PATCH 0/6] Use time_t Junio C Hamano
2017-02-28 11:32   ` Johannes Schindelin
2017-02-28 14:28 ` Jeff King
2017-02-28 15:01   ` Johannes Schindelin
2017-02-28 16:38   ` René Scharfe
2017-02-28 18:55     ` Junio C Hamano
2017-02-28 20:04       ` Jeff King
2017-02-28 20:54       ` Johannes Schindelin
2017-02-28 21:31         ` Jeff King
2017-02-28 21:31         ` René Scharfe
2017-02-28 23:10           ` Johannes Schindelin
2017-03-01  0:59             ` René Scharfe
2017-02-28 17:26   ` Junio C Hamano
2017-02-28 20:01     ` Jeff King
2017-02-28 22:27       ` Junio C Hamano
2017-02-28 22:33         ` Jeff King
2017-03-01 17:23           ` Junio C Hamano
2017-04-02 19:06 ` [PATCH v2 0/8] Introduce timestamp_t for timestamps Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 1/8] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-04-03  4:22     ` Torsten Bögershausen
2017-04-03 22:47       ` Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 2/8] t0006 & t5000: prepare for 64-bit timestamps Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 3/8] t0006 & t5000: skip "far in the future" test when time_t is too limited Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 4/8] Specify explicitly where we parse timestamps Johannes Schindelin
2017-04-03  4:26     ` Torsten Bögershausen
2017-04-03 22:50       ` Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 5/8] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-04-02 19:06   ` [PATCH v2 6/8] Introduce a new data type " Johannes Schindelin
2017-04-02 19:07   ` [PATCH v2 7/8] Abort if the system time cannot handle one of our timestamps Johannes Schindelin
2017-04-02 19:07   ` [PATCH v2 8/8] Use uintmax_t for timestamps Johannes Schindelin
2017-04-20 20:52   ` [PATCH v3 0/8] Introduce timestamp_t " Johannes Schindelin
2017-04-20 20:52     ` [PATCH v3 1/8] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-04-20 20:52     ` [PATCH v3 2/8] t0006 & t5000: prepare for 64-bit timestamps Johannes Schindelin
2017-04-20 20:58     ` [PATCH v3 3/8] t0006 & t5000: skip "far in the future" test when time_t is too limited Johannes Schindelin
2017-04-20 20:58     ` [PATCH v3 4/8] Specify explicitly where we parse timestamps Johannes Schindelin
2017-04-20 20:58     ` [PATCH v3 5/8] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-04-20 20:58     ` [PATCH v3 6/8] Introduce a new data type " Johannes Schindelin
2017-04-20 20:58     ` [PATCH v3 7/8] Abort if the system time cannot handle one of our timestamps Johannes Schindelin
2017-04-20 20:59     ` [PATCH v3 8/8] Use uintmax_t for timestamps Johannes Schindelin
2017-04-21  6:05     ` [PATCH v3 0/8] Introduce timestamp_t " Junio C Hamano
2017-04-21 10:44       ` Johannes Schindelin
2017-04-21 10:45     ` Johannes Schindelin [this message]
2017-04-21 10:45       ` [PATCH v4 1/9] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-04-21 10:45       ` [PATCH v4 2/9] t0006 & t5000: prepare for 64-bit timestamps Johannes Schindelin
2017-04-21 10:45       ` [PATCH v4 3/9] t0006 & t5000: skip "far in the future" test when time_t is too limited Johannes Schindelin
2017-04-21 10:45       ` [PATCH v4 4/9] Specify explicitly where we parse timestamps Johannes Schindelin
2017-04-24  3:19         ` Junio C Hamano
2017-04-21 10:45       ` [PATCH v4 5/9] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-04-21 10:45       ` [PATCH v4 6/9] Introduce a new data type " Johannes Schindelin
2017-04-21 10:45       ` [PATCH v4 7/9] Abort if the system time cannot handle one of our timestamps Johannes Schindelin
2017-04-24  3:16         ` Junio C Hamano
2017-04-24 13:57           ` Johannes Schindelin
2017-04-25  2:37             ` Junio C Hamano
2017-04-25  3:56             ` Junio C Hamano
2017-04-21 10:46       ` [PATCH v4 8/9] Use uintmax_t for timestamps Johannes Schindelin
2017-04-24  3:24         ` Junio C Hamano
2017-04-24 10:28           ` Johannes Schindelin
2017-04-25  3:59             ` Junio C Hamano
2017-04-25 20:10               ` Johannes Schindelin
2017-04-26  1:52                 ` Junio C Hamano
2017-04-26  3:45                   ` Junio C Hamano
2017-04-26  9:32                   ` Johannes Schindelin
2017-04-26 13:18                     ` Junio C Hamano
2017-04-21 10:46       ` [PATCH v4 9/9] show_date_ident(): defer date overflow check Johannes Schindelin
2017-04-24  3:29       ` [PATCH v4 0/9] Introduce timestamp_t for timestamps Junio C Hamano
2017-04-24  6:15         ` Jacob Keller
2017-04-24 14:02           ` Johannes Schindelin
2017-04-24 11:37         ` Jeff King
2017-04-25 20:13           ` Johannes Schindelin
2017-04-24 14:00         ` Johannes Schindelin
2017-04-24 13:57       ` [PATCH v5 0/8] " Johannes Schindelin
2017-04-24 13:57         ` [PATCH v5 1/8] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-04-24 13:57         ` [PATCH v5 2/8] t0006 & t5000: prepare for 64-bit timestamps Johannes Schindelin
2017-04-24 13:58         ` [PATCH v5 3/8] t0006 & t5000: skip "far in the future" test when time_t is too limited Johannes Schindelin
2017-04-24 13:58         ` [PATCH v5 4/8] Specify explicitly where we parse timestamps Johannes Schindelin
2017-04-25  5:59           ` Junio C Hamano
2017-04-24 13:58         ` [PATCH v5 5/8] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-04-24 13:58         ` [PATCH v5 6/8] Introduce a new data type " Johannes Schindelin
2017-04-26 16:43           ` Johannes Sixt
2017-04-26 19:18             ` Johannes Schindelin
2017-04-26 22:32           ` René Scharfe
2017-04-24 13:58         ` [PATCH v5 7/8] Abort if the system time cannot handle one of our timestamps Johannes Schindelin
2017-04-24 13:58         ` [PATCH v5 8/8] Use uintmax_t for timestamps Johannes Schindelin
2017-04-26 16:36           ` Johannes Sixt
2017-04-26 19:09             ` Johannes Schindelin
2017-04-25 21:54         ` [PATCH v5 0/8] Introduce timestamp_t " René Scharfe
2017-04-25 22:22           ` Johannes Schindelin
2017-04-26 22:09             ` René Scharfe
2017-04-26  1:56           ` Junio C Hamano
2017-04-26 19:20         ` [PATCH v6 " Johannes Schindelin
2017-04-26 19:26           ` [PATCH v6 1/8] ref-filter: avoid using `unsigned long` for catch-all data type Johannes Schindelin
2017-04-26 19:26           ` [PATCH v6 2/8] t0006 & t5000: prepare for 64-bit timestamps Johannes Schindelin
2017-04-26 19:26           ` [PATCH v6 3/8] t0006 & t5000: skip "far in the future" test when time_t is too limited Johannes Schindelin
2017-04-26 19:26           ` [PATCH v6 4/8] Specify explicitly where we parse timestamps Johannes Schindelin
2017-04-26 19:29           ` [PATCH v6 5/8] Introduce a new "printf format" for timestamps Johannes Schindelin
2017-04-26 19:29           ` [PATCH v6 6/8] Introduce a new data type " Johannes Schindelin
2017-05-20  5:47             ` [PATCH] name-rev: change a "long" variable to timestamp_t Junio C Hamano
2017-05-22 13:39               ` Johannes Schindelin
2017-04-26 19:29           ` [PATCH v6 7/8] Abort if the system time cannot handle one of our timestamps Johannes Schindelin
2017-04-26 19:29           ` [PATCH v6 8/8] Use uintmax_t for timestamps Johannes Schindelin

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=cover.1492771484.git.johannes.schindelin@gmx.de \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=tboegi@web.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).