Re: [GSoC][PATCHl 5/6] rebase -i: support --ignore-date

[Junio C Hamano <gitster@pobox.com>]

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

> On Tue, 6 Aug 2019, Rohit Ashiwal wrote:
>
>> @@ -1046,6 +1066,8 @@ static int run_git_commit(struct repository *r,
>>  		argv_array_push(&cmd.args, "--amend");
>>  	if (opts->gpg_sign)
>>  		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
>> +	if (opts->ignore_date)
>> +		argv_array_pushf(&cmd.args, "--date=%ld", time(NULL));
>>  	if (defmsg)
>>  		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
>>  	else if (!(flags & EDIT_MSG))
>
> I need this patch to make the code _at least_ compile on Windows again
> (I don't know whether it breaks the test suite yet):
>
> -- snipsnap --
> Subject: [PATCH] fixup! rebase -i: support --ignore-date
>
> It is a mistake to believe that the return value of `time()` is always
> an `unsigned long`.

Good catch.  We can at least expect it to be some integral type ;-)

With or without this fix-up, I think the patch is still not quite
right.  Output from time() formatted as an integer is a string of
digits, and the side that reads that string and interprets it as a
timestamp does so by calling parse_date(); it is up to that function
to decide what datestring format it is in, and it does not
necessarily take it as seconds since epoch.  It is safer to force
the "seconds since epoch" interpretation by writing the timestamp
string like so:

	argv_array_pushf(&args, "--date=@%ld", time(NULL));

See 14ac2864 ("commit: accept more date formats for "--date"",
2014-05-01), which gives a good hint on how to do this right, and
2c733fb2 ("parse_date(): '@' prefix forces git-timestamp",
2012-02-02) for a backstory.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  sequencer.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 539c0ef601b..a4c932d3407 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1070,7 +1070,8 @@ static int run_git_commit(struct repository *r,
>  	if (opts->gpg_sign)
>  		argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
>  	if (opts->ignore_date)
> -		argv_array_pushf(&cmd.args, "--date=%ld", time(NULL));
> +		argv_array_pushf(&cmd.args, "--date=%"PRIuMAX,
> +				 (uintmax_t)time(NULL));
>  	if (defmsg)
>  		argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
>  	else if (!(flags & EDIT_MSG))
> @@ -3642,7 +3643,8 @@ static int do_merge(struct repository *r,
>  			argv_array_push(&cmd.args, opts->gpg_sign);
>  		if (opts->ignore_date)
>  			argv_array_pushf(&cmd.args,
> -					 "GIT_AUTHOR_DATE=%ld", time(NULL));
> +					 "GIT_AUTHOR_DATE=%"PRIuMAX,
> +					 (uintmax_t)time(NULL));
>
>  		/* Add the tips to be merged */
>  		for (j = to_merge; j; j = j->next)
> --
> 2.22.0.windows.1.6.g271c090e89