[PATCH 0/9] Use timer_settime for new platforms

[PATCH 0/9] Use timer_settime for new platforms

[Jonas 'Sortie' Termansen]

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

[PATCH 1/9] git-compat-util.h: Add missing semicolon after struct itimerval

[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

[PATCH 2/9] autoconf: Check for timer_t

[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

[PATCH 3/9] autoconf: Check for struct timespec

[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

[PATCH 4/9] autoconf: Check for struct sigevent

[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

[PATCH 5/9] autoconf: Check for struct itimerval

[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

[PATCH 6/9] autoconf: Check for struct itimerspec

[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

[PATCH 7/9] autoconf: Check for setitimer

[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

[PATCH 8/9] autoconf: Check for timer_settime

[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

[PATCH 9/9] Use timer_settime for new platforms

[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

Re: [PATCH 2/9] autoconf: Check for timer_t

[Jonas 'Sortie' Termansen]

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

Re: [PATCH 5/9] autoconf: Check for struct itimerval

[Junio C Hamano]

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=],

Re: [PATCH 9/9] Use timer_settime for new platforms

[Junio C Hamano]

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.

Re: [PATCH 8/9] autoconf: Check for timer_settime

[Keller, Jacob E]

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

Re: [PATCH 8/9] autoconf: Check for timer_settime

[Jonas 'Sortie' Termansen]

> 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

Re: [PATCH 9/9] Use timer_settime for new platforms

[Keller, Jacob E]

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