* [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
0 siblings, 0 replies; 24+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen
This hasn't been a problem in practice as almost all systems have the
setitimer() API (or it is provided by git in the case of mingw). This code
wasn't used in any default circumstances, as the build system never sets
NO_STRUCT_ITIMERVAL - this breakage only occured if the user asked for it.
We repair this case so we can rely on it in the following commits.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
git-compat-util.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749..f7fae20 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -191,7 +191,7 @@ extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);
struct itimerval {
struct timeval it_interval;
struct timeval it_value;
-}
+};
#endif
#ifdef NO_SETITIMER
--
2.1.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
@ 2014-08-29 16:42 Jacob Keller
2014-08-29 16:42 ` [PATCH 2/9] autoconf: Check for " Jacob Keller
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This hasn't been a problem in practice as almost all systems have the
setitimer() API (or it is provided by git in the case of mingw). This code
wasn't used in any default circumstances, as the build system never sets
NO_STRUCT_ITIMERVAL - this breakage only occured if the user asked for it.
We repair this case so we can rely on it in the following commits.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
git-compat-util.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index f587749b7cf6..f7fae2060771 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -191,7 +191,7 @@ extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);
struct itimerval {
struct timeval it_interval;
struct timeval it_value;
-}
+};
#endif
#ifdef NO_SETITIMER
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/9] autoconf: Check for struct itimerval
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 16:42 ` [PATCH 3/9] autoconf: Check for setitimer Jacob Keller
` (7 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen, Jacob Keller
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
The makefile has provisions for this case, so let's detect it in
the configure script as well.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
configure.ac | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/configure.ac b/configure.ac
index 4b1ae7c3c9f5..652bfdddb2a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -746,6 +746,14 @@ case $ac_cv_type_socklen_t in
esac
GIT_CONF_SUBST([SOCKLEN_T])
+#
+# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval.
+AC_CHECK_TYPES([struct itimerval],
+[NO_STRUCT_ITIMERVAL=],
+[NO_STRUCT_ITIMERVAL=UnfortunatelyYes],
+[#include <sys/time.h>])
+GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
+#
# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
AC_CHECK_MEMBER(struct dirent.d_ino,
[NO_D_INO_IN_DIRENT=],
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] autoconf: Check for setitimer
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
2014-08-29 16:42 ` [PATCH 2/9] autoconf: Check for " Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 16:42 ` [PATCH 4/9] autoconf: Check for timer_t Jacob Keller
` (6 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
The makefile has provisions for this case, so let's detect it in the
configure script as well.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
configure.ac | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/configure.ac b/configure.ac
index 652bfdddb2a9..6af964797f7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -911,6 +911,12 @@ AC_CHECK_LIB([iconv], [locale_charset],
[CHARSET_LIB=-lcharset])])
GIT_CONF_SUBST([CHARSET_LIB])
#
+# Define NO_SETITIMER if you don't have setitimer.
+GIT_CHECK_FUNC(setitimer,
+[NO_SETITIMER=],
+[NO_SETITIMER=YesPlease])
+GIT_CONF_SUBST([NO_SETITIMER])
+#
# Define NO_STRCASESTR if you don't have strcasestr.
GIT_CHECK_FUNC(strcasestr,
[NO_STRCASESTR=],
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] autoconf: Check for timer_t
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
2014-08-29 16:42 ` [PATCH 2/9] autoconf: Check for " Jacob Keller
2014-08-29 16:42 ` [PATCH 3/9] autoconf: Check for setitimer Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 18:20 ` Jonas 'Sortie' Termansen
2014-08-29 16:42 ` [PATCH 5/9] autoconf: Check for struct timespec Jacob Keller
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen, Jacob Keller
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This type will be used in a following commit.
This type was not previously used by git. This can cause trouble for
people on systems without timer_t if they only rely on config.mak.uname.
They will need to set NO_TIMER_T manually.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Makefile | 5 +++++
config.mak.uname | 2 ++
configure.ac | 8 ++++++++
git-compat-util.h | 4 ++++
4 files changed, 19 insertions(+)
diff --git a/Makefile b/Makefile
index 9f984a9e5561..54266fd77eab 100644
--- a/Makefile
+++ b/Makefile
@@ -182,6 +182,8 @@ all::
#
# Define NO_SETITIMER if you don't have setitimer()
#
+# Define NO_TIMER_T if you don't have timer_t.
+#
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
# This also implies NO_SETITIMER
#
@@ -1338,6 +1340,9 @@ endif
ifdef OBJECT_CREATION_USES_RENAMES
COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1
endif
+ifdef NO_TIMER_T
+ COMPAT_CFLAGS += -DNO_TIMER_T
+endif
ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
NO_SETITIMER = YesPlease
diff --git a/config.mak.uname b/config.mak.uname
index 15ee15e98c2c..a5297a242561 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -355,6 +355,7 @@ ifeq ($(uname_S),Windows)
NATIVE_CRLF = YesPlease
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
+ NO_TIMER_T = UnfortunatelyYes
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -504,6 +505,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
+ NO_TIMER_T = UnfortunatelyYes
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
diff --git a/configure.ac b/configure.ac
index 6af964797f7b..126af58365fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -754,6 +754,14 @@ AC_CHECK_TYPES([struct itimerval],
[#include <sys/time.h>])
GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
#
+# Define NO_TIMER_T if you don't have timer_t.
+AC_CHECK_TYPES([timer_t],
+[NO_TIMER_T=],
+[NO_TIMER_T=UnfortunatelyYes],
+[#include <sys/types.h>
+ #include <time.h>])
+GIT_CONF_SUBST([NO_TIMER_T])
+#
# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
AC_CHECK_MEMBER(struct dirent.d_ino,
[NO_D_INO_IN_DIRENT=],
diff --git a/git-compat-util.h b/git-compat-util.h
index f7fae2060771..e0e7a62b642a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -187,6 +187,10 @@ typedef unsigned long uintptr_t;
extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);
#endif
+#ifdef NO_TIMER_T
+typedef int timer_t;
+#endif
+
#ifdef NO_STRUCT_ITIMERVAL
struct itimerval {
struct timeval it_interval;
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] autoconf: Check for struct timespec
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (2 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 4/9] autoconf: Check for timer_t Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 16:42 ` [PATCH 6/9] autoconf: Check for struct sigevent Jacob Keller
` (4 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This type will be used in a following commit.
This type was not previously used by git. This can cause trouble for
people on systems without struct timespec if they only rely on
config.mak.uname. They will need to set NO_STRUCT_TIMESPEC manually.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
Makefile | 5 +++++
config.mak.uname | 2 ++
configure.ac | 7 +++++++
git-compat-util.h | 7 +++++++
4 files changed, 21 insertions(+)
diff --git a/Makefile b/Makefile
index 54266fd77eab..0dd3e35327c9 100644
--- a/Makefile
+++ b/Makefile
@@ -184,6 +184,8 @@ all::
#
# Define NO_TIMER_T if you don't have timer_t.
#
+# Define NO_STRUCT_TIMESPEC if you don't have struct timespec
+#
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
# This also implies NO_SETITIMER
#
@@ -1343,6 +1345,9 @@ endif
ifdef NO_TIMER_T
COMPAT_CFLAGS += -DNO_TIMER_T
endif
+ifdef NO_STRUCT_TIMESPEC
+ COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
+endif
ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
NO_SETITIMER = YesPlease
diff --git a/config.mak.uname b/config.mak.uname
index a5297a242561..812179159be2 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -356,6 +356,7 @@ ifeq ($(uname_S),Windows)
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
NO_TIMER_T = UnfortunatelyYes
+ NO_STRUCT_TIMESPEC = UnfortunatelyYes
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -506,6 +507,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
NO_TIMER_T = UnfortunatelyYes
+ NO_STRUCT_TIMESPEC = UnfortunatelyYes
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
diff --git a/configure.ac b/configure.ac
index 126af58365fb..bed8fe9c9590 100644
--- a/configure.ac
+++ b/configure.ac
@@ -762,6 +762,13 @@ AC_CHECK_TYPES([timer_t],
#include <time.h>])
GIT_CONF_SUBST([NO_TIMER_T])
#
+# Define NO_STRUCT_TIMESPEC if you don't have struct timespec.
+AC_CHECK_TYPES([struct timespec],
+[NO_STRUCT_TIMESPEC=],
+[NO_STRUCT_TIMESPEC=UnfortunatelyYes],
+[#include <sys/time.h>])
+GIT_CONF_SUBST([NO_STRUCT_TIMESPEC])
+#
# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
AC_CHECK_MEMBER(struct dirent.d_ino,
[NO_D_INO_IN_DIRENT=],
diff --git a/git-compat-util.h b/git-compat-util.h
index e0e7a62b642a..e9e7e5451a99 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -191,6 +191,13 @@ extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);
typedef int timer_t;
#endif
+#ifdef NO_STRUCT_TIMESPEC
+struct timespec {
+ time_t tv_sec;
+ long tv_nsec;
+};
+#endif
+
#ifdef NO_STRUCT_ITIMERVAL
struct itimerval {
struct timeval it_interval;
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/9] autoconf: Check for struct sigevent
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (3 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 5/9] autoconf: Check for struct timespec Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 16:42 ` [PATCH 7/9] autoconf: Check for struct itimerspec Jacob Keller
` (3 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This type will be used in a following commit.
This type was not previously used by git. This can cause trouble for
people on systems without struct sigevent if they only rely on
config.mak.uname. They will need to set NO_STRUCT_SIGEVENT manually.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
Makefile | 5 +++++
config.mak.uname | 2 ++
configure.ac | 7 +++++++
git-compat-util.h | 12 ++++++++++++
4 files changed, 26 insertions(+)
diff --git a/Makefile b/Makefile
index 0dd3e35327c9..b76dc4385952 100644
--- a/Makefile
+++ b/Makefile
@@ -186,6 +186,8 @@ all::
#
# Define NO_STRUCT_TIMESPEC if you don't have struct timespec
#
+# Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
+#
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
# This also implies NO_SETITIMER
#
@@ -1348,6 +1350,9 @@ endif
ifdef NO_STRUCT_TIMESPEC
COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
endif
+ifdef NO_STRUCT_SIGEVENT
+ COMPAT_CFLAGS += -DNO_STRUCT_SIGEVENT
+endif
ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
NO_SETITIMER = YesPlease
diff --git a/config.mak.uname b/config.mak.uname
index 812179159be2..892afc573c28 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -357,6 +357,7 @@ ifeq ($(uname_S),Windows)
NO_D_INO_IN_DIRENT = YesPlease
NO_TIMER_T = UnfortunatelyYes
NO_STRUCT_TIMESPEC = UnfortunatelyYes
+ NO_STRUCT_SIGEVENT = UnfortunatelyYes
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -508,6 +509,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_D_INO_IN_DIRENT = YesPlease
NO_TIMER_T = UnfortunatelyYes
NO_STRUCT_TIMESPEC = UnfortunatelyYes
+ NO_STRUCT_SIGEVENT = UnfortunatelyYes
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
diff --git a/configure.ac b/configure.ac
index bed8fe9c9590..103b40704b6f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -776,6 +776,13 @@ AC_CHECK_MEMBER(struct dirent.d_ino,
[#include <dirent.h>])
GIT_CONF_SUBST([NO_D_INO_IN_DIRENT])
#
+# Define NO_STRUCT_SIGEVENT if you don't have struct sigevent.
+AC_CHECK_TYPES([struct sigevent],
+[NO_STRUCT_SIGEVENT=],
+[NO_STRUCT_SIGEVENT=UnfortunatelyYes],
+[#include <signal.h>])
+GIT_CONF_SUBST([NO_STRUCT_SIGEVENT])
+#
# Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
# d_type in struct dirent (latest Cygwin -- will be fixed soonish).
AC_CHECK_MEMBER(struct dirent.d_type,
diff --git a/git-compat-util.h b/git-compat-util.h
index e9e7e5451a99..195eda6f1575 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -198,6 +198,18 @@ struct timespec {
};
#endif
+#ifndef SIGEV_SIGNAL
+#define SIGEV_SIGNAL 1 /* dummy */
+#endif
+
+#ifdef NO_STRUCT_SIGEVENT
+struct sigevent /* dummy */
+{
+ int sigev_notify;
+ int sigev_signo;
+};
+#endif
+
#ifdef NO_STRUCT_ITIMERVAL
struct itimerval {
struct timeval it_interval;
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] autoconf: Check for struct itimerspec
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (4 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 6/9] autoconf: Check for struct sigevent Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 16:42 ` [PATCH 8/9] autoconf: Check for timer_settime Jacob Keller
` (2 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen, Jacob Keller
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This type will be used in a following commit.
This type was not previously used by git. This can cause trouble for
people on systems without struct itimerspec if they only rely on
config.mak.uname. They will need to set NO_STRUCT_ITIMERSPEC manually.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Makefile | 5 +++++
config.mak.uname | 3 +++
configure.ac | 7 +++++++
git-compat-util.h | 7 +++++++
4 files changed, 22 insertions(+)
diff --git a/Makefile b/Makefile
index b76dc4385952..66329e4b372b 100644
--- a/Makefile
+++ b/Makefile
@@ -191,6 +191,8 @@ all::
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
# This also implies NO_SETITIMER
#
+# Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec
+#
# Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
# generally faster on your platform than accessing the working directory.
#
@@ -1357,6 +1359,9 @@ ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
NO_SETITIMER = YesPlease
endif
+ifdef NO_STRUCT_ITIMERSPEC
+ COMPAT_CFLAGS += -DNO_STRUCT_ITIMERSPEC
+endif
ifdef NO_SETITIMER
COMPAT_CFLAGS += -DNO_SETITIMER
endif
diff --git a/config.mak.uname b/config.mak.uname
index 892afc573c28..f0d93ef868a7 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -98,6 +98,7 @@ ifeq ($(uname_S),Darwin)
NO_MEMMEM = YesPlease
USE_ST_TIMESPEC = YesPlease
HAVE_DEV_TTY = YesPlease
+ NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
endif
@@ -358,6 +359,7 @@ ifeq ($(uname_S),Windows)
NO_TIMER_T = UnfortunatelyYes
NO_STRUCT_TIMESPEC = UnfortunatelyYes
NO_STRUCT_SIGEVENT = UnfortunatelyYes
+ NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -510,6 +512,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_TIMER_T = UnfortunatelyYes
NO_STRUCT_TIMESPEC = UnfortunatelyYes
NO_STRUCT_SIGEVENT = UnfortunatelyYes
+ NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
diff --git a/configure.ac b/configure.ac
index 103b40704b6f..954f9ddb03c2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -769,6 +769,13 @@ AC_CHECK_TYPES([struct timespec],
[#include <sys/time.h>])
GIT_CONF_SUBST([NO_STRUCT_TIMESPEC])
#
+# Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec.
+AC_CHECK_TYPES([struct itimerspec],
+[NO_STRUCT_ITIMERSPEC=],
+[NO_STRUCT_ITIMERSPEC=UnfortunatelyYes],
+[#include <time.h>])
+GIT_CONF_SUBST([NO_STRUCT_ITIMERSPEC])
+#
# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
AC_CHECK_MEMBER(struct dirent.d_ino,
[NO_D_INO_IN_DIRENT=],
diff --git a/git-compat-util.h b/git-compat-util.h
index 195eda6f1575..4ef17df86b0e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -217,6 +217,13 @@ struct itimerval {
};
#endif
+#ifdef NO_STRUCT_ITIMERSPEC
+struct itimerspec {
+ struct timespec it_interval;
+ struct timespec it_value;
+};
+#endif
+
#ifdef NO_SETITIMER
#define setitimer(which,value,ovalue)
#endif
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] autoconf: Check for timer_settime
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (5 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 7/9] autoconf: Check for struct itimerspec Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 17:26 ` Johannes Sixt
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
2014-08-29 16:48 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Keller, Jacob E
8 siblings, 1 reply; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen, Jacob Keller
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
This function will be used in a following commit.
The timer_settime function is provided in librt on some systems. We
already use this library sometimes to get clock_gettime, so rework the
logic so we don't link with it twice.
This function was not previously used by git. This can cause trouble for
people on systems without timer_settime if they only rely on
config.mak.uname. They will need to set NO_TIMER_SETTIME manually.
Add proper replacement function macros for setitimer and timer_settime
that implement timer_settime as a wrapper for setitimer. In this way, if
the system has setitimer but not timer_settime then we will be able to
call timer_create, timer_settime, and timer_delete correctly and it will
wrap to setitimer under the hood. This will be used in the following
commit.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Makefile | 21 +++++++++++++++++++++
config.mak.uname | 3 +++
configure.ac | 8 ++++++++
git-compat-util.h | 20 +++++++++++++++++++-
4 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 66329e4b372b..5337ef0b7cd6 100644
--- a/Makefile
+++ b/Makefile
@@ -182,16 +182,22 @@ all::
#
# Define NO_SETITIMER if you don't have setitimer()
#
+# Define NO_TIMER_SETTIME if you don't have timer_settime()
+#
# Define NO_TIMER_T if you don't have timer_t.
+# This also implies NO_TIMER_SETTIME
#
# Define NO_STRUCT_TIMESPEC if you don't have struct timespec
+# This also implies NO_TIMER_SETTIME
#
# Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
+# This also implies NO_TIMER_SETTIME
#
# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
# This also implies NO_SETITIMER
#
# Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec
+# This also implies NO_TIMER_SETTIME
#
# Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
# generally faster on your platform than accessing the working directory.
@@ -1348,12 +1354,15 @@ ifdef OBJECT_CREATION_USES_RENAMES
endif
ifdef NO_TIMER_T
COMPAT_CFLAGS += -DNO_TIMER_T
+ NO_TIMER_SETTIME = YesPlease
endif
ifdef NO_STRUCT_TIMESPEC
COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
+ NO_TIMER_SETTIME = YesPlease
endif
ifdef NO_STRUCT_SIGEVENT
COMPAT_CFLAGS += -DNO_STRUCT_SIGEVENT
+ NO_TIMER_SETTIME = YesPlease
endif
ifdef NO_STRUCT_ITIMERVAL
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
@@ -1361,10 +1370,14 @@ ifdef NO_STRUCT_ITIMERVAL
endif
ifdef NO_STRUCT_ITIMERSPEC
COMPAT_CFLAGS += -DNO_STRUCT_ITIMERSPEC
+ NO_TIMER_SETTIME = YesPlease
endif
ifdef NO_SETITIMER
COMPAT_CFLAGS += -DNO_SETITIMER
endif
+ifdef NO_TIMER_SETTIME
+ COMPAT_CFLAGS += -DNO_TIMER_SETTIME
+endif
ifdef NO_PREAD
COMPAT_CFLAGS += -DNO_PREAD
COMPAT_OBJS += compat/pread.o
@@ -1524,6 +1537,14 @@ endif
ifdef HAVE_CLOCK_GETTIME
BASIC_CFLAGS += -DHAVE_CLOCK_GETTIME
+ LINK_WITH_LIBRT = YesPlease
+endif
+
+ifndef NO_TIMER_SETTIME
+ LINK_WITH_LIBRT = YesPlease
+endif
+
+ifdef LINK_WITH_LIBRT
EXTLIBS += -lrt
endif
diff --git a/config.mak.uname b/config.mak.uname
index f0d93ef868a7..d04deab2dfa8 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -99,6 +99,7 @@ ifeq ($(uname_S),Darwin)
USE_ST_TIMESPEC = YesPlease
HAVE_DEV_TTY = YesPlease
NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
+ NO_TIMER_SETTIME = UnfortunatelyYes
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
endif
@@ -360,6 +361,7 @@ ifeq ($(uname_S),Windows)
NO_STRUCT_TIMESPEC = UnfortunatelyYes
NO_STRUCT_SIGEVENT = UnfortunatelyYes
NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
+ NO_TIMER_SETTIME = UnfortunatelyYes
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -513,6 +515,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_STRUCT_TIMESPEC = UnfortunatelyYes
NO_STRUCT_SIGEVENT = UnfortunatelyYes
NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
+ NO_TIMER_SETTIME = UnfortunatelyYes
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
diff --git a/configure.ac b/configure.ac
index 954f9ddb03c2..9d6ec41acc82 100644
--- a/configure.ac
+++ b/configure.ac
@@ -946,6 +946,14 @@ GIT_CHECK_FUNC(setitimer,
[NO_SETITIMER=YesPlease])
GIT_CONF_SUBST([NO_SETITIMER])
#
+# Define NO_TIMER_SETTIME if you don't have timer_settime
+GIT_CHECK_FUNC(timer_settime,
+[NO_TIMER_SETTIME=],
+[AC_SEARCH_LIBS(timer_settime,[rt],
+ [NO_TIMER_SETTIME=],
+ [NO_TIMER_SETTIME=YesPlease])])
+GIT_CONF_SUBST([NO_TIMER_SETTIME])
+#
# Define NO_STRCASESTR if you don't have strcasestr.
GIT_CHECK_FUNC(strcasestr,
[NO_STRCASESTR=],
diff --git a/git-compat-util.h b/git-compat-util.h
index 4ef17df86b0e..b23602196323 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -225,7 +225,25 @@ struct itimerspec {
#endif
#ifdef NO_SETITIMER
-#define setitimer(which,value,ovalue)
+#define setitimer(which,value,ovalue) ((void) (which), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
+#endif
+
+#ifdef NO_TIMER_SETTIME
+#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
+
+#define timer_delete(timer) do { \
+ struct itimerval v = {{0,},}; \
+ setitimer(ITIMER_REAL, &v, NULL); \
+} while (0)
+
+#define timer_settime(timer, flags, value, ovalue) do { \
+ struct itimerval _ivalue; \
+ _ivalue.it_interval.tv_sec = value.it_interval.tv_sec; \
+ _ivalue.it_interval.tv_usec = value.it_interval.tv_nsec / 1000L; \
+ _ivalue.it_value.tv_sec value.it_value.tv_sec; \
+ _ivalue_it_value.tv_usec = value.it_value.tv_nsec / 1000L; \
+ setitimer(ITIMER_REAL, &_ivalue, NULL); \
+while (0)
#endif
#ifndef NO_LIBGEN_H
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] Use timer_settime for new platforms
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (6 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 8/9] autoconf: Check for timer_settime Jacob Keller
@ 2014-08-29 16:42 ` Jacob Keller
2014-08-29 18:02 ` Junio C Hamano
2014-08-29 18:12 ` Junio C Hamano
2014-08-29 16:48 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Keller, Jacob E
8 siblings, 2 replies; 24+ messages in thread
From: Jacob Keller @ 2014-08-29 16:42 UTC (permalink / raw)
To: git; +Cc: Jonas 'Sortie' Termansen, Jacob Keller
From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
setitimer() is an obsolescent XSI interface and may be removed in a
future standard. Applications should use the core POSIX timer_settime()
instead.
It's important that code doesn't simply check if timer_settime is
available as it can give false positives. Some systems like contemporary
OpenBSD provides the function, but it unconditionally fails with ENOSYS
at runtime.
Clean up the progress reporting and change it to use timer_settime,
which will fall back to setitimer automatically if timer_settime is not
supported. (see git-compat-util.h for how it does this). If both
functions are not present, then git-compat-util.h provides replacements
which will always fail with ENOSYS.
The approach used here enables us to use a single API (timer_settime)
without having to worry about checking for #ifdefs or if blocks which
make it an unreadable nightmare. The major downside is for systems
without timer_settime support, they may fall back on a wrapped
implementation which could have subtle differences. This should be a
minor issue as almost all modern systems provide timer_settime support.
Note that this change means that git should never use setitimer on its
own now, as the fallback implementation of timer_settime assumes that it
is the sole user of ITIMER_REAL, and timer_delete will reset the
ITIMER_REAL.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
builtin/log.c | 27 ++++++++++++++++++++-------
progress.c | 20 +++++++++++++-------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 4389722b4b1e..a39e82d67eb3 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -229,7 +229,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr)
printf(_("Final output: %d %s\n"), nr, stage);
}
-static struct itimerval early_output_timer;
+static timer_t early_output_timer;
static void log_show_early(struct rev_info *revs, struct commit_list *list)
{
@@ -271,9 +271,12 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
* trigger every second even if we're blocked on a
* reader!
*/
- early_output_timer.it_value.tv_sec = 0;
- early_output_timer.it_value.tv_usec = 500000;
- setitimer(ITIMER_REAL, &early_output_timer, NULL);
+ struct itimerspec value;
+ value.it_value.tv_sec = 0;
+ value.it_value.tv_nsec = 500000L * 1000L;
+ value.it_interval.tv_sec = 0;
+ value.it_interval.tv_nsec = 0;
+ timer_settime(early_output_timer, 0, &value, NULL);
}
static void early_output(int signal)
@@ -284,6 +287,7 @@ static void early_output(int signal)
static void setup_early_output(struct rev_info *rev)
{
struct sigaction sa;
+ struct sigevent sev;
/*
* Set up the signal handler, minimally intrusively:
@@ -305,14 +309,23 @@ static void setup_early_output(struct rev_info *rev)
*
* This is a one-time-only trigger.
*/
- early_output_timer.it_value.tv_sec = 0;
- early_output_timer.it_value.tv_usec = 100000;
- setitimer(ITIMER_REAL, &early_output_timer, NULL);
+ memset(&sev, 0, sizeof(sev));
+ sev.sigev_notify = SIGEV_SIGNAL;
+ sev.sigev_signo = SIGALRM;
+ timer_create(CLOCK_MONOTONIC, &sev, &early_output_timer);
+
+ struct itimerspec value;
+ value.it_value.tv_sec = 0;
+ value.it_value.tv_nsec = 100000L * 1000L;
+ value.it_interval.tv_sec = 0;
+ value.it_interval.tv_nsec = 0;
+ timer_settime(early_output_timer, 0, &value, NULL);
}
static void finish_early_output(struct rev_info *rev)
{
int n = estimate_commit_count(rev, rev->commits);
+ timer_delete(early_output_timer);
signal(SIGALRM, SIG_IGN);
show_early_header(rev, "done", n);
}
diff --git a/progress.c b/progress.c
index 412e6b1ecc36..7fe73df35625 100644
--- a/progress.c
+++ b/progress.c
@@ -38,6 +38,7 @@ struct progress {
struct throughput *throughput;
};
+static timer_t progress_timer;
static volatile sig_atomic_t progress_update;
static void progress_interval(int signum)
@@ -48,7 +49,7 @@ static void progress_interval(int signum)
static void set_progress_signal(void)
{
struct sigaction sa;
- struct itimerval v;
+ struct sigevent sev;
progress_update = 0;
@@ -58,16 +59,21 @@ static void set_progress_signal(void)
sa.sa_flags = SA_RESTART;
sigaction(SIGALRM, &sa, NULL);
- v.it_interval.tv_sec = 1;
- v.it_interval.tv_usec = 0;
- v.it_value = v.it_interval;
- setitimer(ITIMER_REAL, &v, NULL);
+ memset(&sev, 0, sizeof(sev));
+ sev.sigev_notify = SIGEV_SIGNAL;
+ sev.sigev_signo = SIGALRM;
+ timer_create(CLOCK_MONOTONIC, &sev, &progress_timer);
+
+ struct itimerspec value;
+ value.it_interval.tv_sec = 1;
+ value.it_interval.tv_nsec = 0;
+ value.it_value = value.it_interval;
+ timer_settime(progress_timer, 0, &value, NULL);
}
static void clear_progress_signal(void)
{
- struct itimerval v = {{0,},};
- setitimer(ITIMER_REAL, &v, NULL);
+ timer_delete(progress_timer);
signal(SIGALRM, SIG_IGN);
progress_update = 0;
}
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
` (7 preceding siblings ...)
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
@ 2014-08-29 16:48 ` Keller, Jacob E
2014-08-29 18:54 ` Jonas 'Sortie' Termansen
8 siblings, 1 reply; 24+ messages in thread
From: Keller, Jacob E @ 2014-08-29 16:48 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: sortie@maxsi.org
On Fri, 2014-08-29 at 09:42 -0700, Jacob Keller wrote:
> From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>
> This hasn't been a problem in practice as almost all systems have the
> setitimer() API (or it is provided by git in the case of mingw). This code
> wasn't used in any default circumstances, as the build system never sets
> NO_STRUCT_ITIMERVAL - this breakage only occured if the user asked for it.
>
> We repair this case so we can rely on it in the following commits.
>
> Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> ---
I fixed up Jonas' series and tried to resolve the issues I found as well
as re-ordered the patches so that they fit what Junio requested. I also
modified the last two patches to make timer_settime fallback to
setitimer. I am not sure this is the best approach, but it should be
easy to take at least the obvious bug fixes from the first three
patches, and this gives an outline for how we could wrap the setitimer
stuff.
One could also write another layer of wrapper in some compat/itimer.c
code, but I don't think this is necessary.
Regards,
Jake
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-08-29 16:42 ` [PATCH 8/9] autoconf: Check for timer_settime Jacob Keller
@ 2014-08-29 17:26 ` Johannes Sixt
2014-08-29 17:40 ` Keller, Jacob E
0 siblings, 1 reply; 24+ messages in thread
From: Johannes Sixt @ 2014-08-29 17:26 UTC (permalink / raw)
To: Jacob Keller, git; +Cc: Jonas 'Sortie' Termansen
Am 29.08.2014 18:42, schrieb Jacob Keller:
> From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>
> This function will be used in a following commit.
>
> The timer_settime function is provided in librt on some systems. We
> already use this library sometimes to get clock_gettime, so rework the
> logic so we don't link with it twice.
>
> This function was not previously used by git. This can cause trouble for
> people on systems without timer_settime if they only rely on
> config.mak.uname. They will need to set NO_TIMER_SETTIME manually.
>
> Add proper replacement function macros for setitimer and timer_settime
> that implement timer_settime as a wrapper for setitimer. In this way, if
> the system has setitimer but not timer_settime then we will be able to
> call timer_create, timer_settime, and timer_delete correctly and it will
> wrap to setitimer under the hood. This will be used in the following
> commit.
>
> Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Makefile | 21 +++++++++++++++++++++
> config.mak.uname | 3 +++
> configure.ac | 8 ++++++++
> git-compat-util.h | 20 +++++++++++++++++++-
> 4 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 66329e4b372b..5337ef0b7cd6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -182,16 +182,22 @@ all::
> #
> # Define NO_SETITIMER if you don't have setitimer()
> #
> +# Define NO_TIMER_SETTIME if you don't have timer_settime()
> +#
> # Define NO_TIMER_T if you don't have timer_t.
> +# This also implies NO_TIMER_SETTIME
> #
> # Define NO_STRUCT_TIMESPEC if you don't have struct timespec
> +# This also implies NO_TIMER_SETTIME
> #
> # Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
> +# This also implies NO_TIMER_SETTIME
> #
> # Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
> # This also implies NO_SETITIMER
> #
> # Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec
> +# This also implies NO_TIMER_SETTIME
> #
> # Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
> # generally faster on your platform than accessing the working directory.
> @@ -1348,12 +1354,15 @@ ifdef OBJECT_CREATION_USES_RENAMES
> endif
> ifdef NO_TIMER_T
> COMPAT_CFLAGS += -DNO_TIMER_T
> + NO_TIMER_SETTIME = YesPlease
> endif
> ifdef NO_STRUCT_TIMESPEC
> COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
> + NO_TIMER_SETTIME = YesPlease
> endif
> ifdef NO_STRUCT_SIGEVENT
> COMPAT_CFLAGS += -DNO_STRUCT_SIGEVENT
> + NO_TIMER_SETTIME = YesPlease
> endif
> ifdef NO_STRUCT_ITIMERVAL
> COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
> @@ -1361,10 +1370,14 @@ ifdef NO_STRUCT_ITIMERVAL
> endif
> ifdef NO_STRUCT_ITIMERSPEC
> COMPAT_CFLAGS += -DNO_STRUCT_ITIMERSPEC
> + NO_TIMER_SETTIME = YesPlease
> endif
> ifdef NO_SETITIMER
> COMPAT_CFLAGS += -DNO_SETITIMER
> endif
> +ifdef NO_TIMER_SETTIME
> + COMPAT_CFLAGS += -DNO_TIMER_SETTIME
> +endif
> ifdef NO_PREAD
> COMPAT_CFLAGS += -DNO_PREAD
> COMPAT_OBJS += compat/pread.o
> @@ -1524,6 +1537,14 @@ endif
>
> ifdef HAVE_CLOCK_GETTIME
> BASIC_CFLAGS += -DHAVE_CLOCK_GETTIME
> + LINK_WITH_LIBRT = YesPlease
> +endif
> +
> +ifndef NO_TIMER_SETTIME
> + LINK_WITH_LIBRT = YesPlease
> +endif
> +
> +ifdef LINK_WITH_LIBRT
> EXTLIBS += -lrt
> endif
>
> diff --git a/config.mak.uname b/config.mak.uname
> index f0d93ef868a7..d04deab2dfa8 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -99,6 +99,7 @@ ifeq ($(uname_S),Darwin)
> USE_ST_TIMESPEC = YesPlease
> HAVE_DEV_TTY = YesPlease
> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> + NO_TIMER_SETTIME = UnfortunatelyYes
> COMPAT_OBJS += compat/precompose_utf8.o
> BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
> endif
> @@ -360,6 +361,7 @@ ifeq ($(uname_S),Windows)
> NO_STRUCT_TIMESPEC = UnfortunatelyYes
> NO_STRUCT_SIGEVENT = UnfortunatelyYes
> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> + NO_TIMER_SETTIME = UnfortunatelyYes
>
> CC = compat/vcbuild/scripts/clink.pl
> AR = compat/vcbuild/scripts/lib.pl
> @@ -513,6 +515,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> NO_STRUCT_TIMESPEC = UnfortunatelyYes
> NO_STRUCT_SIGEVENT = UnfortunatelyYes
> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> + NO_TIMER_SETTIME = UnfortunatelyYes
> COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
> COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
> COMPAT_OBJS += compat/mingw.o compat/winansi.o \
> diff --git a/configure.ac b/configure.ac
> index 954f9ddb03c2..9d6ec41acc82 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -946,6 +946,14 @@ GIT_CHECK_FUNC(setitimer,
> [NO_SETITIMER=YesPlease])
> GIT_CONF_SUBST([NO_SETITIMER])
> #
> +# Define NO_TIMER_SETTIME if you don't have timer_settime
> +GIT_CHECK_FUNC(timer_settime,
> +[NO_TIMER_SETTIME=],
> +[AC_SEARCH_LIBS(timer_settime,[rt],
> + [NO_TIMER_SETTIME=],
> + [NO_TIMER_SETTIME=YesPlease])])
> +GIT_CONF_SUBST([NO_TIMER_SETTIME])
> +#
> # Define NO_STRCASESTR if you don't have strcasestr.
> GIT_CHECK_FUNC(strcasestr,
> [NO_STRCASESTR=],
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 4ef17df86b0e..b23602196323 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -225,7 +225,25 @@ struct itimerspec {
> #endif
>
> #ifdef NO_SETITIMER
> -#define setitimer(which,value,ovalue)
> +#define setitimer(which,value,ovalue) ((void) (which), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
> +#endif
> +
> +#ifdef NO_TIMER_SETTIME
> +#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
> +
> +#define timer_delete(timer) do { \
> + struct itimerval v = {{0,},}; \
> + setitimer(ITIMER_REAL, &v, NULL); \
> +} while (0)
> +
> +#define timer_settime(timer, flags, value, ovalue) do { \
> + struct itimerval _ivalue; \
> + _ivalue.it_interval.tv_sec = value.it_interval.tv_sec; \
> + _ivalue.it_interval.tv_usec = value.it_interval.tv_nsec / 1000L; \
> + _ivalue.it_value.tv_sec value.it_value.tv_sec; \
> + _ivalue_it_value.tv_usec = value.it_value.tv_nsec / 1000L; \
> + setitimer(ITIMER_REAL, &_ivalue, NULL); \
> +while (0)
> #endif
>
> #ifndef NO_LIBGEN_H
>
This looks like that the features that we need can be mapped back and
forth between setitimer() and timer_settime(). So, why don't you define
a compat/setitimer.[ch] for your system that does not have setitimer()
and implement setitimer() in terms of timer_settime() instead of the
other way round? Then you don't need to change any of the client code.
Also, you won't have to worry about the odd timer_settime() in OpenBSD.
-- Hannes
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-08-29 17:26 ` Johannes Sixt
@ 2014-08-29 17:40 ` Keller, Jacob E
2014-09-10 15:33 ` Karsten Blees
0 siblings, 1 reply; 24+ messages in thread
From: Keller, Jacob E @ 2014-08-29 17:40 UTC (permalink / raw)
To: j6t@kdbg.org; +Cc: git@vger.kernel.org, sortie@maxsi.org
On Fri, 2014-08-29 at 19:26 +0200, Johannes Sixt wrote:
> Am 29.08.2014 18:42, schrieb Jacob Keller:
> > From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> >
> > This function will be used in a following commit.
> >
> > The timer_settime function is provided in librt on some systems. We
> > already use this library sometimes to get clock_gettime, so rework the
> > logic so we don't link with it twice.
> >
> > This function was not previously used by git. This can cause trouble for
> > people on systems without timer_settime if they only rely on
> > config.mak.uname. They will need to set NO_TIMER_SETTIME manually.
> >
> > Add proper replacement function macros for setitimer and timer_settime
> > that implement timer_settime as a wrapper for setitimer. In this way, if
> > the system has setitimer but not timer_settime then we will be able to
> > call timer_create, timer_settime, and timer_delete correctly and it will
> > wrap to setitimer under the hood. This will be used in the following
> > commit.
> >
> > Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> > Makefile | 21 +++++++++++++++++++++
> > config.mak.uname | 3 +++
> > configure.ac | 8 ++++++++
> > git-compat-util.h | 20 +++++++++++++++++++-
> > 4 files changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 66329e4b372b..5337ef0b7cd6 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -182,16 +182,22 @@ all::
> > #
> > # Define NO_SETITIMER if you don't have setitimer()
> > #
> > +# Define NO_TIMER_SETTIME if you don't have timer_settime()
> > +#
> > # Define NO_TIMER_T if you don't have timer_t.
> > +# This also implies NO_TIMER_SETTIME
> > #
> > # Define NO_STRUCT_TIMESPEC if you don't have struct timespec
> > +# This also implies NO_TIMER_SETTIME
> > #
> > # Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
> > +# This also implies NO_TIMER_SETTIME
> > #
> > # Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
> > # This also implies NO_SETITIMER
> > #
> > # Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec
> > +# This also implies NO_TIMER_SETTIME
> > #
> > # Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
> > # generally faster on your platform than accessing the working directory.
> > @@ -1348,12 +1354,15 @@ ifdef OBJECT_CREATION_USES_RENAMES
> > endif
> > ifdef NO_TIMER_T
> > COMPAT_CFLAGS += -DNO_TIMER_T
> > + NO_TIMER_SETTIME = YesPlease
> > endif
> > ifdef NO_STRUCT_TIMESPEC
> > COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
> > + NO_TIMER_SETTIME = YesPlease
> > endif
> > ifdef NO_STRUCT_SIGEVENT
> > COMPAT_CFLAGS += -DNO_STRUCT_SIGEVENT
> > + NO_TIMER_SETTIME = YesPlease
> > endif
> > ifdef NO_STRUCT_ITIMERVAL
> > COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
> > @@ -1361,10 +1370,14 @@ ifdef NO_STRUCT_ITIMERVAL
> > endif
> > ifdef NO_STRUCT_ITIMERSPEC
> > COMPAT_CFLAGS += -DNO_STRUCT_ITIMERSPEC
> > + NO_TIMER_SETTIME = YesPlease
> > endif
> > ifdef NO_SETITIMER
> > COMPAT_CFLAGS += -DNO_SETITIMER
> > endif
> > +ifdef NO_TIMER_SETTIME
> > + COMPAT_CFLAGS += -DNO_TIMER_SETTIME
> > +endif
> > ifdef NO_PREAD
> > COMPAT_CFLAGS += -DNO_PREAD
> > COMPAT_OBJS += compat/pread.o
> > @@ -1524,6 +1537,14 @@ endif
> >
> > ifdef HAVE_CLOCK_GETTIME
> > BASIC_CFLAGS += -DHAVE_CLOCK_GETTIME
> > + LINK_WITH_LIBRT = YesPlease
> > +endif
> > +
> > +ifndef NO_TIMER_SETTIME
> > + LINK_WITH_LIBRT = YesPlease
> > +endif
> > +
> > +ifdef LINK_WITH_LIBRT
> > EXTLIBS += -lrt
> > endif
> >
> > diff --git a/config.mak.uname b/config.mak.uname
> > index f0d93ef868a7..d04deab2dfa8 100644
> > --- a/config.mak.uname
> > +++ b/config.mak.uname
> > @@ -99,6 +99,7 @@ ifeq ($(uname_S),Darwin)
> > USE_ST_TIMESPEC = YesPlease
> > HAVE_DEV_TTY = YesPlease
> > NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> > + NO_TIMER_SETTIME = UnfortunatelyYes
> > COMPAT_OBJS += compat/precompose_utf8.o
> > BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
> > endif
> > @@ -360,6 +361,7 @@ ifeq ($(uname_S),Windows)
> > NO_STRUCT_TIMESPEC = UnfortunatelyYes
> > NO_STRUCT_SIGEVENT = UnfortunatelyYes
> > NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> > + NO_TIMER_SETTIME = UnfortunatelyYes
> >
> > CC = compat/vcbuild/scripts/clink.pl
> > AR = compat/vcbuild/scripts/lib.pl
> > @@ -513,6 +515,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
> > NO_STRUCT_TIMESPEC = UnfortunatelyYes
> > NO_STRUCT_SIGEVENT = UnfortunatelyYes
> > NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
> > + NO_TIMER_SETTIME = UnfortunatelyYes
> > COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
> > COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
> > COMPAT_OBJS += compat/mingw.o compat/winansi.o \
> > diff --git a/configure.ac b/configure.ac
> > index 954f9ddb03c2..9d6ec41acc82 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -946,6 +946,14 @@ GIT_CHECK_FUNC(setitimer,
> > [NO_SETITIMER=YesPlease])
> > GIT_CONF_SUBST([NO_SETITIMER])
> > #
> > +# Define NO_TIMER_SETTIME if you don't have timer_settime
> > +GIT_CHECK_FUNC(timer_settime,
> > +[NO_TIMER_SETTIME=],
> > +[AC_SEARCH_LIBS(timer_settime,[rt],
> > + [NO_TIMER_SETTIME=],
> > + [NO_TIMER_SETTIME=YesPlease])])
> > +GIT_CONF_SUBST([NO_TIMER_SETTIME])
> > +#
> > # Define NO_STRCASESTR if you don't have strcasestr.
> > GIT_CHECK_FUNC(strcasestr,
> > [NO_STRCASESTR=],
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index 4ef17df86b0e..b23602196323 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -225,7 +225,25 @@ struct itimerspec {
> > #endif
> >
> > #ifdef NO_SETITIMER
> > -#define setitimer(which,value,ovalue)
> > +#define setitimer(which,value,ovalue) ((void) (which), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
> > +#endif
> > +
> > +#ifdef NO_TIMER_SETTIME
> > +#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
> > +
> > +#define timer_delete(timer) do { \
> > + struct itimerval v = {{0,},}; \
> > + setitimer(ITIMER_REAL, &v, NULL); \
> > +} while (0)
> > +
> > +#define timer_settime(timer, flags, value, ovalue) do { \
> > + struct itimerval _ivalue; \
> > + _ivalue.it_interval.tv_sec = value.it_interval.tv_sec; \
> > + _ivalue.it_interval.tv_usec = value.it_interval.tv_nsec / 1000L; \
> > + _ivalue.it_value.tv_sec value.it_value.tv_sec; \
> > + _ivalue_it_value.tv_usec = value.it_value.tv_nsec / 1000L; \
> > + setitimer(ITIMER_REAL, &_ivalue, NULL); \
> > +while (0)
> > #endif
> >
> > #ifndef NO_LIBGEN_H
> >
>
> This looks like that the features that we need can be mapped back and
> forth between setitimer() and timer_settime(). So, why don't you define
> a compat/setitimer.[ch] for your system that does not have setitimer()
> and implement setitimer() in terms of timer_settime() instead of the
> other way round? Then you don't need to change any of the client code.
> Also, you won't have to worry about the odd timer_settime() in OpenBSD.
>
> -- Hannes
>
You could do that, I suppose, though I personally prefer to use the
newer standard in the core code, and only fallback to older standards in
the case where you don't have what is new (instead of the other way
around).
Overall it doesn't really matter to me.
Regards,
Jake
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] Use timer_settime for new platforms
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
@ 2014-08-29 18:02 ` Junio C Hamano
2014-08-29 18:09 ` Keller, Jacob E
2014-08-29 18:12 ` Junio C Hamano
1 sibling, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2014-08-29 18:02 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Jonas 'Sortie' Termansen
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>
> setitimer() is an obsolescent XSI interface and may be removed in a
> future standard. Applications should use the core POSIX timer_settime()
> instead.
>
> It's important that code doesn't simply check if timer_settime is
> available as it can give false positives. Some systems like contemporary
> OpenBSD provides the function, but it unconditionally fails with ENOSYS
> at runtime.
Doesn't this paragraph need tweaking? I think you lost (which is a
good thing) "notice that timer_settime() call failed with ENOSYS and
switch to setitimer()", no?
> Clean up the progress reporting and change it to use timer_settime,
> which will fall back to setitimer automatically if timer_settime is not
> supported. (see git-compat-util.h for how it does this). If both
> functions are not present, then git-compat-util.h provides replacements
> which will always fail with ENOSYS.
While this paragraph may be true if patch 8b and 9 are taken
together, isn't what it describes mostly what 8b did, not 9?
Here by 8b I mean the change to git-compat-util.h in 8; the patch
might want to be split into two, 8a for the autoconf part whose log
message may begin with "This function was not previously used by
git." and 8b that adds an emulation of timer_settime() API in terms
of setitimer() API, or the other way around.
What 9 did is only "we used to use the setitmer() API to implement
the progress reporting; now we use timer_settime() API" (yes, it is
thanks to the abstraction given by 8, but the "callers has to only
know about one API, not worrying about the other API" is a merit
attributable to 8b, not this one).
> The approach used here enables us to use a single API (timer_settime)
> without having to worry about checking for #ifdefs or if blocks which
> make it an unreadable nightmare. The major downside is for systems
> without timer_settime support, they may fall back on a wrapped
> implementation which could have subtle differences. This should be a
> minor issue as almost all modern systems provide timer_settime support.
As this paragraph.
> Note that this change means that git should never use setitimer on its
> own now, as the fallback implementation of timer_settime assumes that it
> is the sole user of ITIMER_REAL, and timer_delete will reset the
> ITIMER_REAL.
And this one.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] Use timer_settime for new platforms
2014-08-29 18:02 ` Junio C Hamano
@ 2014-08-29 18:09 ` Keller, Jacob E
0 siblings, 0 replies; 24+ messages in thread
From: Keller, Jacob E @ 2014-08-29 18:09 UTC (permalink / raw)
To: gitster@pobox.com; +Cc: git@vger.kernel.org, sortie@maxsi.org
On Fri, 2014-08-29 at 11:02 -0700, Junio C Hamano wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
> > From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> >
> > setitimer() is an obsolescent XSI interface and may be removed in a
> > future standard. Applications should use the core POSIX timer_settime()
> > instead.
> >
> > It's important that code doesn't simply check if timer_settime is
> > available as it can give false positives. Some systems like contemporary
> > OpenBSD provides the function, but it unconditionally fails with ENOSYS
> > at runtime.
>
> Doesn't this paragraph need tweaking? I think you lost (which is a
> good thing) "notice that timer_settime() call failed with ENOSYS and
> switch to setitimer()", no?
>
> > Clean up the progress reporting and change it to use timer_settime,
> > which will fall back to setitimer automatically if timer_settime is not
> > supported. (see git-compat-util.h for how it does this). If both
> > functions are not present, then git-compat-util.h provides replacements
> > which will always fail with ENOSYS.
>
> While this paragraph may be true if patch 8b and 9 are taken
> together, isn't what it describes mostly what 8b did, not 9?
>
> Here by 8b I mean the change to git-compat-util.h in 8; the patch
> might want to be split into two, 8a for the autoconf part whose log
> message may begin with "This function was not previously used by
> git." and 8b that adds an emulation of timer_settime() API in terms
> of setitimer() API, or the other way around.
>
> What 9 did is only "we used to use the setitmer() API to implement
> the progress reporting; now we use timer_settime() API" (yes, it is
> thanks to the abstraction given by 8, but the "callers has to only
> know about one API, not worrying about the other API" is a merit
> attributable to 8b, not this one).
>
You are correct. I can re-base these patches and split them apart a bit
better.
Regards,
Jake
> > The approach used here enables us to use a single API (timer_settime)
> > without having to worry about checking for #ifdefs or if blocks which
> > make it an unreadable nightmare. The major downside is for systems
> > without timer_settime support, they may fall back on a wrapped
> > implementation which could have subtle differences. This should be a
> > minor issue as almost all modern systems provide timer_settime support.
>
> As this paragraph.
>
> > Note that this change means that git should never use setitimer on its
> > own now, as the fallback implementation of timer_settime assumes that it
> > is the sole user of ITIMER_REAL, and timer_delete will reset the
> > ITIMER_REAL.
>
> And this one.
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] Use timer_settime for new platforms
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
2014-08-29 18:02 ` Junio C Hamano
@ 2014-08-29 18:12 ` Junio C Hamano
1 sibling, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2014-08-29 18:12 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Jonas 'Sortie' Termansen
Jacob Keller <jacob.e.keller@intel.com> writes:
> diff --git a/builtin/log.c b/builtin/log.c
> index 4389722b4b1e..a39e82d67eb3 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> ...
> @@ -271,9 +271,12 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)
> * trigger every second even if we're blocked on a
> * reader!
> */
> - early_output_timer.it_value.tv_sec = 0;
> - early_output_timer.it_value.tv_usec = 500000;
> - setitimer(ITIMER_REAL, &early_output_timer, NULL);
> + struct itimerspec value;
> + value.it_value.tv_sec = 0;
> + value.it_value.tv_nsec = 500000L * 1000L;
> + value.it_interval.tv_sec = 0;
> + value.it_interval.tv_nsec = 0;
> + timer_settime(early_output_timer, 0, &value, NULL);
Do I see decl-after-stmt up there?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/9] autoconf: Check for timer_t
2014-08-29 16:42 ` [PATCH 4/9] autoconf: Check for timer_t Jacob Keller
@ 2014-08-29 18:20 ` Jonas 'Sortie' Termansen
0 siblings, 0 replies; 24+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-29 18:20 UTC (permalink / raw)
To: Jacob Keller, git
For the record, this commit doesn't contain my errata for OS X:
ifeq ($(uname_S),Darwin)
...
HAVE_DEV_TTY = YesPlease
+ NO_TIMER_T = UnfortunatelyYes
COMPAT_OBJS += compat/precompose_utf8.o
...
endif
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-29 16:48 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Keller, Jacob E
@ 2014-08-29 18:54 ` Jonas 'Sortie' Termansen
2014-08-29 19:07 ` Junio C Hamano
0 siblings, 1 reply; 24+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-29 18:54 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Keller, Jacob E, Johannes Sixt, Junio C Hamano
Hi,
Thanks for the interest. :)
There's a whole lot of emails being sent. I'll make a nice V2 shortly that
takes your feedback into consideration. :)
But first let's discuss. I think we should define the intended criteria.
I expect to find these systems out there:
* No setitimer and no timer_settime.
* Has setitimer and no timer_settime.
* Has setitimer and timer_settime (broken).
* Has setitimer and timer_settime (works).
* No setitimer and timer_settime (works).
Which of these do we want to support? Right we support the cases where
systems have setitimer, the cases without it is slightly broken prior to
my fixes.
Jake's modified patch set breaks the case where timer_settimer exists and
is broken. As far as I know, that's only OpenBSD among the noticeable free
software world, but could be more systems, perhaps in the future.
The progress bar displayed is rather non-essential. If we go with Jake's
proposal, we support most non-broken platforms, and the broken platforms
will start working when they add POSIX timers.
Ideally, I'd prefer to only support the systems with timer_settime that
works, but real people use git on systems without and it is not too much
work to support all of these combinations.
I see these approaches to the problem:
1) Only use setitimer (do nothing right now).
* Disadvantage: We don't support modern systems without setitimer but that
has timer_settime. Those are few, though, as setitimer is pretty much
universal at the moment.
* Disadvantage: We are using an older interface instead of the modern good
practices.
2) Use setitimer (emulated with timer_create if needed).
* Disadvantage: The core source code doesn't employ current best practices.
3) Use timer_create (emulated with setitimer if needed).
* Disadvantage: The build system may have a false positive when checking
whether timer_settime is available.
4) Use both (decision is made at runtime if both are available)
If we do this well, the bulk of the compatibility code is isolated from
the real source code (that just uses timer_settime naively) and it can
be reduced when broken systems gets fixed.
* Advantage: No regressions.
* Disadvantage: The compatibility logic may be complicated.
I'm personally in favor of proposal 3, but it's more in git's spirit to pick
proposal 4 as supports more of the real systems out there.
My first attempt was essentially proposal 4, but with no effort in trying to
hide the fact that timer_settime might be broken. I'm going to develop a V2
that isolates this compatibility logic from the core code. I'm not convinced
this approach is actually cleaner, but we'll see when it's done. Either way,
isolated compatibility code today can be removed tomorrow when we no longer
need it. :)
Jonas
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-29 18:54 ` Jonas 'Sortie' Termansen
@ 2014-08-29 19:07 ` Junio C Hamano
2014-08-29 19:43 ` Jonas 'Sortie' Termansen
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2014-08-29 19:07 UTC (permalink / raw)
To: Jonas 'Sortie' Termansen
Cc: git@vger.kernel.org, Keller, Jacob E, Johannes Sixt
Jonas 'Sortie' Termansen <Sortie@Maxsi.org> writes:
> Jake's modified patch set breaks the case where timer_settimer exists and
> is broken. As far as I know, that's only OpenBSD among the noticeable free
> software world, but could be more systems, perhaps in the future.
That is easy to fix, isn't it?
Where you said "If you have timer_settimer(), set this makefile
variable", you start the sentence with "If you have a working
timer_settimer()" instead.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-29 19:07 ` Junio C Hamano
@ 2014-08-29 19:43 ` Jonas 'Sortie' Termansen
2014-09-03 0:17 ` Keller, Jacob E
0 siblings, 1 reply; 24+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-29 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Keller, Jacob E, Johannes Sixt
On 08/29/2014 09:07 PM, Junio C Hamano wrote:
> Jonas 'Sortie' Termansen <Sortie@Maxsi.org> writes:
> That is easy to fix, isn't it?
>
> Where you said "If you have timer_settimer(), set this makefile
> variable", you start the sentence with "If you have a working
> timer_settimer()" instead.
That's mostly right. I care about cross-compilation, so that is also
something to keep in mind. It would be best if this could be
automatically and robustly determined (even when cross-compiling),
rather than relying on the user setting magic make variables.
Jonas
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval
2014-08-29 19:43 ` Jonas 'Sortie' Termansen
@ 2014-09-03 0:17 ` Keller, Jacob E
0 siblings, 0 replies; 24+ messages in thread
From: Keller, Jacob E @ 2014-09-03 0:17 UTC (permalink / raw)
To: Jonas 'Sortie' Termansen, Junio C Hamano
Cc: git@vger.kernel.org, Johannes Sixt
> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Jonas 'Sortie' Termansen
> Sent: Friday, August 29, 2014 12:44 PM
> To: Junio C Hamano
> Cc: git@vger.kernel.org; Keller, Jacob E; Johannes Sixt
> Subject: Re: [PATCH 1/9] git-compat-util.h: Add missing semicolon after
> struct itimerval
>
> On 08/29/2014 09:07 PM, Junio C Hamano wrote:
> > Jonas 'Sortie' Termansen <Sortie@Maxsi.org> writes:
> > That is easy to fix, isn't it?
> >
> > Where you said "If you have timer_settimer(), set this makefile
> > variable", you start the sentence with "If you have a working
> > timer_settimer()" instead.
>
> That's mostly right. I care about cross-compilation, so that is also
> something to keep in mind. It would be best if this could be
> automatically and robustly determined (even when cross-compiling),
> rather than relying on the user setting magic make variables.
>
> Jonas
Couldn't this be solved by making our configure script smart enough to tell when it's at least the "known" broken OpenBSD implementation?
Regards,
Jake
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-08-29 17:40 ` Keller, Jacob E
@ 2014-09-10 15:33 ` Karsten Blees
2014-09-10 21:08 ` Junio C Hamano
0 siblings, 1 reply; 24+ messages in thread
From: Karsten Blees @ 2014-09-10 15:33 UTC (permalink / raw)
To: Keller, Jacob E, j6t@kdbg.org; +Cc: git@vger.kernel.org, sortie@maxsi.org
Am 29.08.2014 19:40, schrieb Keller, Jacob E:
> On Fri, 2014-08-29 at 19:26 +0200, Johannes Sixt wrote:
>> Am 29.08.2014 18:42, schrieb Jacob Keller:
>>> From: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>>>
>>> This function will be used in a following commit.
>>>
>>> The timer_settime function is provided in librt on some systems. We
>>> already use this library sometimes to get clock_gettime, so rework the
>>> logic so we don't link with it twice.
>>>
>>> This function was not previously used by git. This can cause trouble for
>>> people on systems without timer_settime if they only rely on
>>> config.mak.uname. They will need to set NO_TIMER_SETTIME manually.
>>>
>>> Add proper replacement function macros for setitimer and timer_settime
>>> that implement timer_settime as a wrapper for setitimer. In this way, if
>>> the system has setitimer but not timer_settime then we will be able to
>>> call timer_create, timer_settime, and timer_delete correctly and it will
>>> wrap to setitimer under the hood. This will be used in the following
>>> commit.
>>>
>>> Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>>> ---
>>> Makefile | 21 +++++++++++++++++++++
>>> config.mak.uname | 3 +++
>>> configure.ac | 8 ++++++++
>>> git-compat-util.h | 20 +++++++++++++++++++-
>>> 4 files changed, 51 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 66329e4b372b..5337ef0b7cd6 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -182,16 +182,22 @@ all::
>>> #
>>> # Define NO_SETITIMER if you don't have setitimer()
>>> #
>>> +# Define NO_TIMER_SETTIME if you don't have timer_settime()
>>> +#
>>> # Define NO_TIMER_T if you don't have timer_t.
>>> +# This also implies NO_TIMER_SETTIME
>>> #
>>> # Define NO_STRUCT_TIMESPEC if you don't have struct timespec
>>> +# This also implies NO_TIMER_SETTIME
>>> #
>>> # Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
>>> +# This also implies NO_TIMER_SETTIME
>>> #
>>> # Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval
>>> # This also implies NO_SETITIMER
>>> #
>>> # Define NO_STRUCT_ITIMERSPEC if you don't have struct itimerspec
>>> +# This also implies NO_TIMER_SETTIME
>>> #
>>> # Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
>>> # generally faster on your platform than accessing the working directory.
>>> @@ -1348,12 +1354,15 @@ ifdef OBJECT_CREATION_USES_RENAMES
>>> endif
>>> ifdef NO_TIMER_T
>>> COMPAT_CFLAGS += -DNO_TIMER_T
>>> + NO_TIMER_SETTIME = YesPlease
>>> endif
>>> ifdef NO_STRUCT_TIMESPEC
>>> COMPAT_CFLAGS += -DNO_STRUCT_TIMESPEC
>>> + NO_TIMER_SETTIME = YesPlease
>>> endif
>>> ifdef NO_STRUCT_SIGEVENT
>>> COMPAT_CFLAGS += -DNO_STRUCT_SIGEVENT
>>> + NO_TIMER_SETTIME = YesPlease
>>> endif
>>> ifdef NO_STRUCT_ITIMERVAL
>>> COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL
>>> @@ -1361,10 +1370,14 @@ ifdef NO_STRUCT_ITIMERVAL
>>> endif
>>> ifdef NO_STRUCT_ITIMERSPEC
>>> COMPAT_CFLAGS += -DNO_STRUCT_ITIMERSPEC
>>> + NO_TIMER_SETTIME = YesPlease
>>> endif
>>> ifdef NO_SETITIMER
>>> COMPAT_CFLAGS += -DNO_SETITIMER
>>> endif
>>> +ifdef NO_TIMER_SETTIME
>>> + COMPAT_CFLAGS += -DNO_TIMER_SETTIME
>>> +endif
>>> ifdef NO_PREAD
>>> COMPAT_CFLAGS += -DNO_PREAD
>>> COMPAT_OBJS += compat/pread.o
>>> @@ -1524,6 +1537,14 @@ endif
>>>
>>> ifdef HAVE_CLOCK_GETTIME
>>> BASIC_CFLAGS += -DHAVE_CLOCK_GETTIME
>>> + LINK_WITH_LIBRT = YesPlease
>>> +endif
>>> +
>>> +ifndef NO_TIMER_SETTIME
>>> + LINK_WITH_LIBRT = YesPlease
>>> +endif
>>> +
>>> +ifdef LINK_WITH_LIBRT
>>> EXTLIBS += -lrt
>>> endif
>>>
>>> diff --git a/config.mak.uname b/config.mak.uname
>>> index f0d93ef868a7..d04deab2dfa8 100644
>>> --- a/config.mak.uname
>>> +++ b/config.mak.uname
>>> @@ -99,6 +99,7 @@ ifeq ($(uname_S),Darwin)
>>> USE_ST_TIMESPEC = YesPlease
>>> HAVE_DEV_TTY = YesPlease
>>> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
>>> + NO_TIMER_SETTIME = UnfortunatelyYes
>>> COMPAT_OBJS += compat/precompose_utf8.o
>>> BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
>>> endif
>>> @@ -360,6 +361,7 @@ ifeq ($(uname_S),Windows)
>>> NO_STRUCT_TIMESPEC = UnfortunatelyYes
>>> NO_STRUCT_SIGEVENT = UnfortunatelyYes
>>> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
>>> + NO_TIMER_SETTIME = UnfortunatelyYes
>>>
>>> CC = compat/vcbuild/scripts/clink.pl
>>> AR = compat/vcbuild/scripts/lib.pl
>>> @@ -513,6 +515,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>>> NO_STRUCT_TIMESPEC = UnfortunatelyYes
>>> NO_STRUCT_SIGEVENT = UnfortunatelyYes
>>> NO_STRUCT_ITIMERSPEC = UnfortunatelyYes
>>> + NO_TIMER_SETTIME = UnfortunatelyYes
>>> COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32
>>> COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
>>> COMPAT_OBJS += compat/mingw.o compat/winansi.o \
>>> diff --git a/configure.ac b/configure.ac
>>> index 954f9ddb03c2..9d6ec41acc82 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -946,6 +946,14 @@ GIT_CHECK_FUNC(setitimer,
>>> [NO_SETITIMER=YesPlease])
>>> GIT_CONF_SUBST([NO_SETITIMER])
>>> #
>>> +# Define NO_TIMER_SETTIME if you don't have timer_settime
>>> +GIT_CHECK_FUNC(timer_settime,
>>> +[NO_TIMER_SETTIME=],
>>> +[AC_SEARCH_LIBS(timer_settime,[rt],
>>> + [NO_TIMER_SETTIME=],
>>> + [NO_TIMER_SETTIME=YesPlease])])
>>> +GIT_CONF_SUBST([NO_TIMER_SETTIME])
>>> +#
>>> # Define NO_STRCASESTR if you don't have strcasestr.
>>> GIT_CHECK_FUNC(strcasestr,
>>> [NO_STRCASESTR=],
>>> diff --git a/git-compat-util.h b/git-compat-util.h
>>> index 4ef17df86b0e..b23602196323 100644
>>> --- a/git-compat-util.h
>>> +++ b/git-compat-util.h
>>> @@ -225,7 +225,25 @@ struct itimerspec {
>>> #endif
>>>
>>> #ifdef NO_SETITIMER
>>> -#define setitimer(which,value,ovalue)
>>> +#define setitimer(which,value,ovalue) ((void) (which), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
>>> +#endif
>>> +
>>> +#ifdef NO_TIMER_SETTIME
>>> +#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
>>> +
>>> +#define timer_delete(timer) do { \
>>> + struct itimerval v = {{0,},}; \
>>> + setitimer(ITIMER_REAL, &v, NULL); \
>>> +} while (0)
>>> +
>>> +#define timer_settime(timer, flags, value, ovalue) do { \
>>> + struct itimerval _ivalue; \
>>> + _ivalue.it_interval.tv_sec = value.it_interval.tv_sec; \
>>> + _ivalue.it_interval.tv_usec = value.it_interval.tv_nsec / 1000L; \
>>> + _ivalue.it_value.tv_sec value.it_value.tv_sec; \
>>> + _ivalue_it_value.tv_usec = value.it_value.tv_nsec / 1000L; \
>>> + setitimer(ITIMER_REAL, &_ivalue, NULL); \
>>> +while (0)
>>> #endif
>>>
>>> #ifndef NO_LIBGEN_H
>>>
This is required to compile on Windows:
--- 8< ---
diff --git a/git-compat-util.h b/git-compat-util.h
index 237c2a5..5dd4ee0 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -227,7 +227,9 @@ struct itimerspec {
#ifdef NO_SETITIMER
#define setitimer(which,value,ovalue) ((void) (which), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
#endif
+#ifndef CLOCK_MONOTONIC
+#define CLOCK_MONOTONIC 1
+#endif
#ifdef NO_TIMER_SETTIME
#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
@@ -238,12 +240,12 @@ struct itimerspec {
#define timer_settime(timer, flags, value, ovalue) do { \
struct itimerval _ivalue; \
- _ivalue.it_interval.tv_sec = value.it_interval.tv_sec; \
- _ivalue.it_interval.tv_usec = value.it_interval.tv_nsec / 1000L; \
- _ivalue.it_value.tv_sec value.it_value.tv_sec; \
- _ivalue_it_value.tv_usec = value.it_value.tv_nsec / 1000L; \
+ _ivalue.it_interval.tv_sec = (value)->it_interval.tv_sec; \
+ _ivalue.it_interval.tv_usec = (value)->it_interval.tv_nsec / 1000L; \
+ _ivalue.it_value.tv_sec = (value)->it_value.tv_sec; \
+ _ivalue.it_value.tv_usec = (value)->it_value.tv_nsec / 1000L; \
setitimer(ITIMER_REAL, &_ivalue, NULL); \
-while (0)
+} while (0)
#endif
#ifndef NO_LIBGEN_H
--- 8<---
>>
>> This looks like that the features that we need can be mapped back and
>> forth between setitimer() and timer_settime(). So, why don't you define
>> a compat/setitimer.[ch] for your system that does not have setitimer()
>> and implement setitimer() in terms of timer_settime() instead of the
>> other way round? Then you don't need to change any of the client code.
>> Also, you won't have to worry about the odd timer_settime() in OpenBSD.
>>
>> -- Hannes
>>
>
> You could do that, I suppose, though I personally prefer to use the
> newer standard in the core code, and only fallback to older standards in
> the case where you don't have what is new (instead of the other way
> around).
>
> Overall it doesn't really matter to me.
>
While the timer extension (timer_settime) has graduated to mandatory in
the current POSIX spec, the monotonic clock extension is still optional
today (i.e. not necessarily supported even on newer Unices). In contrast
to this, the XSI extensions seem to be widely supported.
IMO the 'obsolescent' marker in the current POSIX spec is no reason to
spring into action (at least not yet). E.g. utimes (also in <sys/time.h>)
has been marked LEGACY in the 2004 version and is no longer LEGACY today.
Btw., we'd also have to find a replacement for gettimeofday and probably
a lot of other stuff...
Therefore I tend to agree with Hannes that we should stick with setitimer
and emulate it on systems that don't have it (as we do on Windows).
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-09-10 15:33 ` Karsten Blees
@ 2014-09-10 21:08 ` Junio C Hamano
2014-09-10 21:13 ` Keller, Jacob E
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2014-09-10 21:08 UTC (permalink / raw)
To: Karsten Blees
Cc: Keller, Jacob E, j6t@kdbg.org, git@vger.kernel.org,
sortie@maxsi.org
Karsten Blees <karsten.blees@gmail.com> writes:
> While the timer extension (timer_settime) has graduated to mandatory in
> the current POSIX spec, the monotonic clock extension is still optional
> today (i.e. not necessarily supported even on newer Unices). In contrast
> to this, the XSI extensions seem to be widely supported.
>
> IMO the 'obsolescent' marker in the current POSIX spec is no reason to
> spring into action (at least not yet). E.g. utimes (also in <sys/time.h>)
> has been marked LEGACY in the 2004 version and is no longer LEGACY today.
> Btw., we'd also have to find a replacement for gettimeofday and probably
> a lot of other stuff...
>
> Therefore I tend to agree with Hannes that we should stick with setitimer
> and emulate it on systems that don't have it (as we do on Windows).
Sounds sensible to me.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-09-10 21:08 ` Junio C Hamano
@ 2014-09-10 21:13 ` Keller, Jacob E
0 siblings, 0 replies; 24+ messages in thread
From: Keller, Jacob E @ 2014-09-10 21:13 UTC (permalink / raw)
To: gitster@pobox.com
Cc: karsten.blees@gmail.com, j6t@kdbg.org, git@vger.kernel.org,
sortie@maxsi.org
On Wed, 2014-09-10 at 14:08 -0700, Junio C Hamano wrote:
> Karsten Blees <karsten.blees@gmail.com> writes:
>
> > While the timer extension (timer_settime) has graduated to mandatory in
> > the current POSIX spec, the monotonic clock extension is still optional
> > today (i.e. not necessarily supported even on newer Unices). In contrast
> > to this, the XSI extensions seem to be widely supported.
> >
> > IMO the 'obsolescent' marker in the current POSIX spec is no reason to
> > spring into action (at least not yet). E.g. utimes (also in <sys/time.h>)
> > has been marked LEGACY in the 2004 version and is no longer LEGACY today.
> > Btw., we'd also have to find a replacement for gettimeofday and probably
> > a lot of other stuff...
> >
> > Therefore I tend to agree with Hannes that we should stick with setitimer
> > and emulate it on systems that don't have it (as we do on Windows).
>
> Sounds sensible to me.
Ya that makes sense. Was thinking too much about compatability code for
Linux kernel drivers, which is a different issue.
Let's go with re-implementing settimer in terms of timer_settime on the
one system we have information that doesn't support settimer.
I could spin that patch, but I think the original author should, since
he's the one who could actually test it.
Regards,
Jake
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-09-10 21:13 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-29 16:42 [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jacob Keller
2014-08-29 16:42 ` [PATCH 2/9] autoconf: Check for " Jacob Keller
2014-08-29 16:42 ` [PATCH 3/9] autoconf: Check for setitimer Jacob Keller
2014-08-29 16:42 ` [PATCH 4/9] autoconf: Check for timer_t Jacob Keller
2014-08-29 18:20 ` Jonas 'Sortie' Termansen
2014-08-29 16:42 ` [PATCH 5/9] autoconf: Check for struct timespec Jacob Keller
2014-08-29 16:42 ` [PATCH 6/9] autoconf: Check for struct sigevent Jacob Keller
2014-08-29 16:42 ` [PATCH 7/9] autoconf: Check for struct itimerspec Jacob Keller
2014-08-29 16:42 ` [PATCH 8/9] autoconf: Check for timer_settime Jacob Keller
2014-08-29 17:26 ` Johannes Sixt
2014-08-29 17:40 ` Keller, Jacob E
2014-09-10 15:33 ` Karsten Blees
2014-09-10 21:08 ` Junio C Hamano
2014-09-10 21:13 ` Keller, Jacob E
2014-08-29 16:42 ` [PATCH 9/9] Use timer_settime for new platforms Jacob Keller
2014-08-29 18:02 ` Junio C Hamano
2014-08-29 18:09 ` Keller, Jacob E
2014-08-29 18:12 ` Junio C Hamano
2014-08-29 16:48 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Keller, Jacob E
2014-08-29 18:54 ` Jonas 'Sortie' Termansen
2014-08-29 19:07 ` Junio C Hamano
2014-08-29 19:43 ` Jonas 'Sortie' Termansen
2014-09-03 0:17 ` Keller, Jacob E
-- strict thread matches above, loose matches on Subject: below --
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jonas 'Sortie' Termansen
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).