Hi Junio and Dscho
On Thu, Aug 8, 2019 at 1:52 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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@xxxxxx>
> > ---
> > 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
Thanks for suggestions, I'll incorporate these changes along
with changes suggested by Junio and re-send the patch.
- Rohit
