From: Dongsheng Song <dongsheng.song@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: timezone related bug of git
Date: Mon, 1 Nov 2021 22:31:18 +0800 [thread overview]
Message-ID: <CAE8XmWoHpESNK9QJ3d0u0x+Q-OfJEu7117rmV9YZpQjv7JF5ZQ@mail.gmail.com> (raw)
In-Reply-To: <YX9nLJZXB3rOrMru@coredump.intra.peff.net>
Hi Jeff,
Your patch looks good to me. Here is my test result:
$ patch -p1 < x.patch
patching file cache.h
patching file date.c
patching file strbuf.c
$ make
$ ~/git-testing/bin/git --version
$ ~/git-testing/bin/git log 11990eba -1 --date=format:%s
commit 11990eba0be50d1ad0655ede4062b7130326c41f (HEAD -> trunk,
origin/trunk, origin/HEAD)
Author: rillig <rillig@NetBSD.org>
Date: 1635633678
indent: move debugging functions to a separate section
$ ~/git-testing/bin/git cat-file -p 11990eba
tree 5d62150f5e2bafd3db76641450ca5d902302a039
parent 892557a74bd49983fac28366b772b53c9216ca73
author rillig <rillig@NetBSD.org> 1635633678 +0000
committer rillig <rillig@NetBSD.org> 1635633678 +0000
indent: move debugging functions to a separate section
PS:
Thanks Junio for clarification, Jeff's explanation is very clear,
I'm just frustrated with the limitations of the system, I greatly
appreciate Jeff's work.
Best regards,
Dongsheng Song
On Mon, Nov 1, 2021 at 12:03 PM Jeff King <peff@peff.net> wrote:
>
> On Sun, Oct 31, 2021 at 11:46:40AM -0700, Junio C Hamano wrote:
>
> > Dongsheng Song <dongsheng.song@gmail.com> writes:
> >
> > > Thank you for the clarification, it's really a disappointing answer.
> >
> > The situation may be disappointing, but I found the answer eminently
> > clear and helpful.
>
> The most disappointing thing IMHO is the lousy state of system-level
> date routines. ;)
>
> I have some patches working towards allowing timestamps before 1970, and
> the system routines are quite unreliable (both in giving insufficient
> portable interfaces, but also just doing weird things with negative
> values).
>
> > > Perhaps the manual needs to be clearer about this limitation.
> >
> > Sounds like we have a volunteer ;-)?
>
> Yeah, I'd be happy if somebody wanted to note this in the manual. But if
> anybody wants to pursue manually intercepting %s, I think the patch
> below might point them in the right direction.
>
> I won't be at all surprised if it has funny corner cases. Our
> tm_to_time_t() is pretty basic and hacky. We can't use mktime() because
> it only handles the current system timezone. OTOH, I think the tz_offset
> we're undoing here originally came from comparing mktime() versus
> tm_to_time_t() via local_time_tzoffset(), so it could be cancelling out
> any bugs exactly. :)
>
> So maybe the code below is sufficient, but we'd probably at least want
> some tests on top. Maybe something somebody interested would like to
> pick up and run with?
>
> ---
> diff --git a/cache.h b/cache.h
> index eba12487b9..aa6f380d10 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1588,6 +1588,7 @@ timestamp_t approxidate_careful(const char *, int *);
> timestamp_t approxidate_relative(const char *date);
> void parse_date_format(const char *format, struct date_mode *mode);
> int date_overflows(timestamp_t date);
> +time_t tm_to_time_t(const struct tm *tm);
>
> #define IDENT_STRICT 1
> #define IDENT_NO_DATE 2
> diff --git a/date.c b/date.c
> index c55ea47e96..84bb4451c1 100644
> --- a/date.c
> +++ b/date.c
> @@ -9,7 +9,7 @@
> /*
> * This is like mktime, but without normalization of tm_wday and tm_yday.
> */
> -static time_t tm_to_time_t(const struct tm *tm)
> +time_t tm_to_time_t(const struct tm *tm)
> {
> static const int mdays[] = {
> 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
> diff --git a/strbuf.c b/strbuf.c
> index b22e981655..8b8b1900bc 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -1019,6 +1019,13 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,
> strbuf_addstr(&munged_fmt, "%%");
> fmt++;
> break;
> + case 's':
> + strbuf_addf(&munged_fmt, "%"PRItime,
> + tm_to_time_t(tm) -
> + 3600 * (tz_offset / 100) -
> + 60 * (tz_offset % 100));
> + fmt++;
> + break;
> case 'z':
> strbuf_addf(&munged_fmt, "%+05d", tz_offset);
> fmt++;
next prev parent reply other threads:[~2021-11-01 14:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-31 3:23 timezone related bug of git Dongsheng Song
2021-10-31 8:53 ` Jeff King
2021-10-31 13:18 ` Dongsheng Song
2021-10-31 18:46 ` Junio C Hamano
2021-11-01 4:03 ` Jeff King
2021-11-01 14:31 ` Dongsheng Song [this message]
2021-11-01 18:18 ` Junio C Hamano
2021-11-02 1:43 ` Jeff King
2021-11-02 11:35 ` [PATCH] strbuf_addftime(): handle "%s" manually Jeff King
2021-11-02 15:43 ` Jeff King
2021-11-03 20:28 ` Junio C Hamano
2021-11-04 2:11 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAE8XmWoHpESNK9QJ3d0u0x+Q-OfJEu7117rmV9YZpQjv7JF5ZQ@mail.gmail.com \
--to=dongsheng.song@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).