On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@xxxxxxxxx> wrote:
> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
> systems being used in production. This change makes compiling git
> less tedious on older platforms without CLOCK_MONOTONIC.
The second sentence is implied by the very presence of this patch,
thus adds no value. I, personally, would drop it.
Also, your sign-off is missing (as mentioned in my previous review[1]).
> ---
> diff --git a/Makefile b/Makefile
> index 7482a4d..af551a0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
> EXTLIBS += -lrt
> endif
>
> +ifdef HAVE_CLOCK_MONOTONIC
> + BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
> +endif
You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)
at the top of Makefile (as mentioned in my previous review[1]), so
that people who build without running 'configure' will know that they
may need to tweak it.
> ifeq ($(TCLTK_PATH),)
> NO_TCLTK = NoThanks
> endif
> diff --git a/configure.ac b/configure.ac
> index dcc4bf0..424dec5 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
> [CHARSET_LIB=-lcharset])])
> GIT_CONF_SUBST([CHARSET_LIB])
> #
> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
> +GIT_CHECK_FUNC(clock_gettime,
> +[HAVE_CLOCK_GETTIME=YesPlease],
> +[HAVE_CLOCK_GETTIME=])
> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
You could simplify the above four lines to this one-liner:
GIT_CHECK_FUNC(clock_gettime,
GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))
> +
> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
> +AC_LANG_PROGRAM([[
> +#include <time.h>
> +clockid_t id = CLOCK_MONOTONIC;
> +]], [])])
No need to pass empty trailing arguments in m4. It's customary to drop
them altogether (since they are implied).
> +#
> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
> + [AC_MSG_RESULT([yes])
> + HAVE_CLOCK_MONOTONIC=YesPlease],
> + [AC_MSG_RESULT([no])
> + HAVE_CLOCK_MONOTONIC=])
> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
Ditto regarding simplification:
AC_MSG_CHECKING([for CLOCK_MONOTONIC])
AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
[AC_MSG_RESULT([yes])
GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
[AC_MSG_RESULT([no])])
> +#
> # Define NO_SETITIMER if you don't have setitimer.
> GIT_CHECK_FUNC(setitimer,
> [NO_SETITIMER=],
[1]: http://article.gmane.org/gmane.comp.version-control.git/261630
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html