* [PATCH 1/2] Makefile: re-include cgit.conf in cgit.mk @ 2013-04-01 13:14 john 2013-04-01 13:14 ` [PATCH 2/2] compat/timegm: new compat function for systems lacking timegm() john 0 siblings, 1 reply; 7+ messages in thread From: john @ 2013-04-01 13:14 UTC (permalink / raw) This avoids needed to export every variable that might be used in cgit.mk from the top-level Makefile. Signed-off-by: John Keeping <john at keeping.me.uk> --- cgit.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cgit.mk b/cgit.mk index e1aed63..bf3bbb1 100644 --- a/cgit.mk +++ b/cgit.mk @@ -5,6 +5,8 @@ include Makefile CGIT_PREFIX = ../ +-include $(CGIT_PREFIX)cgit.conf + # The CGIT_* variables are inherited when this file is called from the # main Makefile - they are defined there. -- 1.8.2.540.gf023cfe ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] compat/timegm: new compat function for systems lacking timegm() 2013-04-01 13:14 [PATCH 1/2] Makefile: re-include cgit.conf in cgit.mk john @ 2013-04-01 13:14 ` john 2013-04-01 17:47 ` [PATCH 2/2 v2] " john 0 siblings, 1 reply; 7+ messages in thread From: john @ 2013-04-01 13:14 UTC (permalink / raw) The implementation used here is taken from Samba, since discussions on their mailing list indicate that the "portable version" in timegm(3) does not actually work on Solaris. This commit also adds the infrastructure required for compat functions in CGit itself. Mostly we get everything we need from Git, but timegm is not used in Git so there is no reason for them to carry a compatibility version. Signed-off-by: John Keeping <john at keeping.me.uk> --- I wanted to split this into two patches, one to introduce "compat" infrastructure and one to add timegm.c but I couldn't really see a clean way of doing that and I don't think it's too bad as a single patch. cgit-compat-util.h | 11 ++++++++ cgit.h | 2 +- cgit.mk | 14 ++++++++-- compat/timegm.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 cgit-compat-util.h create mode 100644 compat/timegm.c diff --git a/cgit-compat-util.h b/cgit-compat-util.h new file mode 100644 index 0000000..2573f32 --- /dev/null +++ b/cgit-compat-util.h @@ -0,0 +1,11 @@ +#ifndef CGIT_COMPAT_UTIL_H +#define CGIT_COMPAT_UTIL_H + +#include "git/git-compat-util.h" + +#ifdef NO_TIMEGM +#define timegm cgittimegm +extern time_t cgittimegm(struct tm *tm); +#endif + +#endif diff --git a/cgit.h b/cgit.h index ed5cf14..0a9f7f5 100644 --- a/cgit.h +++ b/cgit.h @@ -2,7 +2,7 @@ #define CGIT_H -#include <git-compat-util.h> +#include "cgit-compat-util.h" #include <cache.h> #include <grep.h> #include <object.h> diff --git a/cgit.mk b/cgit.mk index bf3bbb1..b8c7b68 100644 --- a/cgit.mk +++ b/cgit.mk @@ -15,12 +15,19 @@ $(CGIT_PREFIX)VERSION: force-version -include $(CGIT_PREFIX)VERSION .PHONY: force-version +ifdef NO_TIMEGM + CGIT_COMPAT_CFLAGS += -DNO_TIMEGM + CGIT_COMPAT_OBJ_NAMES += compat/timegm.o +endif + # CGIT_CFLAGS is a separate variable so that we can track it separately # and avoid rebuilding all of Git when these variables change. CGIT_CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"' CGIT_CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"' CGIT_CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"' +CGIT_CFLAGS += $(CGIT_COMPAT_CFLAGS) + ifdef NO_C99_FORMAT CFLAGS += -DNO_C99_FORMAT endif @@ -52,7 +59,7 @@ CGIT_OBJ_NAMES += ui-tag.o CGIT_OBJ_NAMES += ui-tree.o CGIT_OBJ_NAMES += vector.o -CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES)) +CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES) $(CGIT_COMPAT_OBJ_NAMES)) # Only cgit.c reference CGIT_VERSION so we only rebuild its objects when the # version changes. @@ -65,8 +72,11 @@ $(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \ ifeq ($(wildcard $(CGIT_PREFIX).depend),) missing_dep_dirs += $(CGIT_PREFIX).depend endif +ifeq ($(wildcard $(CGIT_PREFIX)compat/.depend),) +missing_dep_dirs += $(CGIT_PREFIX)compat/.depend +endif -$(CGIT_PREFIX).depend: +$(addprefix $(CGIT_PREFIX),.depend compat/.depend): @mkdir -p $@ $(CGIT_PREFIX)CGIT-CFLAGS: FORCE diff --git a/compat/timegm.c b/compat/timegm.c new file mode 100644 index 0000000..1491538 --- /dev/null +++ b/compat/timegm.c @@ -0,0 +1,78 @@ +/* + * Copyright (c) 1997 Kungliga Tekniska H?gskolan + * (Royal Institute of Technology, Stockholm, Sweden). + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * 3. Neither the name of the Institute nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE INSTITUTE AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE INSTITUTE OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* + * Adapted for Samba4 by Andrew Tridgell. + * Adapted for CGit by John Keeping. + */ + +#include <time.h> + +static int is_leap(unsigned y) +{ + y += 1900; + return (y % 4) == 0 && ((y % 100) != 0 || (y % 400) == 0); +} + +time_t cgittimegm(struct tm *tm) +{ + static const unsigned ndays[2][12] ={ + {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}, + {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}}; + time_t res = 0; + unsigned i; + + if (tm->tm_mon > 12 || + tm->tm_mon < 0 || + tm->tm_mday > 31 || + tm->tm_min > 60 || + tm->tm_sec > 60 || + tm->tm_hour > 24) { + /* invalid tm structure */ + return 0; + } + + for (i = 70; i < tm->tm_year; ++i) + res += is_leap(i) ? 366 : 365; + + for (i = 0; i < tm->tm_mon; ++i) + res += ndays[is_leap(tm->tm_year)][i]; + res += tm->tm_mday - 1; + res *= 24; + res += tm->tm_hour; + res *= 60; + res += tm->tm_min; + res *= 60; + res += tm->tm_sec; + return res; +} -- 1.8.2.540.gf023cfe ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] compat/timegm: new compat function for systems lacking timegm() 2013-04-01 13:14 ` [PATCH 2/2] compat/timegm: new compat function for systems lacking timegm() john @ 2013-04-01 17:47 ` john 2013-04-04 14:13 ` Jason 0 siblings, 1 reply; 7+ messages in thread From: john @ 2013-04-01 17:47 UTC (permalink / raw) The implementation used here is taken from Samba, since discussions on their mailing list indicate that the "portable version" in timegm(3) does not actually work on Solaris. This commit also adds the infrastructure required for compat functions in CGit itself. Mostly we get everything we need from Git, but timegm is not used in Git so there is no reason for them to carry a compatibility version. Signed-off-by: John Keeping <john at keeping.me.uk> --- Sorry, the previous version of this used "ifdef" in a Makefile, which is clearly rubbish. This version uses "ifneq" like it should have done to begin with. cgit-compat-util.h | 11 ++++++++ cgit.h | 2 +- cgit.mk | 14 ++++++++-- compat/timegm.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 cgit-compat-util.h create mode 100644 compat/timegm.c diff --git a/cgit-compat-util.h b/cgit-compat-util.h new file mode 100644 index 0000000..2573f32 --- /dev/null +++ b/cgit-compat-util.h @@ -0,0 +1,11 @@ +#ifndef CGIT_COMPAT_UTIL_H +#define CGIT_COMPAT_UTIL_H + +#include "git/git-compat-util.h" + +#ifdef NO_TIMEGM +#define timegm cgittimegm +extern time_t cgittimegm(struct tm *tm); +#endif + +#endif diff --git a/cgit.h b/cgit.h index ed5cf14..0a9f7f5 100644 --- a/cgit.h +++ b/cgit.h @@ -2,7 +2,7 @@ #define CGIT_H -#include <git-compat-util.h> +#include "cgit-compat-util.h" #include <cache.h> #include <grep.h> #include <object.h> diff --git a/cgit.mk b/cgit.mk index bf3bbb1..aa2f344 100644 --- a/cgit.mk +++ b/cgit.mk @@ -15,12 +15,19 @@ $(CGIT_PREFIX)VERSION: force-version -include $(CGIT_PREFIX)VERSION .PHONY: force-version +ifneq ($(NO_TIMEGM),) + CGIT_COMPAT_CFLAGS += -DNO_TIMEGM + CGIT_COMPAT_OBJ_NAMES += compat/timegm.o +endif + # CGIT_CFLAGS is a separate variable so that we can track it separately # and avoid rebuilding all of Git when these variables change. CGIT_CFLAGS += -DCGIT_CONFIG='"$(CGIT_CONFIG)"' CGIT_CFLAGS += -DCGIT_SCRIPT_NAME='"$(CGIT_SCRIPT_NAME)"' CGIT_CFLAGS += -DCGIT_CACHE_ROOT='"$(CACHE_ROOT)"' +CGIT_CFLAGS += $(CGIT_COMPAT_CFLAGS) + ifdef NO_C99_FORMAT CFLAGS += -DNO_C99_FORMAT endif @@ -52,7 +59,7 @@ CGIT_OBJ_NAMES += ui-tag.o CGIT_OBJ_NAMES += ui-tree.o CGIT_OBJ_NAMES += vector.o -CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES)) +CGIT_OBJS := $(addprefix $(CGIT_PREFIX),$(CGIT_OBJ_NAMES) $(CGIT_COMPAT_OBJ_NAMES)) # Only cgit.c reference CGIT_VERSION so we only rebuild its objects when the # version changes. @@ -65,8 +72,11 @@ $(CGIT_VERSION_OBJS): EXTRA_CPPFLAGS = \ ifeq ($(wildcard $(CGIT_PREFIX).depend),) missing_dep_dirs += $(CGIT_PREFIX).depend endif +ifeq ($(wildcard $(CGIT_PREFIX)compat/.depend),) +missing_dep_dirs += $(CGIT_PREFIX)compat/.depend +endif -$(CGIT_PREFIX).depend: +$(addprefix $(CGIT_PREFIX),.depend compat/.depend): @mkdir -p $@ $(CGIT_PREFIX)CGIT-CFLAGS: FORCE diff --git a/compat/timegm.c b/compat/timegm.c new file mode 100644 index 0000000..1491538 --- /dev/null +++ b/compat/timegm.c @@ -0,0 +1,78 @@ +/* + * Copyright (c) 1997 Kungliga Tekniska H?gskolan + * (Royal Institute of Technology, Stockholm, Sweden). + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * 3. Neither the name of the Institute nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE INSTITUTE AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE INSTITUTE OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* + * Adapted for Samba4 by Andrew Tridgell. + * Adapted for CGit by John Keeping. + */ + +#include <time.h> + +static int is_leap(unsigned y) +{ + y += 1900; + return (y % 4) == 0 && ((y % 100) != 0 || (y % 400) == 0); +} + +time_t cgittimegm(struct tm *tm) +{ + static const unsigned ndays[2][12] ={ + {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}, + {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}}; + time_t res = 0; + unsigned i; + + if (tm->tm_mon > 12 || + tm->tm_mon < 0 || + tm->tm_mday > 31 || + tm->tm_min > 60 || + tm->tm_sec > 60 || + tm->tm_hour > 24) { + /* invalid tm structure */ + return 0; + } + + for (i = 70; i < tm->tm_year; ++i) + res += is_leap(i) ? 366 : 365; + + for (i = 0; i < tm->tm_mon; ++i) + res += ndays[is_leap(tm->tm_year)][i]; + res += tm->tm_mday - 1; + res *= 24; + res += tm->tm_hour; + res *= 60; + res += tm->tm_min; + res *= 60; + res += tm->tm_sec; + return res; +} -- 1.8.2.540.gf023cfe ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] compat/timegm: new compat function for systems lacking timegm() 2013-04-01 17:47 ` [PATCH 2/2 v2] " john @ 2013-04-04 14:13 ` Jason 2013-04-04 14:46 ` john 0 siblings, 1 reply; 7+ messages in thread From: Jason @ 2013-04-04 14:13 UTC (permalink / raw) Hi John, Is this strictly necessary? Would we be perhaps better off working around gmtime by using an alternative function all together? Or, if we do require a compat version, the man page suggests this much simpler alternative: > For a portable version of timegm(), set the TZ environment variable to UTC, call mktime(3) and restore the value of TZ. Something like > #include <time.h> > #include <stdlib.h> > time_t > my_timegm(struct tm *tm) > { > time_t ret; > char *tz; > tz = getenv("TZ"); > setenv("TZ", "", 1); > tzset(); > ret = mktime(tm); > if (tz) > setenv("TZ", tz, 1); > else > unsetenv("TZ"); > tzset(); > return ret; > } This might not be entirely safe, were cgit to ever become multi-threaded, but it's certainly a lot simpler than rolling our own. Jason ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] compat/timegm: new compat function for systems lacking timegm() 2013-04-04 14:13 ` Jason @ 2013-04-04 14:46 ` john 2013-04-04 14:59 ` Jason 0 siblings, 1 reply; 7+ messages in thread From: john @ 2013-04-04 14:46 UTC (permalink / raw) On Thu, Apr 04, 2013 at 04:13:04PM +0200, Jason A. Donenfeld wrote: > Is this strictly necessary? Would we be perhaps better off working > around gmtime by using an alternative function all together? Or, if > we do require a compat version, the man page suggests this much > simpler alternative: > > > For a portable version of timegm(), set the TZ environment variable to UTC, call mktime(3) and restore the value of TZ. Something like > > > #include <time.h> > > #include <stdlib.h> > > time_t > > my_timegm(struct tm *tm) > > { > > time_t ret; > > char *tz; > > tz = getenv("TZ"); > > setenv("TZ", "", 1); > > tzset(); > > ret = mktime(tm); > > if (tz) > > setenv("TZ", tz, 1); > > else > > unsetenv("TZ"); > > tzset(); > > return ret; > > } > > > This might not be entirely safe, were cgit to ever become > multi-threaded, but it's certainly a lot simpler than rolling our own. I decided not to do that based on this message: http://lists.samba.org/archive/samba-technical/2002-November/025578.html but of course it doesn't provide any actual reason for why that version doesn't work. We only use timegm in week calculations, where we do: time_t t = timegm(tm); t = <new value>; gmtime_r(&t, tm); so I wonder whether it would be equivalent to use mktime/localtime_r instead of timegm/gmtime_r. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] compat/timegm: new compat function for systems lacking timegm() 2013-04-04 14:46 ` john @ 2013-04-04 14:59 ` Jason 2013-04-04 17:28 ` john 0 siblings, 1 reply; 7+ messages in thread From: Jason @ 2013-04-04 14:59 UTC (permalink / raw) On Thu, Apr 4, 2013 at 4:46 PM, John Keeping <john at keeping.me.uk> wrote: > > I decided not to do that based on this message: > > > http://lists.samba.org/archive/samba-technical/2002-November/025578.html > > but of course it doesn't provide any actual reason for why that version > doesn't work. > Aye yie yie. Time handling functions are a mess. I just went looking at how glibc [1] and uclibc [2] actually go about doing things, and it's a real mess in there. Looks like the replacement function in the patch might not be completely correct. [1] http://git.uclibc.org/uClibc/tree/libc/misc/time/time.c#n2330 [2] http://sourceware.org/git/?p=glibc.git;a=blob;f=time/mktime.c;h=e75132c2e457af4b5629eef302c0d6dac15fef5c;hb=HEAD#l348 (uclibc gets bonus points for using cgit) > > We only use timegm in week calculations, where we do: > > time_t t = timegm(tm); > t = <new value>; > gmtime_r(&t, tm); > > so I wonder whether it would be equivalent to use mktime/localtime_r > instead of timegm/gmtime_r. > gmtime(timegm(tm)) == tm and localtime(mktime(tm)) == tm So I can't imagine there'd be a problem with just replacing those functions. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] compat/timegm: new compat function for systems lacking timegm() 2013-04-04 14:59 ` Jason @ 2013-04-04 17:28 ` john 0 siblings, 0 replies; 7+ messages in thread From: john @ 2013-04-04 17:28 UTC (permalink / raw) On Thu, Apr 04, 2013 at 04:59:50PM +0200, Jason A. Donenfeld wrote: > On Thu, Apr 4, 2013 at 4:46 PM, John Keeping <john at keeping.me.uk> wrote: > > > > I decided not to do that based on this message: > > > > > > http://lists.samba.org/archive/samba-technical/2002-November/025578.html > > > > but of course it doesn't provide any actual reason for why that version > > doesn't work. > > > > Aye yie yie. Time handling functions are a mess. I just went looking at how > glibc [1] and uclibc [2] actually go about doing things, and it's a real > mess in there. Looks like the replacement function in the patch might not > be completely correct. > > [1] http://git.uclibc.org/uClibc/tree/libc/misc/time/time.c#n2330 > [2] > http://sourceware.org/git/?p=glibc.git;a=blob;f=time/mktime.c;h=e75132c2e457af4b5629eef302c0d6dac15fef5c;hb=HEAD#l348 > (uclibc gets bonus points for using cgit) > > > > > We only use timegm in week calculations, where we do: > > > > time_t t = timegm(tm); > > t = <new value>; > > gmtime_r(&t, tm); > > > > so I wonder whether it would be equivalent to use mktime/localtime_r > > instead of timegm/gmtime_r. > > > > > gmtime(timegm(tm)) == tm > > and > > localtime(mktime(tm)) == tm > > > So I can't imagine there'd be a problem with just replacing those functions. Actually, there is a potential for a difference here, since gmtime/timegm will not have daylight savings adjustments but localtime/mktim might. I suspect this means that the calculation we do of adding/subtracting a week from the time could end up being an hour out after converting back to struct tm. I'm not sure what the best way to tackle this is, especially if there is not a simple, correct replacement function that we can use. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-04 17:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-01 13:14 [PATCH 1/2] Makefile: re-include cgit.conf in cgit.mk john 2013-04-01 13:14 ` [PATCH 2/2] compat/timegm: new compat function for systems lacking timegm() john 2013-04-01 17:47 ` [PATCH 2/2 v2] " john 2013-04-04 14:13 ` Jason 2013-04-04 14:46 ` john 2013-04-04 14:59 ` Jason 2013-04-04 17:28 ` john
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 NNTP newsgroup(s).