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

[PATCH v4 0/9] Introduce timestamp_t for timestamps

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Apr 21, 2017, 10:45 UTC
Message-ID
<cover.1492771484.git.johannes.schindelin@gmx.de>
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
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 55 of 113 in “Use time_t”
  1. 0/6 Use time_tJohannes Schindelin, Feb 27, 2017
  2. 5/6 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Feb 27, 2017
  3. 4/6 Prepare for timestamps to use 64-bit signed typesJohannes Schindelin, Feb 27, 2017
  4. 3/6 Introduce a new "printf format" for timestampsJohannes Schindelin, Feb 27, 2017
  5. Junio C HamanoMar 1, 2017
  6. Junio C HamanoMar 1, 2017
  7. 2/6 Specify explicitly where we parse timestampsJohannes Schindelin, Feb 27, 2017
  8. Junio C HamanoFeb 27, 2017
  9. Junio C HamanoFeb 27, 2017
  10. Johannes SchindelinFeb 28, 2017
  11. 1/6 t0006 & t5000: prepare for 64-bit time_tJohannes Schindelin, Feb 27, 2017
  12. Junio C HamanoFeb 27, 2017
  13. 6/6 Use time_t where appropriateJohannes Schindelin, Feb 27, 2017
  14. Junio C HamanoFeb 27, 2017
  15. Johannes SchindelinFeb 28, 2017
  16. Jeff KingFeb 28, 2017
  17. Johannes SchindelinFeb 28, 2017
  18. René ScharfeFeb 28, 2017
  19. Junio C HamanoFeb 28, 2017
  20. Jeff KingFeb 28, 2017
  21. Johannes SchindelinFeb 28, 2017
  22. Jeff KingFeb 28, 2017
  23. René ScharfeFeb 28, 2017
  24. Johannes SchindelinFeb 28, 2017
  25. René ScharfeMar 1, 2017
  26. Junio C HamanoFeb 28, 2017
  27. Jeff KingFeb 28, 2017
  28. Junio C HamanoFeb 28, 2017
  29. Jeff KingFeb 28, 2017
  30. Junio C HamanoMar 1, 2017
  31. 0/8 Introduce timestamp_t for timestampsJohannes Schindelin, Apr 2, 2017
  32. 1/8 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Apr 2, 2017
  33. Torsten BögershausenApr 3, 2017
  34. Johannes SchindelinApr 3, 2017
  35. 2/8 t0006 & t5000: prepare for 64-bit timestampsJohannes Schindelin, Apr 2, 2017
  36. 3/8 t0006 & t5000: skip "far in the future" test when time_t is too limitedJohannes Schindelin, Apr 2, 2017
  37. 4/8 Specify explicitly where we parse timestampsJohannes Schindelin, Apr 2, 2017
  38. Torsten BögershausenApr 3, 2017
  39. Johannes SchindelinApr 3, 2017
  40. 5/8 Introduce a new "printf format" for timestampsJohannes Schindelin, Apr 2, 2017
  41. 6/8 Introduce a new data type for timestampsJohannes Schindelin, Apr 2, 2017
  42. 7/8 Abort if the system time cannot handle one of our timestampsJohannes Schindelin, Apr 2, 2017
  43. 8/8 Use uintmax_t for timestampsJohannes Schindelin, Apr 2, 2017
  44. 0/8 Introduce timestamp_t for timestampsJohannes Schindelin, Apr 20, 2017
  45. 1/8 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Apr 20, 2017
  46. 2/8 t0006 & t5000: prepare for 64-bit timestampsJohannes Schindelin, Apr 20, 2017
  47. 3/8 t0006 & t5000: skip "far in the future" test when time_t is too limitedJohannes Schindelin, Apr 20, 2017
  48. 4/8 Specify explicitly where we parse timestampsJohannes Schindelin, Apr 20, 2017
  49. 5/8 Introduce a new "printf format" for timestampsJohannes Schindelin, Apr 20, 2017
  50. 6/8 Introduce a new data type for timestampsJohannes Schindelin, Apr 20, 2017
  51. 8/8 Use uintmax_t for timestampsJohannes Schindelin, Apr 20, 2017
  52. 7/8 Abort if the system time cannot handle one of our timestampsJohannes Schindelin, Apr 20, 2017
  53. Junio C HamanoApr 21, 2017
  54. Johannes SchindelinApr 21, 2017
  55. 0/9 Introduce timestamp_t for timestampsJohannes Schindelin, Apr 21, 2017
  56. 1/9 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Apr 21, 2017
  57. 2/9 t0006 & t5000: prepare for 64-bit timestampsJohannes Schindelin, Apr 21, 2017
  58. 3/9 t0006 & t5000: skip "far in the future" test when time_t is too limitedJohannes Schindelin, Apr 21, 2017
  59. 4/9 Specify explicitly where we parse timestampsJohannes Schindelin, Apr 21, 2017
  60. Junio C HamanoApr 24, 2017
  61. 5/9 Introduce a new "printf format" for timestampsJohannes Schindelin, Apr 21, 2017
  62. 6/9 Introduce a new data type for timestampsJohannes Schindelin, Apr 21, 2017
  63. 7/9 Abort if the system time cannot handle one of our timestampsJohannes Schindelin, Apr 21, 2017
  64. Junio C HamanoApr 24, 2017
  65. Johannes SchindelinApr 24, 2017
  66. Junio C HamanoApr 25, 2017
  67. Junio C HamanoApr 25, 2017
  68. 8/9 Use uintmax_t for timestampsJohannes Schindelin, Apr 21, 2017
  69. Junio C HamanoApr 24, 2017
  70. Johannes SchindelinApr 24, 2017
  71. Junio C HamanoApr 25, 2017
  72. Johannes SchindelinApr 25, 2017
  73. Junio C HamanoApr 26, 2017
  74. Junio C HamanoApr 26, 2017
  75. Johannes SchindelinApr 26, 2017
  76. Junio C HamanoApr 26, 2017
  77. 9/9 show_date_ident(): defer date overflow checkJohannes Schindelin, Apr 21, 2017
  78. Junio C HamanoApr 24, 2017
  79. Jacob KellerApr 24, 2017
  80. Johannes SchindelinApr 24, 2017
  81. Jeff KingApr 24, 2017
  82. Johannes SchindelinApr 25, 2017
  83. Johannes SchindelinApr 24, 2017
  84. 0/8 Introduce timestamp_t for timestampsJohannes Schindelin, Apr 24, 2017
  85. 1/8 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Apr 24, 2017
  86. 2/8 t0006 & t5000: prepare for 64-bit timestampsJohannes Schindelin, Apr 24, 2017
  87. 3/8 t0006 & t5000: skip "far in the future" test when time_t is too limitedJohannes Schindelin, Apr 24, 2017
  88. 5/8 Introduce a new "printf format" for timestampsJohannes Schindelin, Apr 24, 2017
  89. 4/8 Specify explicitly where we parse timestampsJohannes Schindelin, Apr 24, 2017
  90. Junio C HamanoApr 25, 2017
  91. 6/8 Introduce a new data type for timestampsJohannes Schindelin, Apr 24, 2017
  92. Johannes SixtApr 26, 2017
  93. Johannes SchindelinApr 26, 2017
  94. René ScharfeApr 26, 2017
  95. 7/8 Abort if the system time cannot handle one of our timestampsJohannes Schindelin, Apr 24, 2017
  96. 8/8 Use uintmax_t for timestampsJohannes Schindelin, Apr 24, 2017
  97. Johannes SixtApr 26, 2017
  98. Johannes SchindelinApr 26, 2017
  99. René ScharfeApr 25, 2017
  100. Johannes SchindelinApr 25, 2017
  101. René ScharfeApr 26, 2017
  102. Junio C HamanoApr 26, 2017
  103. 0/8 Introduce timestamp_t for timestampsJohannes Schindelin, Apr 26, 2017
  104. 1/8 ref-filter: avoid using `unsigned long` for catch-all data typeJohannes Schindelin, Apr 26, 2017
  105. 2/8 t0006 & t5000: prepare for 64-bit timestampsJohannes Schindelin, Apr 26, 2017
  106. 3/8 t0006 & t5000: skip "far in the future" test when time_t is too limitedJohannes Schindelin, Apr 26, 2017
  107. 4/8 Specify explicitly where we parse timestampsJohannes Schindelin, Apr 26, 2017
  108. 5/8 Introduce a new "printf format" for timestampsJohannes Schindelin, Apr 26, 2017
  109. 6/8 Introduce a new data type for timestampsJohannes Schindelin, Apr 26, 2017
  110. name-rev: change a "long" variable to timestamp_tJunio C Hamano, May 20, 2017
  111. Johannes SchindelinMay 22, 2017
  112. 7/8 Abort if the system time cannot handle one of our timestampsJohannes Schindelin, Apr 26, 2017
  113. 8/8 Use uintmax_t for timestampsJohannes Schindelin, Apr 26, 2017

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.