git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Jeff King <peff@peff.net>, Andreas Schwab <schwab@linux-m68k.org>,
	git@vger.kernel.org
Subject: Re: [ANNOUNCE] Git v2.9.1
Date: Wed, 13 Jul 2016 12:07:34 -0700	[thread overview]
Message-ID: <xmqq8tx51hmx.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1607132048370.6426@virtualbox> (Johannes Schindelin's message of "Wed, 13 Jul 2016 20:52:14 +0200 (CEST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> How about this one instead (which is part of the time_t-may-be-int64
> branch on https://github.com/dscho/git which I still have to complete, as
> some unsigned longs still slipped out of my previous net)? It strikes me
> as much more robust:

Hmm, sorry, I do not see much upside here (iow, the 2038 test you
came up with is as robust).  When the internal time representation
is updated from "unsigned long" to a signed and wider type [*1*],
test-date has to stop reading the second-from-epoch input with
strtol(), whose property that overflow will always result in
LONG_MAX gives the robustness of the 2038 test, and needs to be
updated.  With this "is64bit" patch, you explicitly measure
"unsigned long", knowing that our internal time representation
currently is that type, and that would need to be updated when
widening happens.  So both need to be updated anyway in the future.

The update to check_show Peff suggested is the same as the previous
one, so there is no upside nor downside.

The prerequisite name 64BITTIME that lost an underscore is harder to
read, so there is a slight downside.

Moving of lazy_prereq to test-lib might be an upside if we were
planning to add a test that depends on the system having or not
having 64-bit timestamp elsewhere, but I do not think of a reason
why such a new test cannot go to t0006-date, which has the perfect
name for such a test and is not overly long right now (114 lines).

So, unless you have a more solid reason to reject the updated t0006
earlier in the thread, I am not sure what we'd gain by replacing it
with this version.


> -- snipsnap --
> From abe59dbb2235bb1d7aad8e78a66e196acb372ec8 Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Tue, 12 Jul 2016 13:19:53 +0200
> Subject: [PATCH] t0006: dates absurdly far require a 64-bit data type
>
> Git's source code refers to timestamps as unsigned longs. On 32-bit
> platforms, as well as on Windows, unsigned long is not large enough to
> capture dates that are "absurdly far in the future".
>
> Let's skip those tests if we know they cannot succeed.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  t/helper/test-date.c | 5 ++++-
>  t/t0006-date.sh      | 6 +++---
>  t/test-lib.sh        | 2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/t/helper/test-date.c b/t/helper/test-date.c
> index d9ab360..1e12d93 100644
> --- a/t/helper/test-date.c
> +++ b/t/helper/test-date.c
> @@ -4,7 +4,8 @@ static const char *usage_msg = "\n"
>  "  test-date relative [time_t]...\n"
>  "  test-date show:<format> [time_t]...\n"
>  "  test-date parse [date]...\n"
> -"  test-date approxidate [date]...\n";
> +"  test-date approxidate [date]...\n"
> +"  test-date is64bit\n";
>  
>  static void show_relative_dates(char **argv, struct timeval *now)
>  {
> @@ -93,6 +94,8 @@ int main(int argc, char **argv)
>  		parse_dates(argv+1, &now);
>  	else if (!strcmp(*argv, "approxidate"))
>  		parse_approxidate(argv+1, &now);
> +	else if (!strcmp(*argv, "is64bit"))
> +		return sizeof(unsigned long) == 8 ? 0 : 1;
>  	else
>  		usage(usage_msg);
>  	return 0;
> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 04ce535..52f6b62 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -31,7 +31,7 @@ check_show () {
>  	format=$1
>  	time=$2
>  	expect=$3
> -	test_expect_${4:-success} "show date ($format:$time)" '
> +	test_expect_success $4 "show date ($format:$time)" '
>  		echo "$time -> $expect" >expect &&
>  		test-date show:$format "$time" >actual &&
>  		test_cmp expect actual
> @@ -50,8 +50,8 @@ check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
>  
>  # arbitrary time absurdly far in the future
>  FUTURE="5758122296 -0400"
> -check_show iso       "$FUTURE" "2152-06-19 18:24:56 -0400"
> -check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000"
> +check_show iso       "$FUTURE" "2152-06-19 18:24:56 -0400" 64BITTIME
> +check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000" 64BITTIME
>  
>  check_parse() {
>  	echo "$1 -> $2" >expect
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 0055ebb..4e1afb0 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1111,3 +1111,5 @@ run_with_limited_cmdline () {
>  }
>  
>  test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
> +
> +test_lazy_prereq 64BITTIME 'test-date is64bit'

  reply	other threads:[~2016-07-13 19:08 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-11 20:13 [ANNOUNCE] Git v2.9.1 Junio C Hamano
2016-07-11 21:35 ` Andreas Schwab
2016-07-11 23:54   ` Jeff King
2016-07-12  0:40     ` Anders Kaseorg
2016-07-12 14:06       ` Jeff King
2016-07-12  0:56     ` Eric Wong
2016-07-12  1:15       ` Jeff King
2016-07-12  1:59     ` Junio C Hamano
2016-07-12  3:57       ` Jeff King
2016-07-12 15:55         ` Junio C Hamano
2016-07-12  7:30       ` Johannes Schindelin
2016-07-12  7:39         ` Jeff King
2016-07-12 11:25           ` Johannes Schindelin
2016-07-12 14:04             ` Jeff King
2016-07-13 11:35               ` Johannes Schindelin
2016-07-13 16:03                 ` Junio C Hamano
2016-07-13 19:10                   ` Johannes Schindelin
2016-07-13 19:41                     ` Junio C Hamano
2016-07-14  7:50                       ` Johannes Schindelin
2016-07-12 18:12             ` Junio C Hamano
2016-07-13  1:53               ` Junio C Hamano
2016-07-13  2:01               ` Jeff King
2016-07-13 16:05                 ` Junio C Hamano
2016-07-13 18:52                   ` Johannes Schindelin
2016-07-13 19:07                     ` Junio C Hamano [this message]
2016-07-14  7:45                       ` Johannes Schindelin
2016-07-14  8:01                         ` Andreas Schwab
2016-07-14  8:15                         ` Jeff King
2016-07-14 16:06                       ` Johannes Schindelin
2016-07-12  7:40         ` Andreas Schwab
2016-07-12 10:57           ` Johannes Schindelin
2016-07-12 13:00             ` Andreas Schwab
2016-07-12 13:22               ` Johannes Schindelin
2016-07-12 13:31                 ` Andreas Schwab
2016-07-12 13:46                   ` Jeff King
2016-07-12 18:38                     ` Duy Nguyen
2016-07-13 11:29                       ` Johannes Schindelin
2016-07-13 11:25                   ` Johannes Schindelin
2016-07-12 14:34         ` Junio C Hamano
2016-07-12 15:16           ` Jeff King
2016-07-12 15:25             ` Junio C Hamano
2016-07-12 15:35               ` Jeff King
2016-07-12 15:41                 ` Junio C Hamano
2016-07-12 16:09                   ` Jeff King
2016-07-12 16:25                     ` Junio C Hamano
2016-07-13 14:00                       ` Johannes Schindelin
2016-07-13 16:10                         ` Junio C Hamano
2016-07-13 18:53                           ` Johannes Schindelin
2016-07-12 18:15                     ` Andreas Schwab
2016-07-13 20:43       ` Junio C Hamano
2016-07-14  7:38         ` Lars Schneider
2016-07-16  5:50           ` Duy Nguyen
2016-07-14  7:58         ` 32-bit Travis, was " Johannes Schindelin
2016-07-14  9:12           ` Mike Hommey
2016-07-14 10:58             ` Johannes Schindelin
2016-07-15  1:59               ` Mike Hommey

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=xmqq8tx51hmx.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=schwab@linux-m68k.org \
    /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).