* [PATCH 0/9] Use timer_settime for new platforms
@ 2014-08-28 1:04 Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval Jonas 'Sortie' Termansen
` (8 more replies)
0 siblings, 9 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git
Hi,
I have ported git to my homemade custom operating system. It implements the
modern core POSIX interface and by design doesn't implement older
obsolescent interfaces that has standardized superior replacements. This
causes some problems here and there, this patch series fixes one of them.
Git uses the setitimer() function to deliver a steady supply of signals,
which cause nice progress bars to show when operations take a long time.
However, this function is part of the XSI option (i.e. not core POSIX) and
has been marked as obsolescent in the latest standard revision (it will
likely be removed in the next revision). Application developers are instead
encouraged to use the timer_settime functions instead as they are more
powerful and are part of core POSIX (added in POSIX 2001).
This patch series transitions git to use the timer_settime function instead
of the setitimer function. The function is widely, but not universally
available on the platforms git current supports, so I need to be careful not
to introduce regressions.
I handle compatibility by checking if setitimer and timer_settime are
available (as well as the structures and types they need). git-compat-util.h
provides stubs for anything that is missing. The real code can then simply
try to use timer_create, and if it fails (such as with ENOSYS on platforms
that has the function, but no real implementation) it falls back on the
older setitimer function.
My first attempt at compatibility was simply adding #ifdef around all the
timer_settime calls, but this proved to be an unmaintainable maze of
preprocessor conditionals. Instead I adopted the approach already used when
setitimer is missing: Just using a preprocessor function macro that expands
calls to nothing, allowing code to pretend it's always there.
To properly pretend timer_settime is available when it's not, I had to stub
the timer_t, struct timespec, struct itimerspec, and struct sigevent types
if they were missing.
I have attempted to properly add make variables for each compatibility case
as well as proper configure tests. Autoconf is not my strong suit, but the
tests appears to be working.
This patch series breaks the build for people using just config.mak.uname
(and not the configure script) on platforms that doesn't have the used types
and functions (timer_t, timespec, itimerspec, sigevent, timer_settime) as
config.mak.uname doesn't yet have NO_FOO cases for those systems. That's
just because I don't have that all of those systems available. It does look
like Cygwin, FreeBSD, Linux, NetBSD, Minix, and OpenBSD have the API at
least. I believe I added the required NO_FOO variables for Darwin, MinGW and
Windows. If you have other systems available, you can help me by telling me
if they need some NO_FOO love in config.mak.uname as well. :-)
Jonas 'Sortie' Termansen
^ permalink raw reply [flat|nested] 16+ messages in thread
* [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
2014-08-28 1:04 ` [PATCH 2/9] autoconf: Check for timer_t Jonas 'Sortie' Termansen
` (7 subsequent siblings)
8 siblings, 0 replies; 16+ 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] 16+ messages in thread
* [PATCH 2/9] autoconf: Check for timer_t
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
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 12:03 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 3/9] autoconf: Check for struct timespec Jonas 'Sortie' Termansen
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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>
---
This patch can be improved by finding out which systems doesn't have
timer_t and adding entries for them to config.mak.uname.
Makefile | 5 +++++
config.mak.uname | 2 ++
configure.ac | 10 +++++++++-
git-compat-util.h | 4 ++++
4 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 9f984a9..54266fd 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 15ee15e..a5297a2 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 4b1ae7c..c57fd25 100644
--- a/configure.ac
+++ b/configure.ac
@@ -745,7 +745,15 @@ case $ac_cv_type_socklen_t in
*) SOCKLEN_T=$git_cv_socklen_t_equiv;;
esac
GIT_CONF_SUBST([SOCKLEN_T])
-
+#
+# 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 f7fae20..e0e7a62 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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/9] autoconf: Check for struct timespec
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
2014-08-28 1:04 ` [PATCH 2/9] autoconf: Check for timer_t Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 4/9] autoconf: Check for struct sigevent Jonas 'Sortie' Termansen
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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>
---
This patch can be improved by finding out which systems doesn't have
struct timespec and adding entries for them to config.mak.uname.
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 54266fd..0dd3e35 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 a5297a2..8121791 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 c57fd25..5a5b167 100644
--- a/configure.ac
+++ b/configure.ac
@@ -754,6 +754,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 e0e7a62..e9e7e54 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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/9] autoconf: Check for struct sigevent
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (2 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 3/9] autoconf: Check for struct timespec Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 5/9] autoconf: Check for struct itimerval Jonas 'Sortie' Termansen
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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>
---
This patch can be improved by finding out which systems doesn't have
struct sigevent and adding entries for them to config.mak.uname.
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 0dd3e35..b76dc43 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 8121791..892afc5 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 5a5b167..31b3218 100644
--- a/configure.ac
+++ b/configure.ac
@@ -768,6 +768,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 e9e7e54..195eda6 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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/9] autoconf: Check for struct itimerval
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (3 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 4/9] autoconf: Check for struct sigevent Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 19:38 ` Junio C Hamano
2014-08-28 1:04 ` [PATCH 6/9] autoconf: Check for struct itimerspec Jonas 'Sortie' Termansen
` (3 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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 | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/configure.ac b/configure.ac
index 31b3218..00842ae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -761,6 +761,13 @@ AC_CHECK_TYPES([struct timespec],
[#include <sys/time.h>])
GIT_CONF_SUBST([NO_STRUCT_TIMESPEC])
#
+# 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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 6/9] autoconf: Check for struct itimerspec
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (4 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 5/9] autoconf: Check for struct itimerval Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 7/9] autoconf: Check for setitimer Jonas 'Sortie' Termansen
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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>
---
This patch can be improved by finding out which systems doesn't have
struct itimerspec and adding entries for them to config.mak.uname.
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 b76dc43..66329e4 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 892afc5..f0d93ef 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 00842ae..3c64251 100644
--- a/configure.ac
+++ b/configure.ac
@@ -768,6 +768,13 @@ AC_CHECK_TYPES([struct itimerval],
[#include <sys/time.h>])
GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
#
+# 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 195eda6..4ef17df 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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 7/9] autoconf: Check for setitimer
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (5 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 6/9] autoconf: Check for struct itimerspec Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 8/9] autoconf: Check for timer_settime Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 9/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
8 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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 3c64251..89eb48f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -939,6 +939,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.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 8/9] autoconf: Check for timer_settime
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (6 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 7/9] autoconf: Check for setitimer Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-29 15:23 ` Keller, Jacob E
2014-08-28 1:04 ` [PATCH 9/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
8 siblings, 1 reply; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
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 evaluates the arguments and fails with ENOSYS to simulate stub
implementations. This will be useful in a following commit.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
This patch can be improved by finding out which systems doesn't have
timer_settime and adding entries for them to config.mak.uname.
Makefile | 21 +++++++++++++++++++++
config.mak.uname | 3 +++
configure.ac | 8 ++++++++
git-compat-util.h | 8 +++++++-
4 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 66329e4..5609e44 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_SETITIMER
#
# Define NO_STRUCT_TIMESPEC if you don't have struct timespec
+# This also implies NO_SETITIMER
#
# Define NO_STRUCT_SIGEVENT if you don't have struct sigevent
+# This also implies NO_SETITIMER
#
# 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 f0d93ef..d04deab 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 89eb48f..354bd38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -945,6 +945,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 4ef17df..91bbf6d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -225,7 +225,13 @@ 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_SETTIMER
+#define timer_create(clockid, sevp, timerp) ((void) (clockid), (void) (sevp), (void) (timerp), errno = ENOSYS, -1)
+#define timer_settime(timer, flags, value, ovalue) ((void) (timer), (void) (flags), (void) (value), (void) (ovalue), errno = ENOSYS, -1)
+#define timer_delete(timer) ((void) (timer), errno = ENOSYS, -1)
#endif
#ifndef NO_LIBGEN_H
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 9/9] Use timer_settime for new platforms
2014-08-28 1:04 [PATCH 0/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
` (7 preceding siblings ...)
2014-08-28 1:04 ` [PATCH 8/9] autoconf: Check for timer_settime Jonas 'Sortie' Termansen
@ 2014-08-28 1:04 ` Jonas 'Sortie' Termansen
2014-08-28 19:43 ` Junio C Hamano
8 siblings, 1 reply; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 1:04 UTC (permalink / raw
To: git; +Cc: Jonas 'Sortie' Termansen
setitimer() is an obsolescent XSI interface and may be removed in a
future standard. Applications should use the core POSIX timer_settime()
instead.
This patch cleans up the progress reporting and changes it to try using
timer_settime, or if that fails, setitimer. If either function is not
provided by the system, then git-compat-util.h provides replacements
that always fail with ENOSYS.
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.
This approach allows the code using timer_settime() and setitimer() to
be simple and readable. My first attempt used #ifdef around each use of
timer_settime(), this quickly turned a into unmaintainable maze of
preprocessor conditionals.
Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
---
builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
progress.c | 34 +++++++++++++++++++++++++++-------
2 files changed, 67 insertions(+), 14 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 4389722..54c85a6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -229,7 +229,8 @@ 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 int is_using_timer_settime;
+static timer_t early_output_timer;
static void log_show_early(struct rev_info *revs, struct commit_list *list)
{
@@ -271,9 +272,21 @@ 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);
+ if (is_using_timer_settime) {
+ 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);
+ } else {
+ struct itimerval value;
+ value.it_value.tv_sec = 0;
+ value.it_value.tv_usec = 500000;
+ value.it_interval.tv_sec = 0;
+ value.it_interval.tv_usec = 0;
+ setitimer(ITIMER_REAL, &value, NULL);
+ }
}
static void early_output(int signal)
@@ -284,6 +297,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 +319,33 @@ 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;
+ if (!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);
+ is_using_timer_settime = 1;
+ } else {
+ struct itimerval value;
+ value.it_value.tv_sec = 0;
+ value.it_value.tv_usec = 100000;
+ value.it_interval.tv_sec = 0;
+ value.it_interval.tv_usec = 0;
+ setitimer(ITIMER_REAL, &value, NULL);
+ is_using_timer_settime = 0;
+ }
}
static void finish_early_output(struct rev_info *rev)
{
int n = estimate_commit_count(rev, rev->commits);
+ if (is_using_timer_settime)
+ timer_delete(early_output_timer);
signal(SIGALRM, SIG_IGN);
show_early_header(rev, "done", n);
}
diff --git a/progress.c b/progress.c
index 412e6b1..b396ee1 100644
--- a/progress.c
+++ b/progress.c
@@ -38,6 +38,8 @@ struct progress {
struct throughput *throughput;
};
+static int is_using_timer_settime;
+static timer_t progress_timer;
static volatile sig_atomic_t progress_update;
static void progress_interval(int signum)
@@ -48,7 +50,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 +60,34 @@ 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;
+ if (!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);
+ is_using_timer_settime = 1;
+ } else {
+ struct itimerval value;
+ value.it_interval.tv_sec = 1;
+ value.it_interval.tv_usec = 0;
+ value.it_value = value.it_interval;
+ setitimer(ITIMER_REAL, &value, NULL);
+ is_using_timer_settime = 0;
+ }
}
static void clear_progress_signal(void)
{
- struct itimerval v = {{0,},};
- setitimer(ITIMER_REAL, &v, NULL);
+ if (is_using_timer_settime) {
+ timer_delete(progress_timer);
+ } else {
+ struct itimerval v = {{0,},};
+ setitimer(ITIMER_REAL, &v, NULL);
+ }
signal(SIGALRM, SIG_IGN);
progress_update = 0;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/9] autoconf: Check for timer_t
2014-08-28 1:04 ` [PATCH 2/9] autoconf: Check for timer_t Jonas 'Sortie' Termansen
@ 2014-08-28 12:03 ` Jonas 'Sortie' Termansen
0 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-28 12:03 UTC (permalink / raw
To: git
It would appear that Darwin does not have timer_t, at least from looking
at the public libc and XNU headers online.
A quick additional change is needed in config.mak.uname:
ifeq ($(uname_S),Darwin)
...
HAVE_DEV_TTY = YesPlease
+ NO_TIMER_T = UnfortunatelyYes
COMPAT_OBJS += compat/precompose_utf8.o
...
endif
I'll include that proper in V2 of my patch series.
Jonas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/9] autoconf: Check for struct itimerval
2014-08-28 1:04 ` [PATCH 5/9] autoconf: Check for struct itimerval Jonas 'Sortie' Termansen
@ 2014-08-28 19:38 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2014-08-28 19:38 UTC (permalink / raw
To: Jonas 'Sortie' Termansen; +Cc: git
Jonas 'Sortie' Termansen <sortie@maxsi.org> writes:
> 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>
> ---
This, 1/9 and later 7/9, are independently good changes to the
current codebase, unlike all the other patches that become only
necessary if/when we want to migrate to timer_settime().
As such, we would prefer to have these "fixes to the current system"
at the beginning of the series before "enhancements to the current
system" patches.
Thanks.
> configure.ac | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 31b3218..00842ae 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -761,6 +761,13 @@ AC_CHECK_TYPES([struct timespec],
> [#include <sys/time.h>])
> GIT_CONF_SUBST([NO_STRUCT_TIMESPEC])
> #
> +# 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=],
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 9/9] Use timer_settime for new platforms
2014-08-28 1:04 ` [PATCH 9/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
@ 2014-08-28 19:43 ` Junio C Hamano
2014-08-29 16:11 ` Keller, Jacob E
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2014-08-28 19:43 UTC (permalink / raw
To: Jonas 'Sortie' Termansen; +Cc: git
Jonas 'Sortie' Termansen <sortie@maxsi.org> writes:
> setitimer() is an obsolescent XSI interface and may be removed in a
> future standard. Applications should use the core POSIX timer_settime()
> instead.
>
> This patch cleans up the progress reporting and changes it to try using
> timer_settime, or if that fails, setitimer. If either function is not
> provided by the system, then git-compat-util.h provides replacements
> that always fail with ENOSYS.
>
> 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.
>
> This approach allows the code using timer_settime() and setitimer() to
> be simple and readable. My first attempt used #ifdef around each use of
> timer_settime(), this quickly turned a into unmaintainable maze of
> preprocessor conditionals.
>
> Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> ---
> builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
> progress.c | 34 +++++++++++++++++++++++++++-------
> 2 files changed, 67 insertions(+), 14 deletions(-)
Yuck. I didn't look at the change very carefully, but are the two
interface so vastly different that you cannot emulate one in terms
of the other, and use a single API at the callsites, isolating the
knowledge of which kind of API is used to interact with the system
timer in one place (perhaps in compat/itimer.c)?
Having to sprinkle "if (is_using_timer_settime)" around means we
need to support two APIs at each and every callsite that wants timer
interrupt actions.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-08-28 1:04 ` [PATCH 8/9] autoconf: Check for timer_settime Jonas 'Sortie' Termansen
@ 2014-08-29 15:23 ` Keller, Jacob E
2014-08-29 16:02 ` Jonas 'Sortie' Termansen
0 siblings, 1 reply; 16+ messages in thread
From: Keller, Jacob E @ 2014-08-29 15:23 UTC (permalink / raw
To: sortie@maxsi.org; +Cc: git@vger.kernel.org
On Thu, 2014-08-28 at 03:04 +0200, Jonas 'Sortie' Termansen wrote:
> 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 evaluates the arguments and fails with ENOSYS to simulate stub
> implementations. This will be useful in a following commit.
>
> Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
>
> ---
>
> This patch can be improved by finding out which systems doesn't have
> timer_settime and adding entries for them to config.mak.uname.
>
> Makefile | 21 +++++++++++++++++++++
> config.mak.uname | 3 +++
> configure.ac | 8 ++++++++
> git-compat-util.h | 8 +++++++-
> 4 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 66329e4..5609e44 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_SETITIMER
Don't you mean it implies NO_TIMER_SETTIME?
It seems to me that these were all added for TIMER_SETTIME, and not
NO_SETTIMER? Or am I just thoroughly confused?
Regards,
Jake
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 8/9] autoconf: Check for timer_settime
2014-08-29 15:23 ` Keller, Jacob E
@ 2014-08-29 16:02 ` Jonas 'Sortie' Termansen
0 siblings, 0 replies; 16+ messages in thread
From: Jonas 'Sortie' Termansen @ 2014-08-29 16:02 UTC (permalink / raw
To: Keller, Jacob E; +Cc: git@vger.kernel.org
> Don't you mean it implies NO_TIMER_SETTIME?
>
> It seems to me that these were all added for TIMER_SETTIME, and not
> NO_SETTIMER? Or am I just thoroughly confused?
Thanks, that's a mistake. I copy-pasted the wrong line. :P
All of those additions should just be:
# This also implies NO_TIMER_SETTIME
Jonas
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 9/9] Use timer_settime for new platforms
2014-08-28 19:43 ` Junio C Hamano
@ 2014-08-29 16:11 ` Keller, Jacob E
0 siblings, 0 replies; 16+ messages in thread
From: Keller, Jacob E @ 2014-08-29 16:11 UTC (permalink / raw
To: gitster@pobox.com; +Cc: git@vger.kernel.org, sortie@maxsi.org
On Thu, 2014-08-28 at 12:43 -0700, Junio C Hamano wrote:
> Jonas 'Sortie' Termansen <sortie@maxsi.org> writes:
>
> > setitimer() is an obsolescent XSI interface and may be removed in a
> > future standard. Applications should use the core POSIX timer_settime()
> > instead.
> >
> > This patch cleans up the progress reporting and changes it to try using
> > timer_settime, or if that fails, setitimer. If either function is not
> > provided by the system, then git-compat-util.h provides replacements
> > that always fail with ENOSYS.
> >
> > 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.
> >
> > This approach allows the code using timer_settime() and setitimer() to
> > be simple and readable. My first attempt used #ifdef around each use of
> > timer_settime(), this quickly turned a into unmaintainable maze of
> > preprocessor conditionals.
> >
> > Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> > ---
> > builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
> > progress.c | 34 +++++++++++++++++++++++++++-------
> > 2 files changed, 67 insertions(+), 14 deletions(-)
>
> Yuck. I didn't look at the change very carefully, but are the two
> interface so vastly different that you cannot emulate one in terms
> of the other, and use a single API at the callsites, isolating the
> knowledge of which kind of API is used to interact with the system
> timer in one place (perhaps in compat/itimer.c)?
>
> Having to sprinkle "if (is_using_timer_settime)" around means we
> need to support two APIs at each and every callsite that wants timer
> interrupt actions.
>
> --
> 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
Agreed! The biggest difference is that timer_settime() uses nanoseconds
value, which is very easy to convert microseconds into nanoseconds.
In fact, you could probably easily provide a wrapper for timer_settime
that works for settimer by simply converting it. This is a much better
approach. (though settimer "could" lose some precision since you ahve to
divide by 1000)...
The other big difference is abstracting away the timer_create aspect
which is ok, since we would always call timer create, and use the
correct timer, but we just fall back to setitimer to use the default
timer.
I'll submit a patch series which does what I think should work, and will
result in less if checks.
Regards,
Jake
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-08-29 16:12 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2014-08-28 1:04 ` [PATCH 2/9] autoconf: Check for timer_t Jonas 'Sortie' Termansen
2014-08-28 12:03 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 3/9] autoconf: Check for struct timespec Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 4/9] autoconf: Check for struct sigevent Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 5/9] autoconf: Check for struct itimerval Jonas 'Sortie' Termansen
2014-08-28 19:38 ` Junio C Hamano
2014-08-28 1:04 ` [PATCH 6/9] autoconf: Check for struct itimerspec Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 7/9] autoconf: Check for setitimer Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 8/9] autoconf: Check for timer_settime Jonas 'Sortie' Termansen
2014-08-29 15:23 ` Keller, Jacob E
2014-08-29 16:02 ` Jonas 'Sortie' Termansen
2014-08-28 1:04 ` [PATCH 9/9] Use timer_settime for new platforms Jonas 'Sortie' Termansen
2014-08-28 19:43 ` Junio C Hamano
2014-08-29 16:11 ` Keller, Jacob E
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).