{"thread":{"id":"38218","subject":"[PATCH 1/3] configure.ac: check tv_nsec field in struct stat","startedAt":"2014-12-21T18:53:34Z","lastAt":"2015-01-08T05:54:38Z","messageCount":22,"participants":["Reuben Hawkins","Eric Sunshine","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"253895","messageId":"1419188016-26134-1-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":null,"subject":"[PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2014-12-21T18:53:34Z","receivedAt":"2014-12-21T18:53:34Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"This check will automatically set the correct NO_NSEC setting.\n---\n configure.ac | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/configure.ac b/configure.ac\nindex 6af9647..3cfdd51 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -754,6 +754,25 @@ AC_CHECK_TYPES([struct itimerval],\n [#include <sys/time.h>])\n GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n #\n+# Define HAVE_ST_MTIM=No if you don't have struct stat.st_mtim.tv_nsec.\n+AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec],\n+[HAVE_ST_MTIM=Yes],\n+[HAVE_ST_MTIM=No],\n+[#include <sys/stat.h>])\n+#\n+# Define HAVE_ST_MTIMESPEC=No if you don't have struct stat.st_mtimespec.tv_nsec.\n+AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec],\n+[HAVE_ST_MTIMESPEC=Yes],\n+[HAVE_ST_MTIMESPEC=No],\n+[#include <sys/stat.h>])\n+#\n+# Define NO_NSEC if both HAVE_ST_MTIMESPEC and HAVE_ST_MTIM are set to No.\n+if test '(' \"$HAVE_ST_MTIM\" = \"No\" ')' -a '(' \"$HAVE_ST_MTIMESPEC\" = \"No\" ')' ; then\n+\tNO_NSEC=YesPlease\n+\tGIT_CONF_SUBST([NO_NSEC])\n+fi\n+\n+#\n # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n AC_CHECK_MEMBER(struct dirent.d_ino,\n [NO_D_INO_IN_DIRENT=],\n-- \n2.2.0.GIT\n"},{"id":"253897","messageId":"1419188016-26134-2-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"1419188016-26134-1-git-send-email-reubenhwk@gmail.com","subject":"[PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2014-12-21T18:53:35Z","receivedAt":"2014-12-21T18:53:35Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\nsystems being used in production.  This change makes compiling git\nless tedious on older platforms.\n---\n configure.ac | 26 ++++++++++++++++++++++++++\n trace.c      |  2 +-\n 2 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 3cfdd51..3900044 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -736,8 +736,10 @@ GIT_UNSTASH_FLAGS($ICONVDIR)\n \n GIT_CONF_SUBST([OLD_ICONV])\n \n+\n ## Checks for typedefs, structures, and compiler characteristics.\n AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])\n+\n #\n TYPE_SOCKLEN_T\n case $ac_cv_type_socklen_t in\n@@ -930,6 +932,30 @@ AC_CHECK_LIB([iconv], [locale_charset],\n                      [CHARSET_LIB=-lcharset])])\n GIT_CONF_SUBST([CHARSET_LIB])\n #\n+# Define NO_CLOCK_GETTIME if you don't have clock_gettime.\n+GIT_CHECK_FUNC(clock_gettime,\n+[HAVE_CLOCK_GETTIME=Yes],\n+[HAVE_CLOCK_GETTIME=])\n+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n+\n+AC_DEFUN([CLOCK_MONOTONIC_SRC], [\n+AC_LANG_PROGRAM([[\n+#include <time.h>\n+clockid_t id = CLOCK_MONOTONIC;\n+]], [])])\n+\n+#\n+# Define NO_CLOCK_MONOTONIC on really old systems that are still in production\n+# if you need GIT to compile but can't update the machine otherwise.\n+AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n+\t[AC_MSG_RESULT([yes])\n+\tHAVE_CLOCK_MONOTONIC=Yes],\n+\t[AC_MSG_RESULT([no])\n+\tHAVE_CLOCK_MONOTONIC=])\n+\n+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n+#\n # Define NO_SETITIMER if you don't have setitimer.\n GIT_CHECK_FUNC(setitimer,\n [NO_SETITIMER=],\ndiff --git a/trace.c b/trace.c\nindex 4778608..bfbd48f 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)\n \treturn !!get_trace_fd(key);\n }\n \n-#ifdef HAVE_CLOCK_GETTIME\n+#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)\n \n static inline uint64_t highres_nanos(void)\n {\n-- \n2.2.0.GIT\n"},{"id":"253896","messageId":"1419188016-26134-3-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"1419188016-26134-1-git-send-email-reubenhwk@gmail.com","subject":"[PATCH 3/3] configure.ac,imap-send.c: check HMAC_CTX_cleanup","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2014-12-21T18:53:36Z","receivedAt":"2014-12-21T18:53:36Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"Older versions of OpenSSL have HMAC_cleanup, but not HMAC_CTX_cleanup.\nThis change checks for both, uses HMAC_CTX_cleanup on platforms which\nhave it, otherwise falls back to HMAC_cleanup.\n\nThis changes makes building GIT on older platforms less tedious.\n---\n Makefile     |  6 ++++++\n configure.ac | 13 +++++++++++++\n imap-send.c  |  6 ++++++\n 3 files changed, 25 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 7482a4d..a495d94 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1059,6 +1059,12 @@ ifndef NO_OPENSSL\n \tifdef NEEDS_CRYPTO_WITH_SSL\n \t\tOPENSSL_LIBSSL += -lcrypto\n \tendif\n+\tifdef HAVE_HMAC_CTX_CLEANUP\n+\t\tBASIC_CFLAGS += -DHAVE_HMAC_CTX_CLEANUP\n+\tendif\n+\tifdef HAVE_HMAC_CLEANUP\n+\t\tBASIC_CFLAGS += -DHAVE_HMAC_CLEANUP\n+\tendif\n else\n \tBASIC_CFLAGS += -DNO_OPENSSL\n \tBLK_SHA1 = 1\ndiff --git a/configure.ac b/configure.ac\nindex 3900044..b22788c 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -899,6 +899,7 @@ GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS])\n ## Checks for library functions.\n ## (in default C library and libraries checked by AC_CHECK_LIB)\n AC_MSG_NOTICE([CHECKS for library functions])\n+\n #\n # Define NO_LIBGEN_H if you don't have libgen.h.\n AC_CHECK_HEADER([libgen.h],\n@@ -932,6 +933,18 @@ AC_CHECK_LIB([iconv], [locale_charset],\n                      [CHARSET_LIB=-lcharset])])\n GIT_CONF_SUBST([CHARSET_LIB])\n #\n+# Define HAVE_HMAC_CTX_CLEANUP=Yes if we have the newer HMAC cleanup function\n+AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],\n+\t[HAVE_HMAC_CTX_CLEANUP=Yes],\n+\t[], [])\n+GIT_CONF_SUBST([HAVE_HMAC_CTX_CLEANUP])\n+#\n+# Define HAVE_HMAC_CLEANUP=Yes if we have the older HMAC cleanup function\n+AC_CHECK_LIB([crypto], [HMAC_cleanup],\n+\t[HAVE_HMAC_CLEANUP=Yes],\n+\t[], [])\n+GIT_CONF_SUBST([HAVE_HMAC_CLEANUP])\n+#\n # Define NO_CLOCK_GETTIME if you don't have clock_gettime.\n GIT_CHECK_FUNC(clock_gettime,\n [HAVE_CLOCK_GETTIME=Yes],\ndiff --git a/imap-send.c b/imap-send.c\nindex 70bcc7a..eec2378 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -861,7 +861,13 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)\n \tHMAC_Init(&hmac, (unsigned char *)pass, strlen(pass), EVP_md5());\n \tHMAC_Update(&hmac, (unsigned char *)challenge, decoded_len);\n \tHMAC_Final(&hmac, hash, NULL);\n+#if defined(HAVE_HMAC_CTX_CLEANUP)\n \tHMAC_CTX_cleanup(&hmac);\n+#elif defined(HAVE_HMAC_CLEANUP)\n+\tHMAC_cleanup(&hmac);\n+#else\n+# error \"no HMAC_cleanup function\"\n+#endif\n \n \thex[32] = 0;\n \tfor (i = 0; i < 16; i++) {\n-- \n2.2.0.GIT\n"},{"id":"253902","messageId":"CAPig+cQWpq12axtJNsjoiKLKTpi6_xreS1fYDXYBtNv0Bj=T2Q@mail.gmail.com","threadId":"38218","inReplyTo":"1419188016-26134-1-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-21T20:20:16Z","receivedAt":"2014-12-21T20:20:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> This check will automatically set the correct NO_NSEC setting.\n\nMissing sign-off. See git/Documentation/SubmittingPatches.\n\n> ---\n> diff --git a/configure.ac b/configure.ac\n> index 6af9647..3cfdd51 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -754,6 +754,25 @@ AC_CHECK_TYPES([struct itimerval],\n>  [#include <sys/time.h>])\n>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n>  #\n> +# Define HAVE_ST_MTIM=No if you don't have struct stat.st_mtim.tv_nsec.\n\nThis comment is misleading. HAVE_ST_MTIM is never actually #defined or\nset manually by the user, or used anywhere outside of the conditional\nbelow. Also, the name itself is misleading: HAVE_ST_MTIM_NSEC would be\nmore appropriate.\n\n> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec],\n> +[HAVE_ST_MTIM=Yes],\n> +[HAVE_ST_MTIM=No],\n\nIn Autoconf, it's customary to use lowercase values (such as \"yes\"\nrather than \"Yes\") for these temporary variables. A \"no\" value is\nusually represented by an empty assignment (HAVE_ST_MTIM=). However,\nAC_CHECK_MEMBER() assigns the test result automatically to a shell\nvariable (in this case, named\nac_cv_member_struct_stat_st_mtim_tv_nsec), so there is no need to\ninvent your own (HAVE_ST_MTIM).\n\nSo, you can reduce this to:\n\n   AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n\nand later check the value with\n\n    test x$ac_cv_member_struct_stat_st_mtim_tv_nsec = yes\n\n> +[#include <sys/stat.h>])\n> +#\n> +# Define HAVE_ST_MTIMESPEC=No if you don't have struct stat.st_mtimespec.tv_nsec.\n\nDitto regarding misleading comment and variable name.\n\n> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec],\n> +[HAVE_ST_MTIMESPEC=Yes],\n> +[HAVE_ST_MTIMESPEC=No],\n> +[#include <sys/stat.h>])\n> +#\n> +# Define NO_NSEC if both HAVE_ST_MTIMESPEC and HAVE_ST_MTIM are set to No.\n> +if test '(' \"$HAVE_ST_MTIM\" = \"No\" ')' -a '(' \"$HAVE_ST_MTIMESPEC\" = \"No\" ')' ; then\n\nUse of  'test -a' is unportable. The Autoconf manual has this to say:\n\n    The -a, -o, ‘(’, and ‘)’ operands are not present in all\n    implementations, and have been marked obsolete by Posix 2008.\n\nInstead, use:\n\n    test ... && test ....\n\n> +       NO_NSEC=YesPlease\n> +       GIT_CONF_SUBST([NO_NSEC])\n\ngit-compat-util.h also needs to know if it should use st_ctimespec\nrather than st_ctim. By this point, although indirect, you've\ndetermined which of the two is available (if any), so as a bonus, you\ncan tell the build system about that, as well. Something like this,\nperhaps:\n\n    if test x$ac_cv_member_struct_stat_st_mtimspec_tv_nsec = xyes; then\n        USE_ST_TIMESPEC=YesPlease\n        GIT_CONF_SUBST([USE_ST_TIMESPEC])\n    elif test x$ac_cv_member_struct_stat_st_mtime_tv_nsec != xyes; then\n        NO_NSEC=YesPlease\n        GIT_CONF_SUBST([NO_NSEC])\n    fi\n\n> +fi\n> +\n> +#\n>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n>  AC_CHECK_MEMBER(struct dirent.d_ino,\n>  [NO_D_INO_IN_DIRENT=],\n> --\n> 2.2.0.GIT\n"},{"id":"253906","messageId":"CAPig+cRu9SoFVmgmU2BF=oi4AUZWCs8RFsjEuKtEvm-wzRkk1g@mail.gmail.com","threadId":"38218","inReplyTo":"1419188016-26134-2-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-21T20:54:30Z","receivedAt":"2014-12-21T20:54:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\n> systems being used in production.  This change makes compiling git\n> less tedious on older platforms.\n\nMissing sign-off.\n\n> ---\n> diff --git a/configure.ac b/configure.ac\n> index 3cfdd51..3900044 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -736,8 +736,10 @@ GIT_UNSTASH_FLAGS($ICONVDIR)\n>\n>  GIT_CONF_SUBST([OLD_ICONV])\n>\n> +\n>  ## Checks for typedefs, structures, and compiler characteristics.\n>  AC_MSG_NOTICE([CHECKS for typedefs, structures, and compiler characteristics])\n> +\n\nSneaking in a couple whitespace changes unrelated the patch's stated\npurpose. Such cleanup should be done typically as a separate\npreparatory patch (though they probably aren't warranted in this\ncase).\n\n>  #\n>  TYPE_SOCKLEN_T\n>  case $ac_cv_type_socklen_t in\n> @@ -930,6 +932,30 @@ AC_CHECK_LIB([iconv], [locale_charset],\n>                       [CHARSET_LIB=-lcharset])])\n>  GIT_CONF_SUBST([CHARSET_LIB])\n>  #\n> +# Define NO_CLOCK_GETTIME if you don't have clock_gettime.\n\nThe comment talks about NO_CLOCK_GETTIME, however, the code names it\nHAVE_CLOCK_GETTIME.\n\n> +GIT_CHECK_FUNC(clock_gettime,\n> +[HAVE_CLOCK_GETTIME=Yes],\n\nFor consistency with other build values: s/Yes/YesPlease/\n\n> +[HAVE_CLOCK_GETTIME=])\n> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n> +\n> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [\n> +AC_LANG_PROGRAM([[\n> +#include <time.h>\n> +clockid_t id = CLOCK_MONOTONIC;\n> +]], [])])\n> +\n> +#\n> +# Define NO_CLOCK_MONOTONIC on really old systems that are still in production\n> +# if you need GIT to compile but can't update the machine otherwise.\n\nThe comment talks about NO_CLOCK_MONOTONIC, however, the code names it\nHAVE_CLOCK_MONOTONIC.\n\nThe \"old systems .. need GIT to compile ... can't update\" commentary\nis unnecessary. That's pretty well implied by the mere use of\nAutoconf. It would be sufficient to say \"Define ... if you don't have\nCLOCK_MONOTONIC.\"\n\n> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n> +       [AC_MSG_RESULT([yes])\n> +       HAVE_CLOCK_MONOTONIC=Yes],\n\nConsistency: s/Yes/YesPlease/\n\n> +       [AC_MSG_RESULT([no])\n> +       HAVE_CLOCK_MONOTONIC=])\n> +\n> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n\nBe aware that this does not actually #define HAVE_CLOCK_MONOTONIC\nanywhere. It only sets a Makefile variable in config.mak.autogen.\n(Consequently, this patch it actually breaks support for this feature\nin trace.c for people who build without running the configure script.)\nYou need to do extra work to get the #define.\n\nIn particular, add a comment to Makefile describing the\nHAVE_CLOCK_MONOTONIC setting. (Placing it just below the existing\ncomment for HAVE_CLOCK_GETTIME would make loads of sense.)\n\nNext, add code to the Makefile to actually #define\nHAVE_CLOCK_MONOTONIC. Something like this (alongside the existing\nHAVE_CLOCK_GETTIME code):\n\n    ifdef HAVE_CLOCK_MONOTONIC\n        BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n    endif\n\nFinally, update config.make.uname to set\nHAVE_CLOCK_MONOTONIC=YesPlease on platforms which are likely to have\nit (for people who don't run the configure script). At the very least,\nit should be enabled by default for Linux.\n\n> +#\n>  # Define NO_SETITIMER if you don't have setitimer.\n>  GIT_CHECK_FUNC(setitimer,\n>  [NO_SETITIMER=],\n> diff --git a/trace.c b/trace.c\n> index 4778608..bfbd48f 100644\n> --- a/trace.c\n> +++ b/trace.c\n> @@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)\n>         return !!get_trace_fd(key);\n>  }\n>\n> -#ifdef HAVE_CLOCK_GETTIME\n> +#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)\n>\n>  static inline uint64_t highres_nanos(void)\n>  {\n> --\n> 2.2.0.GIT\n"},{"id":"253907","messageId":"CAPig+cSFAzsUj2sOaCyPTu_U8_c5qkF1qQo4nzecVzY0TqmJpg@mail.gmail.com","threadId":"38218","inReplyTo":"1419188016-26134-3-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 3/3] configure.ac,imap-send.c: check HMAC_CTX_cleanup","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-21T21:28:18Z","receivedAt":"2014-12-21T21:28:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> Older versions of OpenSSL have HMAC_cleanup, but not HMAC_CTX_cleanup.\n> This change checks for both, uses HMAC_CTX_cleanup on platforms which\n> have it, otherwise falls back to HMAC_cleanup.\n\nOn this project, imperative mood is preferred. Also, this _is_ a patch\n(implying \"change\"), so it's not necessary to say \"This change\". An\nalternate way to write the above might be:\n\n    Check for both, and use HMAC_CTX_cleanup when available,\n    otherwise HMAC_cleanup.\n\n> This changes makes building GIT on older platforms less tedious.\n\nThis is implied by the first paragraph, so no need to state it here explicitly.\n\nMore below.\n\n> ---\n> diff --git a/Makefile b/Makefile\n> index 7482a4d..a495d94 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1059,6 +1059,12 @@ ifndef NO_OPENSSL\n>         ifdef NEEDS_CRYPTO_WITH_SSL\n>                 OPENSSL_LIBSSL += -lcrypto\n>         endif\n> +       ifdef HAVE_HMAC_CTX_CLEANUP\n> +               BASIC_CFLAGS += -DHAVE_HMAC_CTX_CLEANUP\n> +       endif\n> +       ifdef HAVE_HMAC_CLEANUP\n> +               BASIC_CFLAGS += -DHAVE_HMAC_CLEANUP\n> +       endif\n\nSee discussion below regarding build breakage due to this incomplete change.\n\n>  else\n>         BASIC_CFLAGS += -DNO_OPENSSL\n>         BLK_SHA1 = 1\n> diff --git a/configure.ac b/configure.ac\n> index 3900044..b22788c 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -899,6 +899,7 @@ GIT_CONF_SUBST([SNPRINTF_RETURNS_BOGUS])\n>  ## Checks for library functions.\n>  ## (in default C library and libraries checked by AC_CHECK_LIB)\n>  AC_MSG_NOTICE([CHECKS for library functions])\n> +\n\nSneaking in unrelated whitespace change.\n\n>  #\n>  # Define NO_LIBGEN_H if you don't have libgen.h.\n>  AC_CHECK_HEADER([libgen.h],\n> @@ -932,6 +933,18 @@ AC_CHECK_LIB([iconv], [locale_charset],\n>                       [CHARSET_LIB=-lcharset])])\n>  GIT_CONF_SUBST([CHARSET_LIB])\n>  #\n> +# Define HAVE_HMAC_CTX_CLEANUP=Yes if we have the newer HMAC cleanup function\n\ns/Yes/YesPease/\ns/$/./\n\n> +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],\n> +       [HAVE_HMAC_CTX_CLEANUP=Yes],\n> +       [], [])\n\nNo need to pass trailing empty [] arguments explicitly in m4.\n\n> +GIT_CONF_SUBST([HAVE_HMAC_CTX_CLEANUP])\n\nYou need to document this new variable in the comment block at the top\nof Makefile so that people who build without running the configure\nscript will know that they may need to tweak it.\n\n> +#\n> +# Define HAVE_HMAC_CLEANUP=Yes if we have the older HMAC cleanup function\n\ns/Yes/YesPease/\ns/$/./\n\n> +AC_CHECK_LIB([crypto], [HMAC_cleanup],\n> +       [HAVE_HMAC_CLEANUP=Yes],\n> +       [], [])\n> +GIT_CONF_SUBST([HAVE_HMAC_CLEANUP])\n\nThis test for HMAC_cleanup() is likely overkill until we encounter a\nsystem which has neither HMAC_CTX_cleanup() nor HMAC_cleanup(). For\nthe present, it's probably safe to assume that the older\nHMAC_cleanup() is available everywhere when HMAC_CTX_cleanup() is not.\n\n> +#\n>  # Define NO_CLOCK_GETTIME if you don't have clock_gettime.\n>  GIT_CHECK_FUNC(clock_gettime,\n>  [HAVE_CLOCK_GETTIME=Yes],\n> diff --git a/imap-send.c b/imap-send.c\n> index 70bcc7a..eec2378 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -861,7 +861,13 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)\n>         HMAC_Init(&hmac, (unsigned char *)pass, strlen(pass), EVP_md5());\n>         HMAC_Update(&hmac, (unsigned char *)challenge, decoded_len);\n>         HMAC_Final(&hmac, hash, NULL);\n> +#if defined(HAVE_HMAC_CTX_CLEANUP)\n>         HMAC_CTX_cleanup(&hmac);\n> +#elif defined(HAVE_HMAC_CLEANUP)\n> +       HMAC_cleanup(&hmac);\n> +#else\n> +# error \"no HMAC_cleanup function\"\n> +#endif\n\nSeveral issues:\n\nThis change utterly breaks the build for people who do not run the\nconfigure script since neither HAVE_HMAC_CTX_CLEANUP nor\nHAVE_HMAC_CLEANUP Makefile variable is set (which means that the\ncorresponding C #defines are never made).\n\nSomehow, you need to make sure that Makefile variable\nHAVE_HMAC_CTX_CLEANUP is set by default since HMAC_CTX_cleanup() is\navailable on most platforms, and only a few much older ones lack it.\nTypically, you would do this in config.make.uname, however, this is\none of those instances in which it is easier to disable a feature for\nthe rare case than to enable it for the common case. Therefore, it\nwould be more appropriate to name this variable NO_HMAC_CTX_CLEANUP\n(and document it appropriately in Makefile).\n\nFinally, rather than polluting imap-send.c with #if conditionals,\nrelegate the ugliness to git-compat-util.h. For instance, within the\n'#ifndef NO_OPENSSL' conditional, you might add:\n\n    #ifdef NO_HMAC_CTX_CLEANUP\n    #define HMAC_CTX_cleanup HMAC_cleanup\n    #endif\n\nand then you don't have to touch imap-send.c at all.\n\n>         hex[32] = 0;\n>         for (i = 0; i < 16; i++) {\n> --\n> 2.2.0.GIT\n"},{"id":"253908","messageId":"CAPig+cQmFBJYu-fJmOWV5zVRqPphi=aA+fJ8sBAVnHyXPNbAJQ@mail.gmail.com","threadId":"38218","inReplyTo":"CAPig+cQWpq12axtJNsjoiKLKTpi6_xreS1fYDXYBtNv0Bj=T2Q@mail.gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-21T21:47:02Z","receivedAt":"2014-12-21T21:47:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 21, 2014 at 3:20 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec],\n>> +[HAVE_ST_MTIM=Yes],\n>> +[HAVE_ST_MTIM=No],\n>\n> In Autoconf, it's customary to use lowercase values (such as \"yes\"\n> rather than \"Yes\") for these temporary variables. A \"no\" value is\n> usually represented by an empty assignment (HAVE_ST_MTIM=). However,\n> AC_CHECK_MEMBER() assigns the test result automatically to a shell\n> variable (in this case, named\n> ac_cv_member_struct_stat_st_mtim_tv_nsec), so there is no need to\n> invent your own (HAVE_ST_MTIM).\n>\n> So, you can reduce this to:\n>\n>    AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n>\n> and later check the value with\n>\n>     test x$ac_cv_member_struct_stat_st_mtim_tv_nsec = yes\n\nApple auto-correction strikes again: s/yes/xyes/\n"},{"id":"253913","messageId":"20141222041232.GA170128@vauxhall.crustytoothpaste.net","threadId":"38218","inReplyTo":"1419188016-26134-2-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-22T04:12:32Z","receivedAt":"2014-12-22T04:12:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Dec 21, 2014 at 10:53:35AM -0800, Reuben Hawkins wrote:\n> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\n> systems being used in production.  This change makes compiling git\n> less tedious on older platforms.\n\nWhile I'm not opposed to this change, I expect that you'll find there's\nlots of subtle breakage when trying to compile (and run the testsuite\nfor) git on RHEL 3.  I've spoken up before to prevent breakage on\nRHEL/CentOS 5 (since $DAYJOB still supports it), but I'm not sure\nanyone's looking out for something as old as RHEL 3.  I expect you'll\nprobably have some pain points with perl and curl, among others.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253918","messageId":"CAD_8n+TX-foWkRr9BrFX9PHu387WfTCD+w84m7QLjwhMwEKNjQ@mail.gmail.com","threadId":"38218","inReplyTo":"20141222041232.GA170128@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2014-12-22T12:36:17Z","receivedAt":"2014-12-22T12:36:17Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"On Sun, Dec 21, 2014 at 8:12 PM, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> On Sun, Dec 21, 2014 at 10:53:35AM -0800, Reuben Hawkins wrote:\n>> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\n>> systems being used in production.  This change makes compiling git\n>> less tedious on older platforms.\n>\n> While I'm not opposed to this change, I expect that you'll find there's\n> lots of subtle breakage when trying to compile (and run the testsuite\n> for) git on RHEL 3.  I've spoken up before to prevent breakage on\n> RHEL/CentOS 5 (since $DAYJOB still supports it), but I'm not sure\n> anyone's looking out for something as old as RHEL 3.  I expect you'll\n> probably have some pain points with perl and curl, among others.\n\nYes, there are pain points with perl and curl.  Those I've disable\nwith other compile options when building on RHEL3, but reducing the\nnumber of options I have to set manually and increasing the number of\nautomatic checks with configure is helpful.   Sometime over the next\nfew days I'll submit a v2 of the patches with Eric's comments taken\ninto account.\n\n> --\n> brian m. carlson / brian with sandals: Houston, Texas, US\n> +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\n> OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"254421","messageId":"1420662236-27593-1-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"CAPig+cSFAzsUj2sOaCyPTu_U8_c5qkF1qQo4nzecVzY0TqmJpg@mail.gmail.com","subject":"v2 patches for fixes on RHEL3","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T20:23:53Z","receivedAt":"2015-01-07T20:23:53Z","isPatch":false,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"These patches add a few autoconfig checks for nanosecond resolution fields in\nstruct stat, CLOCK_MONOTONIC, and HMAC_CTX_cleanup.  I'm fairly sure I've fixed\nthe concerns/doubts the first set of patched raised.\n\n(I rarely use git-send-email so forgive me if I botch this)\n"},{"id":"254423","messageId":"1420662236-27593-2-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-1-git-send-email-reubenhwk@gmail.com","subject":"[PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T20:23:54Z","receivedAt":"2015-01-07T20:23:54Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"This check will automatically set the correct NO_NSEC setting.\n---\n configure.ac | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/configure.ac b/configure.ac\nindex 6af9647..dcc4bf0 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],\n [#include <sys/time.h>])\n GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n #\n+# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist\n+# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist\n+AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])\n+AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n+if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then\n+\tUSE_ST_TIMESPEC=YesPlease\n+\tGIT_CONF_SUBST([USE_ST_TIMESPEC])\n+elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then\n+\tNO_NSEC=YesPlease\n+\tGIT_CONF_SUBST([NO_NSEC])\n+fi\n+#\n # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n AC_CHECK_MEMBER(struct dirent.d_ino,\n [NO_D_INO_IN_DIRENT=],\n-- \n2.2.0.68.g8f72f0c.dirty\n"},{"id":"254420","messageId":"1420662236-27593-3-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-1-git-send-email-reubenhwk@gmail.com","subject":"[PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T20:23:55Z","receivedAt":"2015-01-07T20:23:55Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\nsystems being used in production.  This change makes compiling git\nless tedious on older platforms without CLOCK_MONOTONIC.\n---\n Makefile         |  4 ++++\n config.mak.uname |  1 +\n configure.ac     | 22 ++++++++++++++++++++++\n trace.c          |  2 +-\n 4 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 7482a4d..af551a0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME\n \tEXTLIBS += -lrt\n endif\n \n+ifdef HAVE_CLOCK_MONOTONIC\n+\tBASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n+endif\n+\n ifeq ($(TCLTK_PATH),)\n NO_TCLTK = NoThanks\n endif\ndiff --git a/config.mak.uname b/config.mak.uname\nindex a2f380f..926773e 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)\n \tLIBC_CONTAINS_LIBINTL = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n \tHAVE_CLOCK_GETTIME = YesPlease\n+\tHAVE_CLOCK_MONOTONIC = YesPlease\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tHAVE_ALLOCA_H = YesPlease\ndiff --git a/configure.ac b/configure.ac\nindex dcc4bf0..424dec5 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],\n                      [CHARSET_LIB=-lcharset])])\n GIT_CONF_SUBST([CHARSET_LIB])\n #\n+# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.\n+GIT_CHECK_FUNC(clock_gettime,\n+[HAVE_CLOCK_GETTIME=YesPlease],\n+[HAVE_CLOCK_GETTIME=])\n+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n+\n+AC_DEFUN([CLOCK_MONOTONIC_SRC], [\n+AC_LANG_PROGRAM([[\n+#include <time.h>\n+clockid_t id = CLOCK_MONOTONIC;\n+]], [])])\n+\n+#\n+# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.\n+AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n+\t[AC_MSG_RESULT([yes])\n+\tHAVE_CLOCK_MONOTONIC=YesPlease],\n+\t[AC_MSG_RESULT([no])\n+\tHAVE_CLOCK_MONOTONIC=])\n+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n+#\n # Define NO_SETITIMER if you don't have setitimer.\n GIT_CHECK_FUNC(setitimer,\n [NO_SETITIMER=],\ndiff --git a/trace.c b/trace.c\nindex 4778608..bfbd48f 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)\n \treturn !!get_trace_fd(key);\n }\n \n-#ifdef HAVE_CLOCK_GETTIME\n+#if defined(HAVE_CLOCK_GETTIME) && defined(HAVE_CLOCK_MONOTONIC)\n \n static inline uint64_t highres_nanos(void)\n {\n-- \n2.2.0.68.g8f72f0c.dirty\n"},{"id":"254422","messageId":"1420662236-27593-4-git-send-email-reubenhwk@gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-1-git-send-email-reubenhwk@gmail.com","subject":"[PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T20:23:56Z","receivedAt":"2015-01-07T20:23:56Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.\nNewer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and\nfall back to HMAC_cleanup when the newer function is missing.\n---\n Makefile          | 3 +++\n configure.ac      | 7 +++++++\n git-compat-util.h | 3 +++\n 3 files changed, 13 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex af551a0..d3c2b58 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1059,6 +1059,9 @@ ifndef NO_OPENSSL\n \tifdef NEEDS_CRYPTO_WITH_SSL\n \t\tOPENSSL_LIBSSL += -lcrypto\n \tendif\n+\tifdef NO_HMAC_CTX_CLEANUP\n+\t\tBASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP\n+\tendif\n else\n \tBASIC_CFLAGS += -DNO_OPENSSL\n \tBLK_SHA1 = 1\ndiff --git a/configure.ac b/configure.ac\nindex 424dec5..c282663 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -923,6 +923,13 @@ AC_CHECK_LIB([iconv], [locale_charset],\n                      [CHARSET_LIB=-lcharset])])\n GIT_CONF_SUBST([CHARSET_LIB])\n #\n+# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.\n+AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],\n+\t[NO_HMAC_CTX_CLEANUP=],\n+\t[NO_HMAC_CTX_CLEANUP=YesPlease],\n+\t[])\n+GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP])\n+#\n # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.\n GIT_CHECK_FUNC(clock_gettime,\n [HAVE_CLOCK_GETTIME=YesPlease],\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 400e921..2fdca2d 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -213,6 +213,9 @@ extern char *gitbasename(char *);\n #ifndef NO_OPENSSL\n #include <openssl/ssl.h>\n #include <openssl/err.h>\n+#ifdef NO_HMAC_CTX_CLEANUP\n+#define HMAC_CTX_cleanup HMAC_cleanup\n+#endif\n #endif\n \n /* On most systems <netdb.h> would have given us this, but\n-- \n2.2.0.68.g8f72f0c.dirty\n"},{"id":"254427","messageId":"CAPig+cSrht0fovEWhEknxPPzwZhA7vzeLcM+omaM7PR752GfGw@mail.gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-2-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-07T21:19:25Z","receivedAt":"2015-01-07T21:19:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> This check will automatically set the correct NO_NSEC setting.\n\nThis commit message neglects to mention the important point that\nyou're also now setting USE_ST_TIMESPEC when detected. You might\nrevise the message like this:\n\n    Detect 'tv_nsec' field in 'struct stat' and set Makefile variable\n    NO_NSEC appropriately.\n\n    A side-effect of the above detection is that we also determine\n    whether 'stat.st_mtimespec' is available, so, as a bonus, set the\n    Makefile variable USE_ST_TIMESPEC, as well.\n\nAlso, your sign-off is missing (as mentioned in my previous review[1]).\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/261626\n\n> ---\n>  configure.ac | 12 ++++++++++++\n>  1 file changed, 12 insertions(+)\n>\n> diff --git a/configure.ac b/configure.ac\n> index 6af9647..dcc4bf0 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],\n>  [#include <sys/time.h>])\n>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n>  #\n> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist\n\nIt would be slightly more accurate to drop the \".tv_nsec\" bit from this comment.\n\nAlso: s/exist/exists./\n\n> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist\n\nPerhaps wrap this long comment over two lines.\n\nAlso: s/exist/exist./\n\n> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])\n> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then\n> +       USE_ST_TIMESPEC=YesPlease\n> +       GIT_CONF_SUBST([USE_ST_TIMESPEC])\n> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then\n> +       NO_NSEC=YesPlease\n> +       GIT_CONF_SUBST([NO_NSEC])\n> +fi\n> +#\n>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n>  AC_CHECK_MEMBER(struct dirent.d_ino,\n>  [NO_D_INO_IN_DIRENT=],\n> --\n> 2.2.0.68.g8f72f0c.dirty\n"},{"id":"254428","messageId":"CAD_8n+RBJHDjFCEY8GmbsqyyyxPEo71Lg03M7y9dMcu74y6m-w@mail.gmail.com","threadId":"38218","inReplyTo":"CAPig+cSrht0fovEWhEknxPPzwZhA7vzeLcM+omaM7PR752GfGw@mail.gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T21:33:35Z","receivedAt":"2015-01-07T21:33:35Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>> This check will automatically set the correct NO_NSEC setting.\n>\n> This commit message neglects to mention the important point that\n> you're also now setting USE_ST_TIMESPEC when detected. You might\n> revise the message like this:\n>\n>     Detect 'tv_nsec' field in 'struct stat' and set Makefile variable\n>     NO_NSEC appropriately.\n>\n>     A side-effect of the above detection is that we also determine\n>     whether 'stat.st_mtimespec' is available, so, as a bonus, set the\n>     Makefile variable USE_ST_TIMESPEC, as well.\n>\n> Also, your sign-off is missing (as mentioned in my previous review[1]).\n>\n> [1]: http://article.gmane.org/gmane.comp.version-control.git/261626\n>\n>> ---\n>>  configure.ac | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/configure.ac b/configure.ac\n>> index 6af9647..dcc4bf0 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],\n>>  [#include <sys/time.h>])\n>>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n>>  #\n>> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist\n>\n> It would be slightly more accurate to drop the \".tv_nsec\" bit from this comment.\n\nThe AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec.  If I drop\ntv_nsec from the comment should I also drop it in the check?\n\nI thought it was better to be very explicit because the code using the\ncheck is using that .tv_nsec field...I figured the check may as well\ndo exactly what the code is doing...\n\n\n>\n> Also: s/exist/exists./\n>\n>> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist\n>\n> Perhaps wrap this long comment over two lines.\n>\n> Also: s/exist/exist./\n>\n>> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])\n>> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n>> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then\n>> +       USE_ST_TIMESPEC=YesPlease\n>> +       GIT_CONF_SUBST([USE_ST_TIMESPEC])\n>> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then\n>> +       NO_NSEC=YesPlease\n>> +       GIT_CONF_SUBST([NO_NSEC])\n>> +fi\n>> +#\n>>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n>>  AC_CHECK_MEMBER(struct dirent.d_ino,\n>>  [NO_D_INO_IN_DIRENT=],\n>> --\n>> 2.2.0.68.g8f72f0c.dirty\n"},{"id":"254429","messageId":"CAPig+cSx9FZtn=RH25OX97EBmbnKfVT66WsgtqenVZpm8LBjHQ@mail.gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-3-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-07T21:37:22Z","receivedAt":"2015-01-07T21:37:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\n> systems being used in production.  This change makes compiling git\n> less tedious on older platforms without CLOCK_MONOTONIC.\n\nThe second sentence is implied by the very presence of this patch,\nthus adds no value. I, personally, would drop it.\n\nAlso, your sign-off is missing (as mentioned in my previous review[1]).\n\n> ---\n> diff --git a/Makefile b/Makefile\n> index 7482a4d..af551a0 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME\n>         EXTLIBS += -lrt\n>  endif\n>\n> +ifdef HAVE_CLOCK_MONOTONIC\n> +       BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n> +endif\n\nYou need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)\nat the top of Makefile (as mentioned in my previous review[1]), so\nthat people who build without running 'configure' will know that they\nmay need to tweak it.\n\n>  ifeq ($(TCLTK_PATH),)\n>  NO_TCLTK = NoThanks\n>  endif\n> diff --git a/configure.ac b/configure.ac\n> index dcc4bf0..424dec5 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],\n>                       [CHARSET_LIB=-lcharset])])\n>  GIT_CONF_SUBST([CHARSET_LIB])\n>  #\n> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.\n> +GIT_CHECK_FUNC(clock_gettime,\n> +[HAVE_CLOCK_GETTIME=YesPlease],\n> +[HAVE_CLOCK_GETTIME=])\n> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n\nYou could simplify the above four lines to this one-liner:\n\n    GIT_CHECK_FUNC(clock_gettime,\n        GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))\n\n> +\n> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [\n> +AC_LANG_PROGRAM([[\n> +#include <time.h>\n> +clockid_t id = CLOCK_MONOTONIC;\n> +]], [])])\n\nNo need to pass empty trailing arguments in m4. It's customary to drop\nthem altogether (since they are implied).\n\n> +#\n> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.\n> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n> +       [AC_MSG_RESULT([yes])\n> +       HAVE_CLOCK_MONOTONIC=YesPlease],\n> +       [AC_MSG_RESULT([no])\n> +       HAVE_CLOCK_MONOTONIC=])\n> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n\nDitto regarding simplification:\n\n    AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n    AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n        [AC_MSG_RESULT([yes])\n        GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],\n        [AC_MSG_RESULT([no])])\n\n> +#\n>  # Define NO_SETITIMER if you don't have setitimer.\n>  GIT_CHECK_FUNC(setitimer,\n>  [NO_SETITIMER=],\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/261630\n"},{"id":"254430","messageId":"CAPig+cQ5dibu7ETWLfO5zbeeuVu2rcTDP3ghN9pt1cLrw141uw@mail.gmail.com","threadId":"38218","inReplyTo":"1420662236-27593-4-git-send-email-reubenhwk@gmail.com","subject":"Re: [PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-07T21:46:07Z","receivedAt":"2015-01-07T21:46:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.\n> Newer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and\n> fall back to HMAC_cleanup when the newer function is missing.\n\nMissing sign-off.\n\nOverall, these patches are nicely improved from the previous round. A\nfew more comments below...\n\n> ---\n>  Makefile          | 3 +++\n>  configure.ac      | 7 +++++++\n>  git-compat-util.h | 3 +++\n>  3 files changed, 13 insertions(+)\n>\n> diff --git a/Makefile b/Makefile\n> index af551a0..d3c2b58 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1059,6 +1059,9 @@ ifndef NO_OPENSSL\n>         ifdef NEEDS_CRYPTO_WITH_SSL\n>                 OPENSSL_LIBSSL += -lcrypto\n>         endif\n> +       ifdef NO_HMAC_CTX_CLEANUP\n> +               BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP\n> +       endif\n\nYou need to document this new Makefile variable (NO_HMAC_CTX_CLEANUP)\nat the top of Makefile (as mentioned in my previous review[1]).\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/261631\n\n>  else\n>         BASIC_CFLAGS += -DNO_OPENSSL\n>         BLK_SHA1 = 1\n> diff --git a/configure.ac b/configure.ac\n> index 424dec5..c282663 100644\n> --- a/configure.ac\n> +++ b/configure.ac\n> @@ -923,6 +923,13 @@ AC_CHECK_LIB([iconv], [locale_charset],\n>                       [CHARSET_LIB=-lcharset])])\n>  GIT_CONF_SUBST([CHARSET_LIB])\n>  #\n> +# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.\n> +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],\n> +       [NO_HMAC_CTX_CLEANUP=],\n> +       [NO_HMAC_CTX_CLEANUP=YesPlease],\n> +       [])\n> +GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP])\n\nIt is customary to drop empty trailing arguments in m4.\n\nAlso, you can simplify this entire check to:\n\n    AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],\n        [], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])])\n\n>  # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.\n>  GIT_CHECK_FUNC(clock_gettime,\n>  [HAVE_CLOCK_GETTIME=YesPlease],\n"},{"id":"254431","messageId":"CAPig+cRLMZMoW7Oe8ViB=3R9iSxp88haR7CZ1SHaW0sCnhJ=0Q@mail.gmail.com","threadId":"38218","inReplyTo":"CAD_8n+RBJHDjFCEY8GmbsqyyyxPEo71Lg03M7y9dMcu74y6m-w@mail.gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-07T21:57:22Z","receivedAt":"2015-01-07T21:57:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 4:33 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>>> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist\n>>\n>> It would be slightly more accurate to drop the \".tv_nsec\" bit from this comment.\n>\n> The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec.  If I drop\n> tv_nsec from the comment should I also drop it in the check?\n\nNo. My observation was just about the comment.\n\n> I thought it was better to be very explicit because the code using the\n> check is using that .tv_nsec field...I figured the check may as well\n> do exactly what the code is doing...\n\nIndeed, the check and code should agree. However, from the perspective\nof the person reading comment, the \".tv_nsec\" is just an\nimplementation detail of the check itself. The final outcome (the\nsetting of USE_ST_TIMESPEC) is independent of how that check was made:\nit matters only that 'stat.st_mtimespec' was detected _somehow_.\n\nAnyhow, it's just a minor observation, hence my qualification of it as\n\"_slightly_ more accurate\". If you feel strongly that it should remain\nas is, then that's fine.\n"},{"id":"254432","messageId":"CAD_8n+RkCWzz0VSPb+3XkR-qbms1csGBbFRJtc-HwMp823QdCQ@mail.gmail.com","threadId":"38218","inReplyTo":"CAPig+cSrht0fovEWhEknxPPzwZhA7vzeLcM+omaM7PR752GfGw@mail.gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T22:19:39Z","receivedAt":"2015-01-07T22:19:39Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>> This check will automatically set the correct NO_NSEC setting.\n>\n> This commit message neglects to mention the important point that\n> you're also now setting USE_ST_TIMESPEC when detected. You might\n> revise the message like this:\n>\n>     Detect 'tv_nsec' field in 'struct stat' and set Makefile variable\n>     NO_NSEC appropriately.\n>\n>     A side-effect of the above detection is that we also determine\n>     whether 'stat.st_mtimespec' is available, so, as a bonus, set the\n>     Makefile variable USE_ST_TIMESPEC, as well.\n\nI see you're single quoted 'tv_nsec' and 'struct stat'.  Should I also\nuse single quotes in the first line of the commit msg like this...\n\nconfigure.ac: check 'tv_nsec' field in 'struct stat'\n\n?\n\n>\n> Also, your sign-off is missing (as mentioned in my previous review[1]).\n>\n> [1]: http://article.gmane.org/gmane.comp.version-control.git/261626\n>\n>> ---\n>>  configure.ac | 12 ++++++++++++\n>>  1 file changed, 12 insertions(+)\n>>\n>> diff --git a/configure.ac b/configure.ac\n>> index 6af9647..dcc4bf0 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],\n>>  [#include <sys/time.h>])\n>>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])\n>>  #\n>> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist\n>\n> It would be slightly more accurate to drop the \".tv_nsec\" bit from this comment.\n>\n> Also: s/exist/exists./\n>\n>> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist\n>\n> Perhaps wrap this long comment over two lines.\n>\n> Also: s/exist/exist./\n>\n>> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])\n>> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])\n>> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then\n>> +       USE_ST_TIMESPEC=YesPlease\n>> +       GIT_CONF_SUBST([USE_ST_TIMESPEC])\n>> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then\n>> +       NO_NSEC=YesPlease\n>> +       GIT_CONF_SUBST([NO_NSEC])\n>> +fi\n>> +#\n>>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.\n>>  AC_CHECK_MEMBER(struct dirent.d_ino,\n>>  [NO_D_INO_IN_DIRENT=],\n>> --\n>> 2.2.0.68.g8f72f0c.dirty\n"},{"id":"254433","messageId":"CAD_8n+TzLhQh0MaE6NJado-mb=ceywWGSA=fhUViR6D3woVXbg@mail.gmail.com","threadId":"38218","inReplyTo":"CAPig+cSx9FZtn=RH25OX97EBmbnKfVT66WsgtqenVZpm8LBjHQ@mail.gmail.com","subject":"Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC","fromName":"Reuben Hawkins","fromEmail":"reubenhwk@gmail.com","sentAt":"2015-01-07T22:31:38Z","receivedAt":"2015-01-07T22:31:38Z","isPatch":true,"sender":{"key":"reubenhwk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/201595?v=4"},"body":"On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>> CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3\n>> systems being used in production.  This change makes compiling git\n>> less tedious on older platforms without CLOCK_MONOTONIC.\n>\n> The second sentence is implied by the very presence of this patch,\n> thus adds no value. I, personally, would drop it.\n>\n> Also, your sign-off is missing (as mentioned in my previous review[1]).\n>\n>> ---\n>> diff --git a/Makefile b/Makefile\n>> index 7482a4d..af551a0 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME\n>>         EXTLIBS += -lrt\n>>  endif\n>>\n>> +ifdef HAVE_CLOCK_MONOTONIC\n>> +       BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n>> +endif\n>\n> You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)\n> at the top of Makefile (as mentioned in my previous review[1]), so\n> that people who build without running 'configure' will know that they\n> may need to tweak it.\n>\n>>  ifeq ($(TCLTK_PATH),)\n>>  NO_TCLTK = NoThanks\n>>  endif\n>> diff --git a/configure.ac b/configure.ac\n>> index dcc4bf0..424dec5 100644\n>> --- a/configure.ac\n>> +++ b/configure.ac\n>> @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],\n>>                       [CHARSET_LIB=-lcharset])])\n>>  GIT_CONF_SUBST([CHARSET_LIB])\n>>  #\n>> +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.\n>> +GIT_CHECK_FUNC(clock_gettime,\n>> +[HAVE_CLOCK_GETTIME=YesPlease],\n>> +[HAVE_CLOCK_GETTIME=])\n>> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n>\n> You could simplify the above four lines to this one-liner:\n>\n>     GIT_CHECK_FUNC(clock_gettime,\n>         GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))\n>\n>> +\n>> +AC_DEFUN([CLOCK_MONOTONIC_SRC], [\n>> +AC_LANG_PROGRAM([[\n>> +#include <time.h>\n>> +clockid_t id = CLOCK_MONOTONIC;\n>> +]], [])])\n>\n> No need to pass empty trailing arguments in m4. It's customary to drop\n> them altogether (since they are implied).\n>\n>> +#\n>> +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.\n>> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n>> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n>> +       [AC_MSG_RESULT([yes])\n>> +       HAVE_CLOCK_MONOTONIC=YesPlease],\n>> +       [AC_MSG_RESULT([no])\n>> +       HAVE_CLOCK_MONOTONIC=])\n>> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n>\n> Ditto regarding simplification:\n>\n>     AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n>     AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n>         [AC_MSG_RESULT([yes])\n>         GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],\n>         [AC_MSG_RESULT([no])])\n\nI *think* there's an issue with this simplification as used right\nhere.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by\nsetting it equal to nothing\n\nHAVE_CLOCK_MONOTONIC=\n\nSo that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =\nYesPlease' will be overridden.\n\nSo this one needs to stay as is.\n\n>\n>> +#\n>>  # Define NO_SETITIMER if you don't have setitimer.\n>>  GIT_CHECK_FUNC(setitimer,\n>>  [NO_SETITIMER=],\n>\n> [1]: http://article.gmane.org/gmane.comp.version-control.git/261630\n"},{"id":"254450","messageId":"CAPig+cSwBZuYAp97nLBLcdCkTrcbCfwZ+xKe0JAEt_ekk3EFRA@mail.gmail.com","threadId":"38218","inReplyTo":"CAD_8n+RkCWzz0VSPb+3XkR-qbms1csGBbFRJtc-HwMp823QdCQ@mail.gmail.com","subject":"Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-08T05:41:57Z","receivedAt":"2015-01-08T05:41:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 5:19 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>>> This check will automatically set the correct NO_NSEC setting.\n>>\n>> This commit message neglects to mention the important point that\n>> you're also now setting USE_ST_TIMESPEC when detected. You might\n>> revise the message like this:\n>>\n>>     Detect 'tv_nsec' field in 'struct stat' and set Makefile variable\n>>     NO_NSEC appropriately.\n>>\n>>     A side-effect of the above detection is that we also determine\n>>     whether 'stat.st_mtimespec' is available, so, as a bonus, set the\n>>     Makefile variable USE_ST_TIMESPEC, as well.\n>\n> I see you're single quoted 'tv_nsec' and 'struct stat'.  Should I also\n> use single quotes in the first line of the commit msg like this...\n>\n> configure.ac: check 'tv_nsec' field in 'struct stat'\n\nQuoting them was just my personal taste, however, consistency of\nformatting between subject and the body of the message would be nice.\nUse whatever seems correct to you.\n"},{"id":"254451","messageId":"CAPig+cSHTuNv8nFiZ4yKrypGW4n4EMtucX-SfZE927d=u40P=Q@mail.gmail.com","threadId":"38218","inReplyTo":"CAD_8n+TzLhQh0MaE6NJado-mb=ceywWGSA=fhUViR6D3woVXbg@mail.gmail.com","subject":"Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-01-08T05:54:38Z","receivedAt":"2015-01-08T05:54:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jan 7, 2015 at 5:31 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n> On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@gmail.com> wrote:\n>>> +GIT_CHECK_FUNC(clock_gettime,\n>>> +[HAVE_CLOCK_GETTIME=YesPlease],\n>>> +[HAVE_CLOCK_GETTIME=])\n>>> +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])\n>>\n>> You could simplify the above four lines to this one-liner:\n>>\n>>     GIT_CHECK_FUNC(clock_gettime,\n>>         GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))\n>>\n>>> +AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n>>> +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n>>> +       [AC_MSG_RESULT([yes])\n>>> +       HAVE_CLOCK_MONOTONIC=YesPlease],\n>>> +       [AC_MSG_RESULT([no])\n>>> +       HAVE_CLOCK_MONOTONIC=])\n>>> +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])\n>>\n>> Ditto regarding simplification:\n>>\n>>     AC_MSG_CHECKING([for CLOCK_MONOTONIC])\n>>     AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],\n>>         [AC_MSG_RESULT([yes])\n>>         GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],\n>>         [AC_MSG_RESULT([no])])\n>\n> I *think* there's an issue with this simplification as used right\n> here.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by\n> setting it equal to nothing\n>\n> HAVE_CLOCK_MONOTONIC=\n>\n> So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =\n> YesPlease' will be overridden.\n>\n> So this one needs to stay as is.\n\nYes, you're right. That means that the HAVE_CLOCK_GETTIME\nsimplification also suffers the same shortcoming. So, neither\nsimplification is appropriate in this instance. Sorry for the noise.\n"}]}