[ANNOUNCE] Git v2.9.1

[ANNOUNCE] Git v2.9.1

[Junio C Hamano]

The latest maintenance release Git v2.9.1 is now available at the
usual places.  This release includes fixes to two bugs that have
been reported on the list recently, among other changes:

 - v2.9.0 changed cloning of submodules in a top-level superproject
   that was cloned shallowly to explicitly ask for the exact commit
   bound at the superproject, which many instances of servers are
   not yet prepared to handle (the feature appeared in a few
   releases ago but the server operators may not have enabled it
   yet).

 - "git bisect" at the end tries to give something similar to an
   output from "git show --raw" for the found culprit, but the step
   was broken due to an internal API change.

The tarballs are found at:

    https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.9.1'
tag and the 'maint' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

----------------------------------------------------------------

Git v2.9.1 Release Notes
========================

Fixes since v2.9
----------------

 * When "git daemon" is run without --[init-]timeout specified, a
   connection from a client that silently goes offline can hang around
   for a long time, wasting resources.  The socket-level KEEPALIVE has
   been enabled to allow the OS to notice such failed connections.

 * The commands in `git log` family take %C(auto) in a custom format
   string.  This unconditionally turned the color on, ignoring
   --no-color or with --color=auto when the output is not connected to
   a tty; this was corrected to make the format truly behave as
   "auto".

 * "git rev-list --count" whose walk-length is limited with "-n"
   option did not work well with the counting optimized to look at the
   bitmap index.

 * "git show -W" (extend hunks to cover the entire function, delimited
   by lines that match the "funcname" pattern) used to show the entire
   file when a change added an entire function at the end of the file,
   which has been fixed.

 * The documentation set has been updated so that literal commands,
   configuration variables and environment variables are consistently
   typeset in fixed-width font and bold in manpages.

 * "git svn propset" subcommand that was added in 2.3 days is
   documented now.

 * The documentation tries to consistently spell "GPG"; when
   referring to the specific program name, "gpg" is used.

 * "git reflog" stopped upon seeing an entry that denotes a branch
   creation event (aka "unborn"), which made it appear as if the
   reflog was truncated.

 * The git-prompt scriptlet (in contrib/) was not friendly with those
   who uses "set -u", which has been fixed.

 * A codepath that used alloca(3) to place an unbounded amount of data
   on the stack has been updated to avoid doing so.

 * "git update-index --add --chmod=+x file" may be usable as an escape
   hatch, but not a friendly thing to force for people who do need to
   use it regularly.  "git add --chmod=+x file" can be used instead.

 * Build improvements for gnome-keyring (in contrib/)

 * "git status" used to say "working directory" when it meant "working
   tree".

 * Comments about misbehaving FreeBSD shells have been clarified with
   the version number (9.x and before are broken, newer ones are OK).

 * "git cherry-pick A" worked on an unborn branch, but "git
   cherry-pick A..B" didn't.

 * "git add -i/-p" learned to honor diff.compactionHeuristic
   experimental knob, so that the user can work on the same hunk split
   as "git diff" output.

 * "log --graph --format=" learned that "%>|(N)" specifies the width
   relative to the terminal's left edge, not relative to the area to
   draw text that is to the right of the ancestry-graph section.  It
   also now accepts negative N that means the column limit is relative
   to the right border.

 * The ownership rule for the piece of memory that hold references to
   be fetched in "git fetch" was screwy, which has been cleaned up.

 * "git bisect" makes an internal call to "git diff-tree" when
   bisection finds the culprit, but this call did not initialize the
   data structure to pass to the diff-tree API correctly.

 * Formats of the various data (and how to validate them) where we use
   GPG signature have been documented.

 * Fix an unintended regression in v2.9 that breaks "clone --depth"
   that recurses down to submodules by forcing the submodules to also
   be cloned shallowly, which many server instances that host upstream
   of the submodules are not prepared for.

 * Fix unnecessarily waste in the idiomatic use of ': ${VAR=default}'
   to set the default value, without enclosing it in double quotes.

 * Some platform-specific code had non-ANSI strict declarations of C
   functions that do not take any parameters, which has been
   corrected.

 * The internal code used to show local timezone offset is not
   prepared to handle timestamps beyond year 2100, and gave a
   bogus offset value to the caller.  Use a more benign looking
   +0000 instead and let "git log" going in such a case, instead
   of aborting.

 * One among four invocations of readlink(1) in our test suite has
   been rewritten so that the test can run on systems without the
   command (others are in valgrind test framework and t9802).

 * t/perf needs /usr/bin/time with GNU extension; the invocation of it
   is updated to "gtime" on Darwin.

 * A bug, which caused "git p4" while running under verbose mode to
   report paths that are omitted due to branch prefix incorrectly, has
   been fixed; the command said "Ignoring file outside of prefix" for
   paths that are _inside_.

 * The top level documentation "git help git" still pointed at the
   documentation set hosted at now-defunct google-code repository.
   Update it to point to https://git.github.io/htmldocs/git.html
   instead.

Also contains minor documentation updates and code clean-ups.

----------------------------------------------------------------

Changes since v2.9.0 are as follows:

Alfred Perlstein (1):
      git-svn: document the 'git svn propset' command

Andrew Oakley (1):
      git-p4: correct hasBranchPrefix verbose output

Armin Kunaschik (1):
      t7800: readlink may not be available

Charles Bailey (1):
      t7810: fix duplicated test title

Dave Nicolson (1):
      Documentation: GPG capitalization

David Turner (1):
      mailmap: use main email address for dturner

Ed Maste (1):
      rebase: update comment about FreeBSD /bin/sh

Edward Thomson (2):
      format_commit_message: honor `color=auto` for `%C(auto)`
      add: add --chmod=+x / --chmod=-x options

Eric Wong (1):
      daemon: enable SO_KEEPALIVE for all sockets

Heiko Becker (1):
      gnome-keyring: Don't hard-code pkg-config executable

Jeff King (9):
      rev-list: "adjust" results of "--count --use-bitmap-index -n"
      rev-list: disable bitmaps when "-n" is used with listing objects
      tree-diff: avoid alloca for large allocations
      fetch: document that pruning happens before fetching
      add--interactive: respect diff.compactionHeuristic
      bisect: always call setup_revisions after init_revisions
      t0006: rename test-date's "show" to "relative"
      t0006: test various date formats
      local_tzoffset: detect errors from tm_to_time_t

Johannes Schindelin (3):
      mingw: let the build succeed with DEVELOPER=1
      perf: accommodate for MacOSX
      t2300: "git --exec-path" is not usable in $PATH on Windows as-is

Jonathan Nieder (1):
      doc: git-htmldocs.googlecode.com is no more

Josef Kufner (1):
      pretty: pass graph width to pretty formatting for use in '%>|(N)'

Junio C Hamano (5):
      blame, line-log: do not loop around deref_tag()
      clone: do not let --depth imply --shallow-submodules
      Start preparing for 2.9.1
      More fixes for 2.9.1
      Git 2.9.1

Keith McGuigan (1):
      builtin/fetch.c: don't free remote->name after fetch

LE Manh Cuong (1):
      sh-setup: enclose setting of ${VAR=default} in double-quotes

Lars Vogel (1):
      Use "working tree" instead of "working directory" for git status

Michael J Gruber (5):
      cherry-pick: allow to pick to unborn branches
      Documentation/technical: describe signature formats
      Documentation/technical: signed tag format
      Documentation/technical: signed commit format
      Documentation/technical: signed merge tag format

Nguyễn Thái Ngọc Duy (1):
      pretty.c: support <direction>|(<negative number>) forms

Peter Colberg (2):
      refs.h: fix misspelt "occurred" in a comment
      config.c: fix misspelt "occurred" in an error message

Pranit Bauva (1):
      strbuf: describe the return value of strbuf_read_file

Ramsay Jones (1):
      regex: fix a SIZE_MAX macro redefinition warning

René Scharfe (9):
      t4051: rewrite, add more tests
      xdiff: factor out match_func_rec()
      xdiff: handle appended chunks better with -W
      xdiff: ignore empty lines before added functions with -W
      xdiff: -W: don't include common trailing empty lines in context
      xdiff: don't trim common tail with -W
      t7810: add test for grep -W and trailing empty context lines
      grep: -W: don't extend context to trailing empty lines
      xdiff: fix merging of appended hunk with -W

SZEDER Gábor (1):
      reflog: continue walking the reflog past root commits

Stefan Beller (1):
      t5614: don't use subshells

Tom Russello (4):
      doc: clearer rule about formatting literals
      doc: change environment variables format
      doc: more consistency in environment variables format
      doc: change configuration variables format

Ville Skyttä (1):
      git-prompt.sh: Don't error on null ${ZSH,BASH}_VERSION, $short_sha

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Junio C Hamano <gitster@pobox.com> writes:

>       local_tzoffset: detect errors from tm_to_time_t

not ok 19 - show date (iso:5758122296 -0400)
#      
#                      echo "$time -> $expect" >expect &&
#                      test-date show:$format "$time" >actual &&
#                      test_cmp expect actual
#              
not ok 20 - show date (iso-local:5758122296 -0400)
#      
#                      echo "$time -> $expect" >expect &&
#                      test-date show:$format "$time" >actual &&
#                      test_cmp expect actual

This is outside the range of 32bit time_t.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Mon, Jul 11, 2016 at 11:35:05PM +0200, Andreas Schwab wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> >       local_tzoffset: detect errors from tm_to_time_t
> 
> not ok 19 - show date (iso:5758122296 -0400)
> #      
> #                      echo "$time -> $expect" >expect &&
> #                      test-date show:$format "$time" >actual &&
> #                      test_cmp expect actual
> #              
> not ok 20 - show date (iso-local:5758122296 -0400)
> #      
> #                      echo "$time -> $expect" >expect &&
> #                      test-date show:$format "$time" >actual &&
> #                      test_cmp expect actual
> 
> This is outside the range of 32bit time_t.

Yes, that's somewhat the point of the test.

How does it fail for you (what does it look like with "-v")? We may be
able to check for an outcome that matches both cases.

Otherwise, we'll have to skip the test, perhaps with something like the
patch below. I suspect the problem is actually the size of "unsigned
long", not time_t, as we use that internally for a bunch of time
computation.

---
diff --git a/help.c b/help.c
index 19328ea..0cea240 100644
--- a/help.c
+++ b/help.c
@@ -419,6 +419,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 	 * with external projects that rely on the output of "git version".
 	 */
 	printf("git version %s\n", git_version_string);
+	while (*++argv) {
+		if (!strcmp(*argv, "--build-options")) {
+			printf("sizeof-unsigned-long: %d",
+			       (int)sizeof(unsigned long));
+			/* maybe also save and output GIT-BUILD_OPTIONS? */
+		}
+	}
 	return 0;
 }
 
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 04ce535..a0b8497 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" 64BIT
+check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000" 64BIT
 
 check_parse() {
 	echo "$1 -> $2" >expect
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0055ebb..d592bdc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1111,3 +1111,12 @@ run_with_limited_cmdline () {
 }
 
 test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
+
+build_option () {
+	git version --build-options |
+	sed -ne "s/^$1: //p"
+}
+
+test_lazy_prereq 64BIT '
+	test 8 -le "$(build_option sizeof-unsigned-long)"
+'

Re: [ANNOUNCE] Git v2.9.1

[Anders Kaseorg]

On 07/11/2016 07:54 PM, Jeff King wrote:
> Yes, that's somewhat the point of the test.
>
> How does it fail for you (what does it look like with "-v")? We may be
> able to check for an outcome that matches both cases.

On Ubuntu i386 and Ubuntu armhf, I get the following verbose output from 
t0006-date.sh:


expecting success:
		echo "$time -> $expect" >expect &&
		test-date show:$format "$time" >actual &&
		test_cmp expect actual
	
--- expect	2016-07-11 23:20:55.835136188 +0000
+++ actual	2016-07-11 23:20:55.835136188 +0000
@@ -1 +1 @@
-5758122296 -0400 -> 2152-06-19 18:24:56 -0400
+5758122296 -0400 -> 2038-01-18 23:14:07 -0400
not ok 19 - show date (iso:5758122296 -0400)
#	
#			echo "$time -> $expect" >expect &&
#			test-date show:$format "$time" >actual &&
#			test_cmp expect actual
#		

expecting success:
		echo "$time -> $expect" >expect &&
		test-date show:$format "$time" >actual &&
		test_cmp expect actual
	
--- expect	2016-07-11 23:20:55.839136188 +0000
+++ actual	2016-07-11 23:20:55.839136188 +0000
@@ -1 +1 @@
-5758122296 -0400 -> 2152-06-19 22:24:56 +0000
+5758122296 -0400 -> 2038-01-19 03:14:07 +0000
not ok 20 - show date (iso-local:5758122296 -0400)


Anders

Re: [ANNOUNCE] Git v2.9.1

[Eric Wong]

Jeff King <peff@peff.net> wrote:
> Otherwise, we'll have to skip the test, perhaps with something like the
> patch below. I suspect the problem is actually the size of "unsigned
> long", not time_t, as we use that internally for a bunch of time
> computation.

We should probably be using int64_t for time calculations;
"unsigned long" is 32-bits on 32-bit x86 systems.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 12:56:11AM +0000, Eric Wong wrote:

> Jeff King <peff@peff.net> wrote:
> > Otherwise, we'll have to skip the test, perhaps with something like the
> > patch below. I suspect the problem is actually the size of "unsigned
> > long", not time_t, as we use that internally for a bunch of time
> > computation.
> 
> We should probably be using int64_t for time calculations;
> "unsigned long" is 32-bits on 32-bit x86 systems.

I'd agree (or probably just "time_t", which I would hope would be a
future-proof length even on 32-bit systems).

It's just that it's baked into a lot of different function interfaces
and structs in the code, and nobody has taken the time to review and
change it all (since it doesn't actually affect current timestamps and
won't for a while).

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Jul 11, 2016 at 11:35:05PM +0200, Andreas Schwab wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> >       local_tzoffset: detect errors from tm_to_time_t
>> 
>> not ok 19 - show date (iso:5758122296 -0400)
>> #      
>> #                      echo "$time -> $expect" >expect &&
>> #                      test-date show:$format "$time" >actual &&
>> #                      test_cmp expect actual
>> #              
>> not ok 20 - show date (iso-local:5758122296 -0400)
>> #      
>> #                      echo "$time -> $expect" >expect &&
>> #                      test-date show:$format "$time" >actual &&
>> #                      test_cmp expect actual
>> 
>> This is outside the range of 32bit time_t.
>
> Yes, that's somewhat the point of the test.
>
> How does it fail for you (what does it look like with "-v")? We may be
> able to check for an outcome that matches both cases.
>
> Otherwise, we'll have to skip the test, perhaps with something like the
> patch below. I suspect the problem is actually the size of "unsigned
> long", not time_t, as we use that internally for a bunch of time
> computation.
>
> ---
> diff --git a/help.c b/help.c
> index 19328ea..0cea240 100644
> --- a/help.c
> +++ b/help.c
> @@ -419,6 +419,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)
>  	 * with external projects that rely on the output of "git version".
>  	 */
>  	printf("git version %s\n", git_version_string);
> +	while (*++argv) {
> +		if (!strcmp(*argv, "--build-options")) {
> +			printf("sizeof-unsigned-long: %d",
> +			       (int)sizeof(unsigned long));
> +			/* maybe also save and output GIT-BUILD_OPTIONS? */
> +		}
> +	}
>  	return 0;
>  }

I had the same thought, except that I would have expected this to go
to one of these test-* helpers, and then a lazy prereq for 64-bit
time_t would be written on top of it to skip these new tests.

> +build_option () {
> +	git version --build-options |
> +	sed -ne "s/^$1: //p"
> +}
> +
> +test_lazy_prereq 64BIT '
> +	test 8 -le "$(build_option sizeof-unsigned-long)"
> +'

It is somewhat disturbing that nobody seems to be regularly building
on 32-bit platforms these days, which is the only reason I can think
of why this was never reported until it hit a maintenance track.
This should have been caught last week at f6a729f3 (Merge branch
'jk/tzoffset-fix', 2016-07-06) when the topic hit 'master' at the
latest, and more preferrably it should have already been caught last
month at 08ec8c5e (Merge branch 'jk/tzoffset-fix' into next,
2016-06-28).

Those who care about 32-bit builds need to start building and
testing 'next' and 'master' regularly, or similar breakages are
bound to continue happening X-<.

Volunteers?

Thanks.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Mon, Jul 11, 2016 at 06:59:51PM -0700, Junio C Hamano wrote:

> > diff --git a/help.c b/help.c
> > index 19328ea..0cea240 100644
> > --- a/help.c
> > +++ b/help.c
> > @@ -419,6 +419,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)
> >  	 * with external projects that rely on the output of "git version".
> >  	 */
> >  	printf("git version %s\n", git_version_string);
> > +	while (*++argv) {
> > +		if (!strcmp(*argv, "--build-options")) {
> > +			printf("sizeof-unsigned-long: %d",
> > +			       (int)sizeof(unsigned long));
> > +			/* maybe also save and output GIT-BUILD_OPTIONS? */
> > +		}
> > +	}
> >  	return 0;
> >  }
> 
> I had the same thought, except that I would have expected this to go
> to one of these test-* helpers, and then a lazy prereq for 64-bit
> time_t would be written on top of it to skip these new tests.

Yeah, that would certainly work.

However, I was thinking that it might be handy to have this and some
other information available for helping with debugging. E.g., that we
could ask bug reporters for "git version --build-options" when we
suspect something related to their config.

Something along the lines of the patch below, which lets you do:

  $ ./git version --build-options
  git version 2.9.0.243.g5c589a7.dirty
  sizeof-unsigned-long: 8
  SHELL_PATH: /bin/sh
  PERL_PATH: /usr/bin/perl
  DIFF: diff
  PYTHON_PATH: /usr/bin/python
  TAR: tar
  NO_CURL: 
  NO_EXPAT: 
  USE_LIBPCRE: YesPlease
  NO_PERL: 
  NO_PYTHON: 
  NO_UNIX_SOCKETS: 
  GIT_TEST_OPTS: --root=/var/ram/git-tests
  NO_GETTEXT: Nope
  GETTEXT_POISON: 

That's not all of the knobs, though; it's just what we stick in
BUILD-OPTIONS to trigger script rebuilds and communicate with the test
scripts. So I think there would potentially be further work to do. But
it's at least a start. I dunno.

> It is somewhat disturbing that nobody seems to be regularly building
> on 32-bit platforms these days, which is the only reason I can think
> of why this was never reported until it hit a maintenance track.
> This should have been caught last week at f6a729f3 (Merge branch
> 'jk/tzoffset-fix', 2016-07-06) when the topic hit 'master' at the
> latest, and more preferrably it should have already been caught last
> month at 08ec8c5e (Merge branch 'jk/tzoffset-fix' into next,
> 2016-06-28).

I dream of a world where there are no 32-bit platforms at all, but sadly
we are stuck in the middle ground where they are rare enough that nobody
bothers to test on them early, but not rare enough that somebody doesn't
complain within 24 hours of making a release. :-/

-Peff

---
diff --git a/Makefile b/Makefile
index de5a030..78d96a0 100644
--- a/Makefile
+++ b/Makefile
@@ -669,6 +669,7 @@ XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
 
 GENERATED_H += common-cmds.h
+GENERATED_H += build-options.h
 
 LIB_H = $(shell $(FIND) . \
 	-name .git -prune -o \
@@ -1711,7 +1712,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \
 		$(BUILTIN_OBJS) $(LIBS)
 
-help.sp help.s help.o: common-cmds.h
+help.sp help.s help.o: common-cmds.h build-options.h
 
 builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
@@ -1735,6 +1736,9 @@ common-cmds.h: generate-cmdlist.sh command-list.txt
 common-cmds.h: $(wildcard Documentation/git-*.txt)
 	$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh command-list.txt >$@+ && mv $@+ $@
 
+build-options.h: generate-build-options.sh GIT-BUILD-OPTIONS
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-build-options.sh GIT-BUILD-OPTIONS >$@+ && mv $@+ $@
+
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
 	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
 	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP)
diff --git a/generate-build-options.sh b/generate-build-options.sh
new file mode 100644
index 0000000..250728f
--- /dev/null
+++ b/generate-build-options.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+c_quote () {
+	printf '%s' "$1" |
+	sed -e 's/\\/\\\\/g' -e 's/"/\\"/g'
+}
+
+echo '#ifndef BUILD_OPTIONS_H'
+echo '#define BUILD_OPTIONS_H'
+echo "/* Automatically generated by $0 */"
+echo
+echo '#define BUILD_OPTIONS \'
+
+for source in "$@"; do
+	# source/eval trickery is there to unquote the values
+	. "./$source"
+	for var in $(cut -d= -f1 "$source"); do
+		printf '"%s: ' "$var"
+		eval "c_quote \"\$$var\""
+		printf '\\n" \\\n'
+	done
+done
+
+echo
+echo '#endif /* BUILD_OPTIONS_H */'
diff --git a/help.c b/help.c
index 19328ea..1cbee86 100644
--- a/help.c
+++ b/help.c
@@ -8,6 +8,7 @@
 #include "column.h"
 #include "version.h"
 #include "refs.h"
+#include "build-options.h"
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
@@ -419,6 +420,13 @@ int cmd_version(int argc, const char **argv, const char *prefix)
 	 * with external projects that rely on the output of "git version".
 	 */
 	printf("git version %s\n", git_version_string);
+	while (*++argv) {
+		if (!strcmp(*argv, "--build-options")) {
+			printf("sizeof-unsigned-long: %d\n",
+			       (int)sizeof(unsigned long));
+			printf("%s", BUILD_OPTIONS);
+		}
+	}
 	return 0;
 }
 

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio & Peff,

On Mon, 11 Jul 2016, Junio C Hamano wrote:

> Those who care about 32-bit builds need to start building and
> testing 'next' and 'master' regularly, or similar breakages are
> bound to continue happening X-<.

Please note that "unsigned long" is 32-bit even on 64-bit Windows.

FWIW I have this monster patch as a starting point (I plan to work more on
this today):

-- snipsnap --
From b65de5a56e18b038d432ba828d7714880b8e285c Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Tue, 12 Jul 2016 09:09:07 +0200
Subject: [PATCH] HOTFIX: use time_t wherever appropriate

Git's source code assumes that unsigned long is at least as precise as
time_t. Well, Git's source code is wrong.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/technical/api-parse-options.txt |  4 +-
 builtin/fsck.c                                |  2 +-
 builtin/merge-base.c                          |  2 +-
 builtin/prune.c                               |  2 +-
 builtin/reflog.c                              | 24 +++----
 builtin/show-branch.c                         |  4 +-
 builtin/worktree.c                            |  2 +-
 cache.h                                       | 14 ++---
 date.c                                        | 90 +++++++++++++++------------
 parse-options-cb.c                            |  4 +-
 reflog-walk.c                                 |  8 +--
 refs.c                                        | 14 ++---
 refs.h                                        |  8 +--
 refs/files-backend.c                          |  4 +-
 revision.c                                    |  6 +-
 sha1_name.c                                   |  6 +-
 t/helper/test-date.c                          |  2 +-
 t/helper/test-parse-options.c                 |  4 +-
 vcs-svn/svndump.c                             |  2 +-
 wt-status.c                                   |  2 +-
 20 files changed, 106 insertions(+), 98 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 27bd701..6801aad 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -178,11 +178,11 @@ There are some macros to easily define options:
 	scale the provided value by 1024, 1024^2 or 1024^3 respectively.
 	The scaled value is put into `unsigned_long_var`.
 
-`OPT_DATE(short, long, &int_var, description)`::
+`OPT_DATE(short, long, &time_t_var, description)`::
 	Introduce an option with date argument, see `approxidate()`.
 	The timestamp is put into `int_var`.
 
-`OPT_EXPIRY_DATE(short, long, &int_var, description)`::
+`OPT_EXPIRY_DATE(short, long, &time_t_var, description)`::
 	Introduce an option with expiry date argument, see `parse_expiry_date()`.
 	The timestamp is put into `int_var`.
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 3f27456..3dfa32b 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -385,7 +385,7 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1)
 }
 
 static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-		const char *email, unsigned long timestamp, int tz,
+		const char *email, time_t timestamp, int tz,
 		const char *message, void *cb_data)
 {
 	const char *refname = cb_data;
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index c0d1822..85ba332 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -132,7 +132,7 @@ static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
 }
 
 static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-				  const char *ident, unsigned long timestamp,
+				  const char *ident, time_t timestamp,
 				  int tz, const char *message, void *cbdata)
 {
 	struct rev_collect *revs = cbdata;
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f052..5219504 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -13,7 +13,7 @@ static const char * const prune_usage[] = {
 };
 static int show_only;
 static int verbose;
-static unsigned long expire;
+static time_t expire;
 static int show_progress = -1;
 
 static int prune_tmp_file(const char *fullpath)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e..331c874 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -16,14 +16,14 @@ static const char reflog_delete_usage[] =
 static const char reflog_exists_usage[] =
 "git reflog exists <ref>";
 
-static unsigned long default_reflog_expire;
-static unsigned long default_reflog_expire_unreachable;
+static time_t default_reflog_expire;
+static time_t default_reflog_expire_unreachable;
 
 struct cmd_reflog_expire_cb {
 	struct rev_info revs;
 	int stalefix;
-	unsigned long expire_total;
-	unsigned long expire_unreachable;
+	time_t expire_total;
+	time_t expire_unreachable;
 	int recno;
 };
 
@@ -219,7 +219,7 @@ static int keep_entry(struct commit **it, unsigned char *sha1)
 static void mark_reachable(struct expire_reflog_policy_cb *cb)
 {
 	struct commit_list *pending;
-	unsigned long expire_limit = cb->mark_limit;
+	time_t expire_limit = cb->mark_limit;
 	struct commit_list *leftover = NULL;
 
 	for (pending = cb->mark_list; pending; pending = pending->next)
@@ -284,7 +284,7 @@ static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit
  * Return true iff the specified reflog entry should be expired.
  */
 static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-				    const char *email, unsigned long timestamp, int tz,
+				    const char *email, time_t timestamp, int tz,
 				    const char *message, void *cb_data)
 {
 	struct expire_reflog_policy_cb *cb = cb_data;
@@ -392,8 +392,8 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus
 
 static struct reflog_expire_cfg {
 	struct reflog_expire_cfg *next;
-	unsigned long expire_total;
-	unsigned long expire_unreachable;
+	time_t expire_total;
+	time_t expire_unreachable;
 	char pattern[FLEX_ARRAY];
 } *reflog_expire_cfg, **reflog_expire_cfg_tail;
 
@@ -415,7 +415,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
 	return ent;
 }
 
-static int parse_expire_cfg_value(const char *var, const char *value, unsigned long *expire)
+static int parse_expire_cfg_value(const char *var, const char *value, time_t *expire)
 {
 	if (!value)
 		return config_error_nonbool(var);
@@ -433,7 +433,7 @@ static int reflog_expire_config(const char *var, const char *value, void *cb)
 {
 	const char *pattern, *key;
 	int pattern_len;
-	unsigned long expire;
+	time_t expire;
 	int slot;
 	struct reflog_expire_cfg *ent;
 
@@ -515,7 +515,7 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c
 static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 {
 	struct expire_reflog_policy_cb cb;
-	unsigned long now = time(NULL);
+	time_t now = time(NULL);
 	int i, status, do_all;
 	int explicit_expiry = 0;
 	unsigned int flags = 0;
@@ -616,7 +616,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 }
 
 static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-		const char *email, unsigned long timestamp, int tz,
+		const char *email, time_t timestamp, int tz,
 		const char *message, void *cb_data)
 {
 	struct expire_reflog_policy_cb *cb = cb_data;
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 2566935..56dd9d2 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -742,7 +742,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			base = strtoul(reflog_base, &ep, 10);
 			if (*ep) {
 				/* Ah, that is a date spec... */
-				unsigned long at;
+				time_t at;
 				at = approxidate(reflog_base);
 				read_ref_at(ref, flags, at, -1, oid.hash, NULL,
 					    NULL, NULL, &base);
@@ -753,7 +753,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			char *logmsg;
 			char *nth_desc;
 			const char *msg;
-			unsigned long timestamp;
+			time_t timestamp;
 			int tz;
 
 			if (read_ref_at(ref, flags, 0, base+i, oid.hash, &logmsg,
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 96a2834..d81857e 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -28,7 +28,7 @@ struct add_opts {
 
 static int show_only;
 static int verbose;
-static unsigned long expire;
+static time_t expire;
 
 static int prune_worktree(const char *id, struct strbuf *reason)
 {
diff --git a/cache.h b/cache.h
index 1537c00..b972659 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,18 +1241,18 @@ struct date_mode {
 #define DATE_MODE(t) date_mode_from_type(DATE_##t)
 struct date_mode *date_mode_from_type(enum date_mode_type type);
 
-const char *show_date(unsigned long time, int timezone, const struct date_mode *mode);
-void show_date_relative(unsigned long time, int tz, const struct timeval *now,
+const char *show_date(time_t time, int timezone, const struct date_mode *mode);
+void show_date_relative(time_t time, int tz, const struct timeval *now,
 			struct strbuf *timebuf);
 int parse_date(const char *date, struct strbuf *out);
-int parse_date_basic(const char *date, unsigned long *timestamp, int *offset);
-int parse_expiry_date(const char *date, unsigned long *timestamp);
+int parse_date_basic(const char *date, time_t *timestamp, int *offset);
+int parse_expiry_date(const char *date, time_t *timestamp);
 void datestamp(struct strbuf *out);
 #define approxidate(s) approxidate_careful((s), NULL)
-unsigned long approxidate_careful(const char *, int *);
-unsigned long approxidate_relative(const char *date, const struct timeval *now);
+time_t approxidate_careful(const char *, int *);
+time_t approxidate_relative(const char *date, const struct timeval *now);
 void parse_date_format(const char *format, struct date_mode *mode);
-int date_overflows(unsigned long date);
+int date_overflows(time_t date);
 
 #define IDENT_STRICT	       1
 #define IDENT_NO_DATE	       2
diff --git a/date.c b/date.c
index 4c7aa9b..7584a78 100644
--- a/date.c
+++ b/date.c
@@ -39,7 +39,7 @@ static const char *weekday_names[] = {
 	"Sundays", "Mondays", "Tuesdays", "Wednesdays", "Thursdays", "Fridays", "Saturdays"
 };
 
-static time_t gm_time_t(unsigned long time, int tz)
+static time_t gm_time_t(time_t time, int tz)
 {
 	int minutes;
 
@@ -54,7 +54,7 @@ static time_t gm_time_t(unsigned long time, int tz)
  * thing, which means that tz -0100 is passed in as the integer -100,
  * even though it means "sixty minutes off"
  */
-static struct tm *time_to_tm(unsigned long time, int tz)
+static struct tm *time_to_tm(time_t time, int tz)
 {
 	time_t t = gm_time_t(time, tz);
 	return gmtime(&t);
@@ -64,7 +64,7 @@ static struct tm *time_to_tm(unsigned long time, int tz)
  * What value of "tz" was in effect back then at "time" in the
  * local timezone?
  */
-static int local_tzoffset(unsigned long time)
+static int local_tzoffset(time_t time)
 {
 	time_t t, t_local;
 	struct tm tm;
@@ -88,11 +88,11 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
-void show_date_relative(unsigned long time, int tz,
+void show_date_relative(time_t time, int tz,
 			       const struct timeval *now,
 			       struct strbuf *timebuf)
 {
-	unsigned long diff;
+	time_t diff;
 	if (now->tv_sec < time) {
 		strbuf_addstr(timebuf, _("in the future"));
 		return;
@@ -100,65 +100,65 @@ void show_date_relative(unsigned long time, int tz,
 	diff = now->tv_sec - time;
 	if (diff < 90) {
 		strbuf_addf(timebuf,
-			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
+			 Q_("%" PRIuMAX " second ago", "%" PRIuMAX " seconds ago", diff), diff);
 		return;
 	}
 	/* Turn it into minutes */
 	diff = (diff + 30) / 60;
 	if (diff < 90) {
 		strbuf_addf(timebuf,
-			 Q_("%lu minute ago", "%lu minutes ago", diff), diff);
+			 Q_("%" PRIuMAX " minute ago", "%" PRIuMAX " minutes ago", diff), diff);
 		return;
 	}
 	/* Turn it into hours */
 	diff = (diff + 30) / 60;
 	if (diff < 36) {
 		strbuf_addf(timebuf,
-			 Q_("%lu hour ago", "%lu hours ago", diff), diff);
+			 Q_("%" PRIuMAX " hour ago", "%" PRIuMAX " hours ago", diff), diff);
 		return;
 	}
 	/* We deal with number of days from here on */
 	diff = (diff + 12) / 24;
 	if (diff < 14) {
 		strbuf_addf(timebuf,
-			 Q_("%lu day ago", "%lu days ago", diff), diff);
+			 Q_("%" PRIuMAX " day ago", "%" PRIuMAX " days ago", diff), diff);
 		return;
 	}
 	/* Say weeks for the past 10 weeks or so */
 	if (diff < 70) {
 		strbuf_addf(timebuf,
-			 Q_("%lu week ago", "%lu weeks ago", (diff + 3) / 7),
+			 Q_("%" PRIuMAX " week ago", "%" PRIuMAX " weeks ago", (diff + 3) / 7),
 			 (diff + 3) / 7);
 		return;
 	}
 	/* Say months for the past 12 months or so */
 	if (diff < 365) {
 		strbuf_addf(timebuf,
-			 Q_("%lu month ago", "%lu months ago", (diff + 15) / 30),
+			 Q_("%" PRIuMAX " month ago", "%" PRIuMAX " months ago", (diff + 15) / 30),
 			 (diff + 15) / 30);
 		return;
 	}
 	/* Give years and months for 5 years or so */
 	if (diff < 1825) {
-		unsigned long totalmonths = (diff * 12 * 2 + 365) / (365 * 2);
-		unsigned long years = totalmonths / 12;
-		unsigned long months = totalmonths % 12;
+		time_t totalmonths = (diff * 12 * 2 + 365) / (365 * 2);
+		time_t years = totalmonths / 12;
+		time_t months = totalmonths % 12;
 		if (months) {
 			struct strbuf sb = STRBUF_INIT;
-			strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
+			strbuf_addf(&sb, Q_("%" PRIuMAX " year", "%" PRIuMAX " years", years), years);
 			strbuf_addf(timebuf,
 				 /* TRANSLATORS: "%s" is "<n> years" */
-				 Q_("%s, %lu month ago", "%s, %lu months ago", months),
+				 Q_("%s, %" PRIuMAX " month ago", "%s, %" PRIuMAX " months ago", months),
 				 sb.buf, months);
 			strbuf_release(&sb);
 		} else
 			strbuf_addf(timebuf,
-				 Q_("%lu year ago", "%lu years ago", years), years);
+				 Q_("%" PRIuMAX " year ago", "%" PRIuMAX " years ago", years), years);
 		return;
 	}
 	/* Otherwise, just years. Centuries is probably overkill. */
 	strbuf_addf(timebuf,
-		 Q_("%lu year ago", "%lu years ago", (diff + 183) / 365),
+		 Q_("%" PRIuMAX " year ago", "%" PRIuMAX " years ago", (diff + 183) / 365),
 		 (diff + 183) / 365);
 }
 
@@ -172,7 +172,7 @@ struct date_mode *date_mode_from_type(enum date_mode_type type)
 	return &mode;
 }
 
-const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
+const char *show_date(time_t time, int tz, const struct date_mode *mode)
 {
 	struct tm *tm;
 	static struct strbuf timebuf = STRBUF_INIT;
@@ -182,7 +182,7 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
 
 	if (mode->type == DATE_RAW) {
 		strbuf_reset(&timebuf);
-		strbuf_addf(&timebuf, "%lu %+05d", time, tz);
+		strbuf_addf(&timebuf, "%" PRIuMAX " %+05d", time, tz);
 		return timebuf.buf;
 	}
 
@@ -419,7 +419,7 @@ static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,
 	return 0;
 }
 
-static int match_multi_number(unsigned long num, char c, const char *date,
+static int match_multi_number(time_t num, char c, const char *date,
 			      char *end, struct tm *tm, time_t now)
 {
 	struct tm now_tm;
@@ -495,6 +495,12 @@ static inline int nodate(struct tm *tm)
 		tm->tm_sec) < 0;
 }
 
+#if 0 /* sizeof(time_t) <= sizeof(unsigned long) */
+#define parse_time_t strtoul
+#else
+#define parse_time_t strtoull
+#endif
+
 /*
  * We've seen a digit. Time? Year? Date?
  */
@@ -502,9 +508,9 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
 {
 	int n;
 	char *end;
-	unsigned long num;
+	time_t num;
 
-	num = strtoul(date, &end, 10);
+	num = parse_time_t(date, &end, 10);
 
 	/*
 	 * Seconds since 1970? We trigger on that for any numbers with
@@ -629,7 +635,7 @@ static int match_tz(const char *date, int *offp)
 	return end - date;
 }
 
-static void date_string(unsigned long date, int offset, struct strbuf *buf)
+static void date_string(time_t date, int offset, struct strbuf *buf)
 {
 	int sign = '+';
 
@@ -637,22 +643,22 @@ static void date_string(unsigned long date, int offset, struct strbuf *buf)
 		offset = -offset;
 		sign = '-';
 	}
-	strbuf_addf(buf, "%lu %c%02d%02d", date, sign, offset / 60, offset % 60);
+	strbuf_addf(buf, "%" PRIuMAX " %c%02d%02d", date, sign, offset / 60, offset % 60);
 }
 
 /*
  * Parse a string like "0 +0000" as ancient timestamp near epoch, but
  * only when it appears not as part of any other string.
  */
-static int match_object_header_date(const char *date, unsigned long *timestamp, int *offset)
+static int match_object_header_date(const char *date, time_t *timestamp, int *offset)
 {
 	char *end;
-	unsigned long stamp;
+	time_t stamp;
 	int ofs;
 
 	if (*date < '0' || '9' < *date)
 		return -1;
-	stamp = strtoul(date, &end, 10);
+	stamp = parse_time_t(date, &end, 10);
 	if (*end != ' ' || stamp == ULONG_MAX || (end[1] != '+' && end[1] != '-'))
 		return -1;
 	date = end + 2;
@@ -669,11 +675,11 @@ static int match_object_header_date(const char *date, unsigned long *timestamp,
 
 /* Gr. strptime is crap for this; it doesn't have a way to require RFC2822
    (i.e. English) day/month names, and it doesn't work correctly with %z. */
-int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
+int parse_date_basic(const char *date, time_t *timestamp, int *offset)
 {
 	struct tm tm;
 	int tm_gmt;
-	unsigned long dummy_timestamp;
+	time_t dummy_timestamp;
 	int dummy_offset;
 
 	if (!timestamp)
@@ -741,7 +747,7 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
 	return 0; /* success */
 }
 
-int parse_expiry_date(const char *date, unsigned long *timestamp)
+int parse_expiry_date(const char *date, time_t *timestamp)
 {
 	int errors = 0;
 
@@ -765,7 +771,7 @@ int parse_expiry_date(const char *date, unsigned long *timestamp)
 
 int parse_date(const char *date, struct strbuf *result)
 {
-	unsigned long timestamp;
+	time_t timestamp;
 	int offset;
 	if (parse_date_basic(date, &timestamp, &offset))
 		return -1;
@@ -837,7 +843,7 @@ void datestamp(struct strbuf *out)
  * Relative time update (eg "2 days ago").  If we haven't set the time
  * yet, we need to set it from current time.
  */
-static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)
+static time_t update_tm(struct tm *tm, struct tm *now, time_t sec)
 {
 	time_t n;
 
@@ -1058,7 +1064,7 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num,
 				     time_t now)
 {
 	char *end;
-	unsigned long number = strtoul(date, &end, 10);
+	time_t number = parse_time_t(date, &end, 10);
 
 	switch (*end) {
 	case ':':
@@ -1106,7 +1112,7 @@ static void pending_number(struct tm *tm, int *num)
 	}
 }
 
-static unsigned long approxidate_str(const char *date,
+static time_t approxidate_str(const char *date,
 				     const struct timeval *tv,
 				     int *error_ret)
 {
@@ -1143,9 +1149,9 @@ static unsigned long approxidate_str(const char *date,
 	return update_tm(&tm, &now, 0);
 }
 
-unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+time_t approxidate_relative(const char *date, const struct timeval *tv)
 {
-	unsigned long timestamp;
+	time_t timestamp;
 	int offset;
 	int errors = 0;
 
@@ -1154,10 +1160,10 @@ unsigned long approxidate_relative(const char *date, const struct timeval *tv)
 	return approxidate_str(date, tv, &errors);
 }
 
-unsigned long approxidate_careful(const char *date, int *error_ret)
+time_t approxidate_careful(const char *date, int *error_ret)
 {
 	struct timeval tv;
-	unsigned long timestamp;
+	time_t timestamp;
 	int offset;
 	int dummy = 0;
 	if (!error_ret)
@@ -1172,13 +1178,15 @@ unsigned long approxidate_careful(const char *date, int *error_ret)
 	return approxidate_str(date, &tv, error_ret);
 }
 
-int date_overflows(unsigned long t)
+int date_overflows(time_t t)
 {
 	time_t sys;
 
-	/* If we overflowed our unsigned long, that's bad... */
+#if 0 /* sizeof(time_t) <= sizeof(unsigned long) */
+	/* If we overflowed our time_t, that's bad... */
 	if (t == ULONG_MAX)
 		return 1;
+#endif
 
 	/*
 	 * ...but we also are going to feed the result to system
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 239898d..9940e68 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -31,14 +31,14 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
 int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
 			     int unset)
 {
-	*(unsigned long *)(opt->value) = approxidate(arg);
+	*(time_t *)(opt->value) = approxidate(arg);
 	return 0;
 }
 
 int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
 			     int unset)
 {
-	return parse_expiry_date(arg, (unsigned long *)opt->value);
+	return parse_expiry_date(arg, (time_t *)opt->value);
 }
 
 int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af2..d1a8673 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -12,7 +12,7 @@ struct complete_reflogs {
 	struct reflog_info {
 		unsigned char osha1[20], nsha1[20];
 		char *email;
-		unsigned long timestamp;
+		time_t timestamp;
 		int tz;
 		char *message;
 	} *items;
@@ -20,7 +20,7 @@ struct complete_reflogs {
 };
 
 static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
-		const char *email, unsigned long timestamp, int tz,
+		const char *email, time_t timestamp, int tz,
 		const char *message, void *cb_data)
 {
 	struct complete_reflogs *array = cb_data;
@@ -69,7 +69,7 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 }
 
 static int get_reflog_recno_by_time(struct complete_reflogs *array,
-	unsigned long timestamp)
+	time_t timestamp)
 {
 	int i;
 	for (i = array->nr - 1; i >= 0; i--)
@@ -141,7 +141,7 @@ void init_reflog_walk(struct reflog_walk_info **info)
 int add_reflog_for_walk(struct reflog_walk_info *info,
 		struct commit *commit, const char *name)
 {
-	unsigned long timestamp = 0;
+	time_t timestamp = 0;
 	int recno = -1;
 	struct string_list_item *item;
 	struct complete_reflogs *reflogs;
diff --git a/refs.c b/refs.c
index 87dc82f..ed1970f 100644
--- a/refs.c
+++ b/refs.c
@@ -620,7 +620,7 @@ int is_branch(const char *refname)
 
 struct read_ref_at_cb {
 	const char *refname;
-	unsigned long at_time;
+	time_t at_time;
 	int cnt;
 	int reccnt;
 	unsigned char *sha1;
@@ -629,15 +629,15 @@ struct read_ref_at_cb {
 	unsigned char osha1[20];
 	unsigned char nsha1[20];
 	int tz;
-	unsigned long date;
+	time_t date;
 	char **msg;
-	unsigned long *cutoff_time;
+	time_t *cutoff_time;
 	int *cutoff_tz;
 	int *cutoff_cnt;
 };
 
 static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
-		const char *email, unsigned long timestamp, int tz,
+		const char *email, time_t timestamp, int tz,
 		const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
@@ -684,7 +684,7 @@ static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
 }
 
 static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
-				  const char *email, unsigned long timestamp,
+				  const char *email, time_t timestamp,
 				  int tz, const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
@@ -704,9 +704,9 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
 	return 1;
 }
 
-int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time, int cnt,
+int read_ref_at(const char *refname, unsigned int flags, time_t at_time, int cnt,
 		unsigned char *sha1, char **msg,
-		unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
+		time_t *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
 {
 	struct read_ref_at_cb cb;
 
diff --git a/refs.h b/refs.h
index 56089d5..cac8fbf 100644
--- a/refs.h
+++ b/refs.h
@@ -246,9 +246,9 @@ int safe_create_reflog(const char *refname, int force_create, struct strbuf *err
 
 /** Reads log for the value of ref during at_time. **/
 extern int read_ref_at(const char *refname, unsigned int flags,
-		       unsigned long at_time, int cnt,
+		       time_t at_time, int cnt,
 		       unsigned char *sha1, char **msg,
-		       unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
+		       time_t *cutoff_time, int *cutoff_tz, int *cutoff_cnt);
 
 /** Check if a particular reflog exists */
 extern int reflog_exists(const char *refname);
@@ -274,7 +274,7 @@ extern int delete_refs(struct string_list *refnames);
 extern int delete_reflog(const char *refname);
 
 /* iterate over reflog entries */
-typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
+typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, time_t, int, const char *, void *);
 int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_data);
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);
 
@@ -512,7 +512,7 @@ typedef void reflog_expiry_prepare_fn(const char *refname,
 typedef int reflog_expiry_should_prune_fn(unsigned char *osha1,
 					  unsigned char *nsha1,
 					  const char *email,
-					  unsigned long timestamp, int tz,
+					  time_t timestamp, int tz,
 					  const char *message, void *cb_data);
 typedef void reflog_expiry_cleanup_fn(void *cb_data);
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1f38076..d809c92 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2816,7 +2816,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 {
 	unsigned char osha1[20], nsha1[20];
 	char *email_end, *message;
-	unsigned long timestamp;
+	time_t timestamp;
 	int tz;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
@@ -3297,7 +3297,7 @@ struct expire_reflog_cb {
 };
 
 static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-			     const char *email, unsigned long timestamp, int tz,
+			     const char *email, time_t timestamp, int tz,
 			     const char *message, void *cb_data)
 {
 	struct expire_reflog_cb *cb = cb_data;
diff --git a/revision.c b/revision.c
index d30d1c4..7b7b353 100644
--- a/revision.c
+++ b/revision.c
@@ -896,7 +896,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)
 /* How many extra uninteresting commits we want to see.. */
 #define SLOP 5
 
-static int still_interesting(struct commit_list *src, unsigned long date, int slop,
+static int still_interesting(struct commit_list *src, time_t date, int slop,
 			     struct commit **interesting_cache)
 {
 	/*
@@ -1030,7 +1030,7 @@ static void limit_left_right(struct commit_list *list, struct rev_info *revs)
 static int limit_list(struct rev_info *revs)
 {
 	int slop = SLOP;
-	unsigned long date = ~0ul;
+	time_t date = ~0ul;
 	struct commit_list *list = revs->commits;
 	struct commit_list *newlist = NULL;
 	struct commit_list **p = &newlist;
@@ -1227,7 +1227,7 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 }
 
 static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-		const char *email, unsigned long timestamp, int tz,
+		const char *email, time_t timestamp, int tz,
 		const char *message, void *cb_data)
 {
 	handle_one_reflog_commit(osha1, cb_data);
diff --git a/sha1_name.c b/sha1_name.c
index ca7ddd6..7d25734 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -542,8 +542,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,
 
 	if (reflog_len) {
 		int nth, i;
-		unsigned long at_time;
-		unsigned long co_time;
+		time_t at_time;
+		time_t co_time;
 		int co_tz, co_cnt;
 
 		/* Is it asking for N-th entry, or approxidate? */
@@ -935,7 +935,7 @@ struct grab_nth_branch_switch_cbdata {
 };
 
 static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
-				  const char *email, unsigned long timestamp, int tz,
+				  const char *email, time_t timestamp, int tz,
 				  const char *message, void *cb_data)
 {
 	struct grab_nth_branch_switch_cbdata *cb = cb_data;
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index d9ab360..be45d3b 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -32,7 +32,7 @@ static void show_dates(char **argv, const char *format)
 		 * Do not use our normal timestamp parsing here, as the point
 		 * is to test the formatting code in isolation.
 		 */
-		t = strtol(arg, &arg, 10);
+		t = strtoull(arg, &arg, 10);
 		while (*arg == ' ')
 			arg++;
 		tz = atoi(arg);
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 8a1235d..14cecf4 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -5,7 +5,7 @@
 static int boolean = 0;
 static int integer = 0;
 static unsigned long magnitude = 0;
-static unsigned long timestamp;
+static time_t timestamp;
 static int abbrev = 7;
 static int verbose = -1; /* unspecified */
 static int dry_run = 0, quiet = 0;
@@ -161,7 +161,7 @@ int main(int argc, char **argv)
 	show(&expect, &ret, "boolean: %d", boolean);
 	show(&expect, &ret, "integer: %d", integer);
 	show(&expect, &ret, "magnitude: %lu", magnitude);
-	show(&expect, &ret, "timestamp: %lu", timestamp);
+	show(&expect, &ret, "timestamp: %" PRIuMAX, timestamp);
 	show(&expect, &ret, "string: %s", string ? string : "(not set)");
 	show(&expect, &ret, "abbrev: %d", abbrev);
 	show(&expect, &ret, "verbose: %d", verbose);
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index e4b3959..9e1eb8d 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -47,7 +47,7 @@ static struct {
 
 static struct {
 	uint32_t revision;
-	unsigned long timestamp;
+	time_t timestamp;
 	struct strbuf log, author, note;
 } rev_ctx;
 
diff --git a/wt-status.c b/wt-status.c
index 4ce4e35..67858ad 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1303,7 +1303,7 @@ struct grab_1st_switch_cbdata {
 };
 
 static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
-			   const char *email, unsigned long timestamp, int tz,
+			   const char *email, time_t timestamp, int tz,
 			   const char *message, void *cb_data)
 {
 	struct grab_1st_switch_cbdata *cb = cb_data;
-- 
2.9.0.278.g1caae67

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 09:30:28AM +0200, Johannes Schindelin wrote:

> On Mon, 11 Jul 2016, Junio C Hamano wrote:
> 
> > Those who care about 32-bit builds need to start building and
> > testing 'next' and 'master' regularly, or similar breakages are
> > bound to continue happening X-<.
> 
> Please note that "unsigned long" is 32-bit even on 64-bit Windows.

Yeah, that was part of the reason my run-time test checked
sizeof(unsigned long), and not any kind of "are we 64-bit?" compiler
options or output. So the "64BIT" prereq is actually a bit of a
misnomer; it is really "is your ULL 64-bit, because that's what we use
for time". :-/

> FWIW I have this monster patch as a starting point (I plan to work more on
> this today):

Cool! Thanks for working on this.

I suspect we should still do something about skipping those tests,
though, if only because the v2.9.x maint track is broken, and switching
to time_t is a sufficiently large change that we probably don't want it
for maint (it _seems_ like it shouldn't cause any problems, but I'm
wondering if we might inadvertently trigger funny issues around
signedness or something).

> From b65de5a56e18b038d432ba828d7714880b8e285c Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Tue, 12 Jul 2016 09:09:07 +0200
> Subject: [PATCH] HOTFIX: use time_t wherever appropriate
> 
> Git's source code assumes that unsigned long is at least as precise as
> time_t. Well, Git's source code is wrong.

Another problem with "unsigned long" is that we cannot handle times
older than 1970. Assuming most systems are sane enough to have a signed
time_t these days, this would fix that problem as well.

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

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

> @@ -88,11 +88,11 @@ static int local_tzoffset(unsigned long time)
>  	return offset * eastwest;
>  }
>  
> -void show_date_relative(unsigned long time, int tz,
> +void show_date_relative(time_t time, int tz,
>  			       const struct timeval *now,
>  			       struct strbuf *timebuf)
>  {
> -	unsigned long diff;
> +	time_t diff;
>  	if (now->tv_sec < time) {
>  		strbuf_addstr(timebuf, _("in the future"));
>  		return;
> @@ -100,65 +100,65 @@ void show_date_relative(unsigned long time, int tz,
>  	diff = now->tv_sec - time;
>  	if (diff < 90) {
>  		strbuf_addf(timebuf,
> -			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
> +			 Q_("%" PRIuMAX " second ago", "%" PRIuMAX " seconds ago", diff), diff);

PRIuMAX isn't compatible with time_t.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Andreas,

On Tue, 12 Jul 2016, Andreas Schwab wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > @@ -88,11 +88,11 @@ static int local_tzoffset(unsigned long time)
> >  	return offset * eastwest;
> >  }
> >  
> > -void show_date_relative(unsigned long time, int tz,
> > +void show_date_relative(time_t time, int tz,
> >  			       const struct timeval *now,
> >  			       struct strbuf *timebuf)
> >  {
> > -	unsigned long diff;
> > +	time_t diff;
> >  	if (now->tv_sec < time) {
> >  		strbuf_addstr(timebuf, _("in the future"));
> >  		return;
> > @@ -100,65 +100,65 @@ void show_date_relative(unsigned long time, int tz,
> >  	diff = now->tv_sec - time;
> >  	if (diff < 90) {
> >  		strbuf_addf(timebuf,
> > -			 Q_("%lu second ago", "%lu seconds ago", diff), diff);
> > +			 Q_("%" PRIuMAX " second ago", "%" PRIuMAX " seconds ago", diff), diff);
> 
> PRIuMAX isn't compatible with time_t.

That statement is wrong. But you probably meant that PRIuMAX is *not
necessarily* the correct thing to use.

And I would agree with that. I had to have a patch that 1) compiles and 2)
fixes t0006 on Windows, and the patch I presented achieved both goals.

I hoped that my brief "starting point" hint would make it obvious that I
do not think that this patch is acceptable?

My idea was to introduce a TIME_T_LARGER_THAN_ULONG and take it from
there, but I had to switch contexts before I could finish that part of the
patch, yet I still wanted to let y'all know that patches are in the
working.

For future record: I appreciate feedback especially when it is
constructive, i.e. when "that's wrong" is not left on its own, but is
instead followed by "why not do XYZ instead".

Ciao,
Johannes

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Peff,

On Tue, 12 Jul 2016, Jeff King wrote:

> On Tue, Jul 12, 2016 at 09:30:28AM +0200, Johannes Schindelin wrote:
> 
> > FWIW I have this monster patch as a starting point (I plan to work more on
> > this today):
> 
> Cool! Thanks for working on this.

Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
do that with a failing test suite.

> I suspect we should still do something about skipping those tests,
> though, if only because the v2.9.x maint track is broken, and switching
> to time_t is a sufficiently large change that we probably don't want it
> for maint (it _seems_ like it shouldn't cause any problems, but I'm
> wondering if we might inadvertently trigger funny issues around
> signedness or something).

I totally agree that this patch is very, very large. And yeah, this change
is intrusive enough that it should not hit maint (but then, IMO neither
should the change that triggered this test failure have hit maint).

So I think I'll go with this for the moment (as we all know, my patch
series often go through a couple of commenting rounds, and are not always
picked up quickly, and I do not wait that long with Git for Windows
v2.9.1):

-- snipsnap --
[PATCH] Work around test failures due to timestamps being unsigned long

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".

While we will fix this issue properly by replacing unsigned long ->
time_t, on the maint track we want to be a bit more conservative and
just skip those tests.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t0006-date.sh | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 04ce535..d185640 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -48,10 +48,17 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '1466000000 +0200'
 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"
+case "$(test-date show:iso 9999999999)" in
+*2038*)
+	# on this platform, unsigned long is 32-bit, i.e. not large enough
+	;;
+*)
+	# 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"
+	;;
+esac
 
 check_parse() {
 	echo "$1 -> $2" >expect
-- 
2.9.0.278.g1caae67

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Johannes Schindelin <schindelin@wisc.edu> writes:

>> PRIuMAX isn't compatible with time_t.
>
> That statement is wrong.

No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
(even if they happen to have the same representation).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Andreas,

On Tue, 12 Jul 2016, Andreas Schwab wrote:

> Johannes Schindelin <schindelin@wisc.edu> writes:
> 
> >> PRIuMAX isn't compatible with time_t.
> >
> > That statement is wrong.
> 
> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> (even if they happen to have the same representation).

Sigh.

So if it is wrong, what is right? I hoped that my gentle reprimand in my
previous reply would result in a more helpful response.

Let me guess: PRId64 is what you would have suggested if you had followed
up your negative statement with a "do this instead"?

Ciao,
Johannes

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

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

> Hi Andreas,
>
> On Tue, 12 Jul 2016, Andreas Schwab wrote:
>
>> Johannes Schindelin <schindelin@wisc.edu> writes:
>> 
>> >> PRIuMAX isn't compatible with time_t.
>> >
>> > That statement is wrong.
>> 
>> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
>> (even if they happen to have the same representation).
>
> Sigh.
>
> So if it is wrong, what is right?

The right thing is to add a cast, of course.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 03:31:00PM +0200, Andreas Schwab wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Hi Andreas,
> >
> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
> >
> >> Johannes Schindelin <schindelin@wisc.edu> writes:
> >> 
> >> >> PRIuMAX isn't compatible with time_t.
> >> >
> >> > That statement is wrong.
> >> 
> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> >> (even if they happen to have the same representation).
> >
> > Sigh.
> >
> > So if it is wrong, what is right?
> 
> The right thing is to add a cast, of course.

In general, I think the right cast for time_t should be to (intmax_t),
and the formatting string should be PRIdMAX (which, as an aside, needs
an entry in git-compat-util.h).

In this particular code (show_date_relative), though, I think you can
get away with treating it as unsigned, because it's not actually a
time_t but rather a difference. And we handle the negative difference at
the top of the function already ("in the future").

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 01:25:20PM +0200, Johannes Schindelin wrote:

> [PATCH] Work around test failures due to timestamps being unsigned long
> 
> 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".
> 
> While we will fix this issue properly by replacing unsigned long ->
> time_t, on the maint track we want to be a bit more conservative and
> just skip those tests.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

This looks like a reasonable interim fix for both Windows and for the
general maint track. If we reliably produce "2038" for the 999... value
then that's simpler than adding in magic to ask about sizeof(ulong). I
also wondered if we should test forthe _correct_ value, but that would
defeat the point of the test (999... is also far in the future).

One minor comment:

> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 04ce535..d185640 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -48,10 +48,17 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
>  check_show raw "$TIME" '1466000000 +0200'
>  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"
> +case "$(test-date show:iso 9999999999)" in
> +*2038*)
> +	# on this platform, unsigned long is 32-bit, i.e. not large enough
> +	;;
> +*)
> +	# 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"
> +	;;
> +esac

It would be nice to wrap this in a prereq, like:

  test_lazy_prereq 64BIT_TIME '
	case "$(test-date show:short 9999999999)" in
	*2038*)
		false
		;;
	*)
		true
		;;
	esac
  '

  ...
  check_show 64BIT_TIME iso       "$FUTURE" "2152-06-19 18:24:56 -0400"
  check_show 64BIT_TIME iso-local "$FUTURE" "2152-06-19 22:24:56 +0000"

The main advantage is that it will number the tests consistently, and
give us a "skipped" message (which can remind us to drop the prereq
later when everything goes 64-bit).

But it also will do the right thing with test-date's output with respect
to "-v" (not that we expect it to produce any output).

You'll need to adjust check_show as I did in my earlier patch.

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Mon, Jul 11, 2016 at 08:40:56PM -0400, Anders Kaseorg wrote:

> On 07/11/2016 07:54 PM, Jeff King wrote:
> > Yes, that's somewhat the point of the test.
> > 
> > How does it fail for you (what does it look like with "-v")? We may be
> > able to check for an outcome that matches both cases.
> 
> On Ubuntu i386 and Ubuntu armhf, I get the following verbose output from
> t0006-date.sh:
> 
> 
> expecting success:
> 		echo "$time -> $expect" >expect &&
> 		test-date show:$format "$time" >actual &&
> 		test_cmp expect actual
> 	
> --- expect	2016-07-11 23:20:55.835136188 +0000
> +++ actual	2016-07-11 23:20:55.835136188 +0000
> @@ -1 +1 @@
> -5758122296 -0400 -> 2152-06-19 18:24:56 -0400
> +5758122296 -0400 -> 2038-01-18 23:14:07 -0400

Thank you for this, by the way. Your comment got drowned out by the rest
of the thread, but this would have been very helpful for me in writing a
patch (fortunately Dscho beat me to it, so I did not have to. :) ).

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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

> Git's source code assumes that unsigned long is at least as precise as
> time_t. Well, Git's source code is wrong.
> ...

That is correct.  As people mentioned downthread already, "unsigned
long" has two problems, it may not be wide enough, and it cannot
represent time before the epoch.

But moving the internal time representation used in various fields
like commit->date to time_t is likely to be a wrong thing to do,
because the first problem with "unsigned long", i.e. "may not be
wide enough", is not limited to "not wide enough to hold time_t".
It also includes "it may not be wide enough to hold time somebody
else recorded in existing objects".

Since some platforms have time_t that is not wide enough, but still
have intmax_t that is wider, I think we would be better off to pick
an integral type to use for the internal representation that is the
widest throughout the API, and use time_t only at places that we
interact with the system libraries (e.g. when we ask "what is the
time now?" to time(2), when we ask "break down this timestamp" to
gmtime(3)).

Thanks for starting this, and from a brief read, the hotfix to skip
the test downthread looked good.  The places this "starting point"
patch covers look like a good set that interacts with time obtained
locally (e.g. prune that compares with filesystem timestamp); just
make sure you don't go too far and end up shoving timestamps from
other people into time_t, which may not fit.

Thanks.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 07:34:14AM -0700, Junio C Hamano wrote:

> > Git's source code assumes that unsigned long is at least as precise as
> > time_t. Well, Git's source code is wrong.
> > ...
> 
> That is correct.  As people mentioned downthread already, "unsigned
> long" has two problems, it may not be wide enough, and it cannot
> represent time before the epoch.
> 
> But moving the internal time representation used in various fields
> like commit->date to time_t is likely to be a wrong thing to do,
> because the first problem with "unsigned long", i.e. "may not be
> wide enough", is not limited to "not wide enough to hold time_t".
> It also includes "it may not be wide enough to hold time somebody
> else recorded in existing objects".

But that's a problem no matter what size we choose. The ascii format in
the commit objects is arbitrary-length, so somebody can always overflow
it. So even with intmax_t we have to clamp it to a sentinel value at
some point. IMHO we are better off to do so at parse time, and then have
consistent sizes through the rest of the code base, without worrying
about juggling intmax_t to time_t truncation in multiple places.

IOW, I think we probably interact with the system time libraries more
often than we parse (and it's easy to wrap the parsing in a function,
but there are a lot of system time functions).

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

On Tue, Jul 12, 2016 at 8:16 AM, Jeff King <peff@peff.net> wrote:
>>
>> But moving the internal time representation used in various fields
>> like commit->date to time_t is likely to be a wrong thing to do,
>> because the first problem with "unsigned long", i.e. "may not be
>> wide enough", is not limited to "not wide enough to hold time_t".
>> It also includes "it may not be wide enough to hold time somebody
>> else recorded in existing objects".
>
> But that's a problem no matter what size we choose.

Yes, if somebody's time_t is larger than my intmax_t, the problem
cannot be solved for me, if that timestamp is too far in the future or
in the past.

But that is not the problem I am pointing out. I heard earlier in the
thread that time_t on one system was 32-bit (was it Linux?) but I think
they have "long long". Choosing time_t is strictly inferior choice when
we already know that a platform with not-wide-enough time_t need to
be supported, and a type that is wider than that is available.

I was envisioning that we would have typedef <sometime> gittime_t
with conversion helpers between it and time_t that allow us to do some
range checks while at it.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 08:25:51AM -0700, Junio C Hamano wrote:

> On Tue, Jul 12, 2016 at 8:16 AM, Jeff King <peff@peff.net> wrote:
> >>
> >> But moving the internal time representation used in various fields
> >> like commit->date to time_t is likely to be a wrong thing to do,
> >> because the first problem with "unsigned long", i.e. "may not be
> >> wide enough", is not limited to "not wide enough to hold time_t".
> >> It also includes "it may not be wide enough to hold time somebody
> >> else recorded in existing objects".
> >
> > But that's a problem no matter what size we choose.
> 
> Yes, if somebody's time_t is larger than my intmax_t, the problem
> cannot be solved for me, if that timestamp is too far in the future or
> in the past.

I am less worried about their time_t and more about whatever crap they
write in ascii into their objects. :)

> But that is not the problem I am pointing out. I heard earlier in the
> thread that time_t on one system was 32-bit (was it Linux?) but I think
> they have "long long". Choosing time_t is strictly inferior choice when
> we already know that a platform with not-wide-enough time_t need to
> be supported, and a type that is wider than that is available.

I am not certain that there is a modern system with 32-bit time_t. We
know there are systems with 32-bit unsigned long, and I think that is
what produced the results people saw. I'd expect even 32-bit systems to
use "int64_t" or similar for their time_t these days.

I'm also not convinced that we would be helping much to carry around a
wider gittime_t. Most of the display code ends up touching a system time
function one way or another, so I find it unlikely it would produce much
better output.

It would help for simple cases like commit->date where we really do just
parse it into a number and never do more with it. But...

> I was envisioning that we would have typedef <sometime> gittime_t
> with conversion helpers between it and time_t that allow us to do some
> range checks while at it.

I guess I am just willing to trust that time_t is basically that. And if
your platform has a grossly undersized time_t, then too bad, we clamp
everything it can't hold to 2038 or whatever, and hopefully your
terrible platform dies out or gets a clue sometime in the next 20 years.

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I am not certain that there is a modern system with 32-bit time_t. We
> know there are systems with 32-bit unsigned long, and I think that is
> what produced the results people saw. I'd expect even 32-bit systems to
> use "int64_t" or similar for their time_t these days.

OK.

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> However, I was thinking that it might be handy to have this and some
> other information available for helping with debugging. E.g., that we
> could ask bug reporters for "git version --build-options" when we
> suspect something related to their config.

Sure.  I was wrong to phrase it as "I would have..."; either is fine
by me, and being able to ask "git version --build-options" is
probably a plus.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 08:41:42AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I am not certain that there is a modern system with 32-bit time_t. We
> > know there are systems with 32-bit unsigned long, and I think that is
> > what produced the results people saw. I'd expect even 32-bit systems to
> > use "int64_t" or similar for their time_t these days.
> 
> OK.

In case it wasn't clear, I was mostly guessing there. So I dug a bit
further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
on i386 because of the ABI headaches.

That being said, I still think the "clamp to time_t" strategy is
reasonable. Unless you are doing something really exotic like pretending
to be from the future, nobody will care for 20 years. And at that point,
systems with a 32-bit time_t are going to have to do _something_,
because time() is going to start returning bogus values. So as long as
we behave reasonably (e.g., clamping values and not generating wrapped
nonsense), I think that's a fine solution.

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> In case it wasn't clear, I was mostly guessing there. So I dug a bit
> further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> on i386 because of the ABI headaches.

X-< (yes, I knew).

> That being said, I still think the "clamp to time_t" strategy is
> reasonable. Unless you are doing something really exotic like pretending
> to be from the future, nobody will care for 20 years.

Yup.  It is a minor regression for them to go from ulong to time_t,
because they didn't have to care for 90 years or so but now they do
in 20 years, I'd guess, but hopefully after that many years,
everybody's time_t would be sufficiently large.

I suspect Cobol programmers in the 50s would have said a similar
thing about the y2k timebomb they created back then, though ;-)

> And at that point, systems with a 32-bit time_t are going to have
> to do _something_, because time() is going to start returning
> bogus values. So as long as we behave reasonably (e.g., clamping
> values and not generating wrapped nonsense), I think that's a fine
> solution.

OK.

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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

>> Cool! Thanks for working on this.
>
> Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
> do that with a failing test suite.

Let's do 2.9.2 together, as this is not limited to GfW.

Taking Peff's suggestions into account, perhaps like the attached?

It wasn't readily apparent to me why 2038 check worked, so I added a
short paragraph at the end, but those who know the test helper well
enough may find it redundant, in which case I am fine with removing
it.

Thanks.

-- >8 --
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Tue, 12 Jul 2016 13:25:20 +0200
Subject: t0006: skip "far in the future" test when ulong is not long enough

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".

While we can fix this issue properly by replacing unsigned long with
a larger type, we want to be a bit more conservative and just skip
those tests on the maint track.

The test helper reads the timestamp given from the command line into
"unsigned long" using strtol(), which gives us LONG_MAX upon
overflow, so feeding 9999999999 and seeing the resulting "timestamp"
shown in year 2038 is a sufficient check for that condition.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0006-date.sh | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 04ce535..6070c29 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
@@ -48,10 +48,22 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '1466000000 +0200'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
 
+test_lazy_prereq 64BIT_TIME '
+	case "$(test-date show:iso 9999999999)" in
+	*" -> 2038-"*)
+		# on this platform, unsigned long is 32-bit, i.e. not large enough
+		false
+		;;
+	*)
+		true
+		;;
+	esac
+'
+
 # 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" 64BIT_TIME
+check_show iso-local "$FUTURE" "2152-06-19 22:24:56 +0000" 64BIT_TIME
 
 check_parse() {
 	echo "$1 -> $2" >expect
-- 
2.9.1-500-g4c1e1e0

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

Jeff King <peff@peff.net> writes:

> In case it wasn't clear, I was mostly guessing there. So I dug a bit
> further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> on i386 because of the ABI headaches.

This is being worked on.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Duy Nguyen]

On Tue, Jul 12, 2016 at 3:46 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Jul 12, 2016 at 03:31:00PM +0200, Andreas Schwab wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > Hi Andreas,
>> >
>> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
>> >
>> >> Johannes Schindelin <schindelin@wisc.edu> writes:
>> >>
>> >> >> PRIuMAX isn't compatible with time_t.
>> >> >
>> >> > That statement is wrong.
>> >>
>> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
>> >> (even if they happen to have the same representation).
>> >
>> > Sigh.
>> >
>> > So if it is wrong, what is right?
>>
>> The right thing is to add a cast, of course.
>
> In general, I think the right cast for time_t should be to (intmax_t),
> and the formatting string should be PRIdMAX (which, as an aside, needs
> an entry in git-compat-util.h).

Coincidentally, I have the same problem with unsigned long being
32-bit and have to switch to off_t in some places. Does anybody know
what a fallback in git-compat-util for PRIdMAX would look like? I
guess it's "lld"...
-- 
Duy

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

On Tue, Jul 12, 2016 at 11:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Let's do 2.9.2 together, as this is not limited to GfW.
>
> Taking Peff's suggestions into account, perhaps like the attached?

If I can get positive comments soon enough, I can do 2.9.2 early
tomorrow my time.
Or a reasonable counter-proposal (including "without Peff's
suggestion, because...")
would work well, too, for that.

Thanks.

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

On Tue, Jul 12, 2016 at 11:12:25AM -0700, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Cool! Thanks for working on this.
> >
> > Well, I had to. Git for Windows v2.9.1 needs to get released, and I won't
> > do that with a failing test suite.
> 
> Let's do 2.9.2 together, as this is not limited to GfW.
> 
> Taking Peff's suggestions into account, perhaps like the attached?

It looks good to me.

> It wasn't readily apparent to me why 2038 check worked, so I added a
> short paragraph at the end, but those who know the test helper well
> enough may find it redundant, in which case I am fine with removing
> it.

Definitely keep that paragraph. I am quite familiar with the test
helper and it was not the outcome I initially expected.

> +test_lazy_prereq 64BIT_TIME '
> +	case "$(test-date show:iso 9999999999)" in
> +	*" -> 2038-"*)
> +		# on this platform, unsigned long is 32-bit, i.e. not large enough
> +		false

I see you tightened up the match a little. TBH, I think we could
probably just match the whole output string, but I doubt there's much
chance of a false positive either way.

-Peff

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Andreas,

On Tue, 12 Jul 2016, Andreas Schwab wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
> >
> >> Johannes Schindelin <schindelin@wisc.edu> writes:
> >> 
> >> >> PRIuMAX isn't compatible with time_t.
> >> >
> >> > That statement is wrong.
> >> 
> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> >> (even if they happen to have the same representation).
> >
> > Sigh.
> >
> > So if it is wrong, what is right?
> 
> The right thing is to add a cast, of course.

This was not helpful.

Ciao,
Johannes

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Duy,

On Tue, 12 Jul 2016, Duy Nguyen wrote:

> On Tue, Jul 12, 2016 at 3:46 PM, Jeff King <peff@peff.net> wrote:
> > On Tue, Jul 12, 2016 at 03:31:00PM +0200, Andreas Schwab wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > On Tue, 12 Jul 2016, Andreas Schwab wrote:
> >> >
> >> >> Johannes Schindelin <schindelin@wisc.edu> writes:
> >> >>
> >> >> >> PRIuMAX isn't compatible with time_t.
> >> >> >
> >> >> > That statement is wrong.
> >> >>
> >> >> No, it isn't.  PRIuMAX is for uintmax_t, and time_t is not uintmax_t
> >> >> (even if they happen to have the same representation).
> >> >
> >> > Sigh.
> >> >
> >> > So if it is wrong, what is right?
> >>
> >> The right thing is to add a cast, of course.
> >
> > In general, I think the right cast for time_t should be to (intmax_t),
> > and the formatting string should be PRIdMAX (which, as an aside, needs
> > an entry in git-compat-util.h).
> 
> Coincidentally, I have the same problem with unsigned long being
> 32-bit and have to switch to off_t in some places. Does anybody know
> what a fallback in git-compat-util for PRIdMAX would look like? I
> guess it's "lld"...

Yes, judging from the existing fallback for PRIuMAX, "lld" would be the
correct thing to do. And then it would be nice to introduce

	#define PRIdMAX "I64d"

next to the PRIuMAX definition in compat/mingw.h, too.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Peff,

On Tue, 12 Jul 2016, Jeff King wrote:

> On Tue, Jul 12, 2016 at 01:25:20PM +0200, Johannes Schindelin wrote:
> 
> > [PATCH] Work around test failures due to timestamps being unsigned long
> > 
> > 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".
> > 
> > While we will fix this issue properly by replacing unsigned long ->
> > time_t, on the maint track we want to be a bit more conservative and
> > just skip those tests.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> This looks like a reasonable interim fix for both Windows and for the
> general maint track. If we reliably produce "2038" for the 999... value
> then that's simpler than adding in magic to ask about sizeof(ulong). I
> also wondered if we should test forthe _correct_ value, but that would
> defeat the point of the test (999... is also far in the future).

This was just a quick fix, intended to allow me to release Git for Windows
v2.9.1 in a timely manner (which is now delayed for other reasons).

I think I can do even better than to inspect the source code whether it
reliably clamps the dates to PI hours on January 19th, 2038: We already
have a t/helper/test-date and we can easily teach it one new trick.

> One minor comment:
> 
> > diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> > index 04ce535..d185640 100755
> > --- a/t/t0006-date.sh
> > +++ b/t/t0006-date.sh
> > @@ -48,10 +48,17 @@ check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
> >  check_show raw "$TIME" '1466000000 +0200'
> >  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"
> > +case "$(test-date show:iso 9999999999)" in
> > +*2038*)
> > +	# on this platform, unsigned long is 32-bit, i.e. not large enough
> > +	;;
> > +*)
> > +	# 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"
> > +	;;
> > +esac
> 
> It would be nice to wrap this in a prereq, like:
> 
>   test_lazy_prereq 64BIT_TIME '
> 	case "$(test-date show:short 9999999999)" in
> 	*2038*)
> 		false
> 		;;
> 	*)
> 		true
> 		;;
> 	esac
>   '
> 
>   ...
>   check_show 64BIT_TIME iso       "$FUTURE" "2152-06-19 18:24:56 -0400"
>   check_show 64BIT_TIME iso-local "$FUTURE" "2152-06-19 22:24:56 +0000"
> 
> The main advantage is that it will number the tests consistently, and
> give us a "skipped" message (which can remind us to drop the prereq
> later when everything goes 64-bit).
> 
> But it also will do the right thing with test-date's output with respect
> to "-v" (not that we expect it to produce any output).
> 
> You'll need to adjust check_show as I did in my earlier patch.

Makes sense!

Thanks,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi,

On Tue, 12 Jul 2016, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > In case it wasn't clear, I was mostly guessing there. So I dug a bit
> > further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> > on i386 because of the ABI headaches.
> 
> X-< (yes, I knew).
> 
> > That being said, I still think the "clamp to time_t" strategy is
> > reasonable. Unless you are doing something really exotic like pretending
> > to be from the future, nobody will care for 20 years.
> 
> Yup.  It is a minor regression for them to go from ulong to time_t,
> because they didn't have to care for 90 years or so but now they do
> in 20 years, I'd guess, but hopefully after that many years,
> everybody's time_t would be sufficiently large.
> 
> I suspect Cobol programmers in the 50s would have said a similar
> thing about the y2k timebomb they created back then, though ;-)
> 
> > And at that point, systems with a 32-bit time_t are going to have
> > to do _something_, because time() is going to start returning
> > bogus values. So as long as we behave reasonably (e.g., clamping
> > values and not generating wrapped nonsense), I think that's a fine
> > solution.
> 
> OK.

I kept the unsigned long -> time_t conversion after reading the thread so
far.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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

> This was just a quick fix, intended to allow me to release Git for Windows
> v2.9.1 in a timely manner (which is now delayed for other reasons).
> ...
>> You'll need to adjust check_show as I did in my earlier patch.
>
> Makes sense!

Hmph, so what is your preferred approach?  You'll do your own v2.9.1
that is different from others at t0006?

I was hoping to hear from you sooner and do v2.9.2 with your t0006
workaround with lazy-prereq changes from Peff (i.e. "Makes sense!"
above), so that you do not have to do two releases in a row
(i.e. skipping v2.9.1 saying "Git for Windows skipped that one
because it was not quite right; this release fixes the issue" in
your v2.9.2 announcement).

Thanks.

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Definitely keep that paragraph. I am quite familiar with the test
> helper and it was not the outcome I initially expected.
>
>> +test_lazy_prereq 64BIT_TIME '
>> +	case "$(test-date show:iso 9999999999)" in
>> +	*" -> 2038-"*)
>> +		# on this platform, unsigned long is 32-bit, i.e. not large enough
>> +		false
>
> I see you tightened up the match a little. TBH, I think we could
> probably just match the whole output string, but I doubt there's much
> chance of a false positive either way.

Ah, it wasn't meant to be a tightening; rather the above is for
documentation value, i.e. make it stand out what 2038 we are
matching---its answer being "the year portion of the result of the
conversion".

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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

> On Tue, 12 Jul 2016, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > In case it wasn't clear, I was mostly guessing there. So I dug a bit
>> > further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
>> > on i386 because of the ABI headaches.
>> 
>> X-< (yes, I knew).
>> 
>> > That being said, I still think the "clamp to time_t" strategy is
>> > reasonable. Unless you are doing something really exotic like pretending
>> > to be from the future, nobody will care for 20 years.
>> 
>> Yup.  It is a minor regression for them to go from ulong to time_t,
>> because they didn't have to care for 90 years or so but now they do
>> in 20 years, I'd guess, but hopefully after that many years,
>> everybody's time_t would be sufficiently large.
>> 
>> I suspect Cobol programmers in the 50s would have said a similar
>> thing about the y2k timebomb they created back then, though ;-)
>> 
>> > And at that point, systems with a 32-bit time_t are going to have
>> > to do _something_, because time() is going to start returning
>> > bogus values. So as long as we behave reasonably (e.g., clamping
>> > values and not generating wrapped nonsense), I think that's a fine
>> > solution.
>> 
>> OK.
>
> I kept the unsigned long -> time_t conversion after reading the thread so
> far.

That's OK at this point; it is not v2.9.x material anyway.

The primary reason why I cared 32-bit time_t is not about 2038, by
the way.  I recall that people wanted to store historical document
with ancient timestamp; even if we update to support negative
timestamps, they cannot go back to 19th century with 32-bit time_t,
but they can with long long or whatever intmax_t is on their system.

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Definitely keep that paragraph. I am quite familiar with the test
> > helper and it was not the outcome I initially expected.
> >
> >> +test_lazy_prereq 64BIT_TIME '
> >> +	case "$(test-date show:iso 9999999999)" in
> >> +	*" -> 2038-"*)
> >> +		# on this platform, unsigned long is 32-bit, i.e. not large enough
> >> +		false
> >
> > I see you tightened up the match a little. TBH, I think we could
> > probably just match the whole output string, but I doubt there's much
> > chance of a false positive either way.
> 
> Ah, it wasn't meant to be a tightening; rather the above is for
> documentation value, i.e. make it stand out what 2038 we are
> matching---its answer being "the year portion of the result of the
> conversion".

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:

-- 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'
-- 
2.9.0.278.g1caae67

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Tue, 12 Jul 2016, Junio C Hamano wrote:
> >
> >> Jeff King <peff@peff.net> writes:
> >> 
> >> > In case it wasn't clear, I was mostly guessing there. So I dug a bit
> >> > further, and indeed, I am wrong. Linux never bumped to a 64-bit time_t
> >> > on i386 because of the ABI headaches.
> >> 
> >> X-< (yes, I knew).
> >> 
> >> > That being said, I still think the "clamp to time_t" strategy is
> >> > reasonable. Unless you are doing something really exotic like pretending
> >> > to be from the future, nobody will care for 20 years.
> >> 
> >> Yup.  It is a minor regression for them to go from ulong to time_t,
> >> because they didn't have to care for 90 years or so but now they do
> >> in 20 years, I'd guess, but hopefully after that many years,
> >> everybody's time_t would be sufficiently large.
> >> 
> >> I suspect Cobol programmers in the 50s would have said a similar
> >> thing about the y2k timebomb they created back then, though ;-)
> >> 
> >> > And at that point, systems with a 32-bit time_t are going to have
> >> > to do _something_, because time() is going to start returning
> >> > bogus values. So as long as we behave reasonably (e.g., clamping
> >> > values and not generating wrapped nonsense), I think that's a fine
> >> > solution.
> >> 
> >> OK.
> >
> > I kept the unsigned long -> time_t conversion after reading the thread so
> > far.
> 
> That's OK at this point; it is not v2.9.x material anyway.

Got it. I will try to get the patches submitted soon, anyway.

> The primary reason why I cared 32-bit time_t is not about 2038, by
> the way.  I recall that people wanted to store historical document
> with ancient timestamp; even if we update to support negative
> timestamps, they cannot go back to 19th century with 32-bit time_t,
> but they can with long long or whatever intmax_t is on their system.

Fair enough.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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'

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This was just a quick fix, intended to allow me to release Git for Windows
> > v2.9.1 in a timely manner (which is now delayed for other reasons).
> > ...
> >> You'll need to adjust check_show as I did in my earlier patch.
> >
> > Makes sense!
> 
> Hmph, so what is your preferred approach?  You'll do your own v2.9.1
> that is different from others at t0006?

I may do a Git for Windows v2.9.1 that is different from Git v2.9.1 in
t0006, yes. Git for Windows still has tons of patches on top of Git
because I seem to be unable to drain the patch queue as fast as I want, so
I do not believe it is a big deal.

> I was hoping to hear from you sooner and do v2.9.2 with your t0006
> workaround with lazy-prereq changes from Peff (i.e. "Makes sense!"
> above), so that you do not have to do two releases in a row
> (i.e. skipping v2.9.1 saying "Git for Windows skipped that one
> because it was not quite right; this release fixes the issue" in
> your v2.9.2 announcement).

I am sorry that you expected me to be more available. It is a pretty crazy
week already (and trying to get a Git for Windows v2.9.1 out of the door
after dropping everything else on Tuesday morning added quite a bit to the
load), so I am at times unable to even read the Git mailing list.

As I am more concerned with Jeff Hostetler's patch now (the "very verbose
porcelain status"; I merged the Pull Request after seeing no comments on
his mail, but then Peff provided some good feedback, so now I am not quite
certain how to go about it: revert, or wait if the 2nd iteration is
acceptable and take that), so I am not immediately releasing any version,
anyway.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

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

>> I was hoping to hear from you sooner and do v2.9.2 with your t0006
>> workaround with lazy-prereq changes from Peff (i.e. "Makes sense!"
>> above), so that you do not have to do two releases in a row
>> (i.e. skipping v2.9.1 saying "Git for Windows skipped that one
>> because it was not quite right; this release fixes the issue" in
>> your v2.9.2 announcement).
>
> I am sorry that you expected me to be more available. It is a pretty crazy
> week already (and trying to get a Git for Windows v2.9.1 out of the door
> after dropping everything else on Tuesday morning added quite a bit to the
> load), so I am at times unable to even read the Git mailing list.

Actually these back-and-forth was an attempt to help you reduce the
load by not having to worry about the t0006 workaround.  Checking
your inbox would have been quicker than writing another of your own
version.

> As I am more concerned with Jeff Hostetler's patch now (the "very verbose
> porcelain status"; I merged the Pull Request after seeing no comments on
> his mail, but then Peff provided some good feedback, so now I am not quite
> certain how to go about it: revert, or wait if the 2nd iteration is
> acceptable and take that), so I am not immediately releasing any version,
> anyway.

As I said, I'd be waiting for a reroll of that to queue on 'pu', but
as a new feature, it won't appear in any of the v2.9.x releases.

Re: [ANNOUNCE] Git v2.9.1

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> It is somewhat disturbing that nobody seems to be regularly building
> on 32-bit platforms these days, which is the only reason I can think
> of why this was never reported until it hit a maintenance track.
> This should have been caught last week at f6a729f3 (Merge branch
> 'jk/tzoffset-fix', 2016-07-06) when the topic hit 'master' at the
> latest, and more preferrably it should have already been caught last
> month at 08ec8c5e (Merge branch 'jk/tzoffset-fix' into next,
> 2016-06-28).
>
> Those who care about 32-bit builds need to start building and
> testing 'next' and 'master' regularly, or similar breakages are
> bound to continue happening X-<.
>
> Volunteers?

We might eventually see a volunteer or two but that hasn't happened
yet, at least in the past few days.

Does Travis CI testing have an option to run our tests on some
32-bit platforms?

Re: [ANNOUNCE] Git v2.9.1

[Lars Schneider]

On 13 Jul 2016, at 22:43, Junio C Hamano <gitster@pobox.com> wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> It is somewhat disturbing that nobody seems to be regularly building
>> on 32-bit platforms these days, which is the only reason I can think
>> of why this was never reported until it hit a maintenance track.
>> This should have been caught last week at f6a729f3 (Merge branch
>> 'jk/tzoffset-fix', 2016-07-06) when the topic hit 'master' at the
>> latest, and more preferrably it should have already been caught last
>> month at 08ec8c5e (Merge branch 'jk/tzoffset-fix' into next,
>> 2016-06-28).
>> 
>> Those who care about 32-bit builds need to start building and
>> testing 'next' and 'master' regularly, or similar breakages are
>> bound to continue happening X-<.
>> 
>> Volunteers?
> 
> We might eventually see a volunteer or two but that hasn't happened
> yet, at least in the past few days.
> 
> Does Travis CI testing have an option to run our tests on some
> 32-bit platforms?

TravisCI does not support 32-bit platforms natively:
https://github.com/travis-ci/travis-ci/issues/986#issuecomment-124141683

However, there seems to be a way to enter a 32 bit Trusty chroot on 
64 bit Travis via Docker:
https://github.com/travis-ci/travis-ci/issues/5770

@Duy:
You mentioned that you compiled Git on Docker before ($gmane/297963). 
What do you think the chroot approach? Could that work? Would that
be reliable?

@Sid:
Does GitLab CI support 32-bit platforms?

Thanks,
Lars

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> 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).

Unless you, or Peff, performed a thorough analysis whether the dates are
always cut off at 2038 holds true, I am highly doubtful that the previous
tes is robust at all. I certainly only tested on Windows and never
investigated how that 2038 came about. For what I know, it might be a
platform-dependent behavior of strtoul().

> 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(),

It's strtoul(), actually.

> whose property that overflow will always result in LONG_MAX gives the
> robustness of the 2038 test, and needs to be updated.

So I got curious and looked at the man page. It says indeed that strtoul()
returns ULONG_MAX, which happens to translate into a date in the year
2038. It seems that this behavior is standardized:

	http://pubs.opengroup.org/onlinepubs/007908775/xsh/strtoul.html

although it does not say that ANSI C requires that behavior.

I also could not fail to notice that negative values will be parsed and
simply negated, and that return values 0 and ULONG_MAX *can* denote errors
(in which case errno is set, otherwise it is *not* set). Two rather
surprising facts, at least surprising to me, and facts that our code does
not deal with.

Please also note that ULONG_MAX is not required to be either 2^32-1 or
2^64-1. Which means that the 2038 test is really not robust.

> 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.

Yes, I already update that in my topic branch.

Please note, however, that it is much more natural to update yet another
instance of "unsigned long" to "time_t" than having to explain how that
2038 test is affected.

Also note that the 640bit test is very explicit, and hence robust. As a
consequence it would skip the absurd dates on systems switching to
int128_t for time_t.

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

It is not a downside. It is something easily fixed.

> 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).

Happenstance. And I was merely imitating the patch of Peff thar I found on
gmane.

> 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.

My solid reason is that it is utterky unobvious why the magic number 2038
should do the work. You would have to spend quite some time to convince
the average programmer that it is correct.

Contrast that to the 64-bit test.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> I was hoping to hear from you sooner and do v2.9.2 with your t0006
> >> workaround with lazy-prereq changes from Peff (i.e. "Makes sense!"
> >> above), so that you do not have to do two releases in a row (i.e.
> >> skipping v2.9.1 saying "Git for Windows skipped that one because it
> >> was not quite right; this release fixes the issue" in your v2.9.2
> >> announcement).
> >
> > I am sorry that you expected me to be more available. It is a pretty
> > crazy week already (and trying to get a Git for Windows v2.9.1 out of
> > the door after dropping everything else on Tuesday morning added quite
> > a bit to the load), so I am at times unable to even read the Git
> > mailing list.
> 
> Actually these back-and-forth was an attempt to help you reduce the load
> by not having to worry about the t0006 workaround.  Checking your inbox
> would have been quicker than writing another of your own version.

In this instance, you're correct. I made it a habit of not checking the
inbox at all when in the zone, though. Certainly when the time zone
difference virtually guarantee that your work hours and mine do not
overlap (the announcement of Git v2.9.1 arrived around 4h after I logged
off for the day, for example).

> > As I am more concerned with Jeff Hostetler's patch now (the "very
> > verbose porcelain status"; I merged the Pull Request after seeing no
> > comments on his mail, but then Peff provided some good feedback, so
> > now I am not quite certain how to go about it: revert, or wait if the
> > 2nd iteration is acceptable and take that), so I am not immediately
> > releasing any version, anyway.
> 
> As I said, I'd be waiting for a reroll of that to queue on 'pu', but as
> a new feature, it won't appear in any of the v2.9.x releases.

Yes, the feature will appear in some v2.9.x release: in Git for Windows.

Ciao,
Dscho

32-bit Travis, was Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Does Travis CI testing have an option to run our tests on some
> 32-bit platforms?

AFAIR Docker does not support 32-bit, and IIRC that's what Travis uses.

However, it is possible to install a 32-bit toolchain and use that to
compile Git.

It would be even easier with MacOSX because XCode's gcc can target both
architectures. Simply adding the -m32 CFLAG/LDFLAG might be enough.

Sadly, I lack the time to investigate further. Sounds like a fun project.

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Andreas Schwab]

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

> So I got curious and looked at the man page. It says indeed that strtoul()
> returns ULONG_MAX, which happens to translate into a date in the year
> 2038.

4294967295 would rather be 2106.

$ date -d @$((0xffffffff))
So 7. Feb 07:28:15 CET 2106

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [ANNOUNCE] Git v2.9.1

[Jeff King]

tl;dr I don't really care which fix goes in. They are both fine with me,
and in practice I cannot imagine either causing a big problem. But here
are my thoughts because I know you want them.

On Thu, Jul 14, 2016 at 09:45:12AM +0200, Johannes Schindelin wrote:

> > Hmm, sorry, I do not see much upside here (iow, the 2038 test you
> > came up with is as robust).
> 
> Unless you, or Peff, performed a thorough analysis whether the dates are
> always cut off at 2038 holds true, I am highly doubtful that the previous
> tes is robust at all. I certainly only tested on Windows and never
> investigated how that 2038 came about. For what I know, it might be a
> platform-dependent behavior of strtoul().

I think that when a long is 32-bit signed, you will always get 2038 from
strtol.  There could be systems where that is the case, though, and
time_t is of a different size. I'm not sure how much it would be worth
caring about them.

One nice thing about looking for "if we got 2038, we know we can skip"
as opposed to "did we correctly format this to 2286" is that we err on
the side of failing the test. So if we did ever find such an oddball
platform, the test would fail and we could address it then.

> > 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(),
> 
> It's strtoul(), actually.

I think both you and Junio are mistaken in the quoted text. :)

The code in question is in t/helper/test-date.c:show_dates(), and _does_
call the signed strtol(). However, it is storing it not in an "unsigned
long" (which would be utterly silly), but in a time_t.

And the value is clamped to LONG_MAX there, so the representation
elsewhere does not matter at all, as long as it big enough to store
LONG_MAX. By definition, "unsigned long" should be. In practice, I'd
guess time_t is, though perhaps one could come up with a case of
compiling a 64-bit program against a 32-bit ABI? I don't know if that's
possible.

That also explains why we get 2038 here, and not our usual sentinel
value of "(time_t)0". We _do_ have overflow checks when formatting
pretty-printed dates from commits (see show_ident_date), but the test
helper isn't using them.

> Please also note that ULONG_MAX is not required to be either 2^32-1 or
> 2^64-1. Which means that the 2038 test is really not robust.

Of course not; but as I mentioned above, I think the test can be written
to complain in the unlikely case that it is not one of those, and we can
deal with it then.

> Also note that the 640bit test is very explicit, and hence robust. As a
> consequence it would skip the absurd dates on systems switching to
> int128_t for time_t.

Actually, I think that is a bad thing. The case that the test in
question was added for was not about overflowing "unsigned long", but
about having a far-future date that tm_to_time_t() could not handle. And
that maxes out at 2100. Testing it on a 128-bit system would be
completely appropriate.

-Peff

Re: 32-bit Travis, was Re: [ANNOUNCE] Git v2.9.1

[Mike Hommey]

On Thu, Jul 14, 2016 at 09:58:45AM +0200, Johannes Schindelin wrote:
> Hi Junio,
> 
> On Wed, 13 Jul 2016, Junio C Hamano wrote:
> 
> > Does Travis CI testing have an option to run our tests on some
> > 32-bit platforms?
> 
> AFAIR Docker does not support 32-bit, and IIRC that's what Travis uses.
> 
> However, it is possible to install a 32-bit toolchain and use that to
> compile Git.

You just need to install gcc-multilib on travis, and you can use -m32. I
did that for jemalloc recently.
See https://github.com/jemalloc/jemalloc/blob/dev/.travis.yml

Mike

Re: 32-bit Travis, was Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Mike,

On Thu, 14 Jul 2016, Mike Hommey wrote:

> On Thu, Jul 14, 2016 at 09:58:45AM +0200, Johannes Schindelin wrote:
> > Hi Junio,
> > 
> > On Wed, 13 Jul 2016, Junio C Hamano wrote:
> > 
> > > Does Travis CI testing have an option to run our tests on some
> > > 32-bit platforms?
> > 
> > AFAIR Docker does not support 32-bit, and IIRC that's what Travis uses.
> > 
> > However, it is possible to install a 32-bit toolchain and use that to
> > compile Git.
> 
> You just need to install gcc-multilib on travis, and you can use -m32. I
> did that for jemalloc recently.
> See https://github.com/jemalloc/jemalloc/blob/dev/.travis.yml

Would we not also need

	apt:
		packages:
			lib32z1

(or ia32libs in case of an older Ubuntu)?

Ciao,
Dscho

Re: [ANNOUNCE] Git v2.9.1

[Johannes Schindelin]

Hi Junio,

On Wed, 13 Jul 2016, Junio C Hamano wrote:

> Moving of [64BIT_TIME] 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).

Turns out we *already* have another test in `master` that needs this lazy
prereq:

https://github.com/git/git/blob/79ed43c28f/t/t5000-tar-tree.sh#L378-L384

Ciao,
Dscho

Re: 32-bit Travis, was Re: [ANNOUNCE] Git v2.9.1

[Mike Hommey]

On Thu, Jul 14, 2016 at 12:58:47PM +0200, Johannes Schindelin wrote:
> Hi Mike,
> 
> On Thu, 14 Jul 2016, Mike Hommey wrote:
> 
> > On Thu, Jul 14, 2016 at 09:58:45AM +0200, Johannes Schindelin wrote:
> > > Hi Junio,
> > > 
> > > On Wed, 13 Jul 2016, Junio C Hamano wrote:
> > > 
> > > > Does Travis CI testing have an option to run our tests on some
> > > > 32-bit platforms?
> > > 
> > > AFAIR Docker does not support 32-bit, and IIRC that's what Travis uses.
> > > 
> > > However, it is possible to install a 32-bit toolchain and use that to
> > > compile Git.
> > 
> > You just need to install gcc-multilib on travis, and you can use -m32. I
> > did that for jemalloc recently.
> > See https://github.com/jemalloc/jemalloc/blob/dev/.travis.yml
> 
> Would we not also need
> 
> 	apt:
> 		packages:
> 			lib32z1
> 
> (or ia32libs in case of an older Ubuntu)?

And probably some libcurl-something-dev:i386 package too. 

Mike

Re: [ANNOUNCE] Git v2.9.1

[Duy Nguyen]

On Thu, Jul 14, 2016 at 9:38 AM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>
> On 13 Jul 2016, at 22:43, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> It is somewhat disturbing that nobody seems to be regularly building
>>> on 32-bit platforms these days, which is the only reason I can think
>>> of why this was never reported until it hit a maintenance track.
>>> This should have been caught last week at f6a729f3 (Merge branch
>>> 'jk/tzoffset-fix', 2016-07-06) when the topic hit 'master' at the
>>> latest, and more preferrably it should have already been caught last
>>> month at 08ec8c5e (Merge branch 'jk/tzoffset-fix' into next,
>>> 2016-06-28).
>>>
>>> Those who care about 32-bit builds need to start building and
>>> testing 'next' and 'master' regularly, or similar breakages are
>>> bound to continue happening X-<.
>>>
>>> Volunteers?
>>
>> We might eventually see a volunteer or two but that hasn't happened
>> yet, at least in the past few days.
>>
>> Does Travis CI testing have an option to run our tests on some
>> 32-bit platforms?
>
> TravisCI does not support 32-bit platforms natively:
> https://github.com/travis-ci/travis-ci/issues/986#issuecomment-124141683
>
> However, there seems to be a way to enter a 32 bit Trusty chroot on
> 64 bit Travis via Docker:
> https://github.com/travis-ci/travis-ci/issues/5770
>
> @Duy:
> You mentioned that you compiled Git on Docker before ($gmane/297963).
> What do you think the chroot approach? Could that work? Would that
> be reliable?

"Docker chroot" is a weird term because they are not the same. If you
can launch a new docker process from travis-ci, I suppose you can use
a docker image with multilib support, then just run 32-bit binaries
and it should work (unless the host kernel is built to only support
64-bit).
-- 
Duy