On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@xxxxxxxxx> wrote: >> This check will automatically set the correct NO_NSEC setting. > > This commit message neglects to mention the important point that > you're also now setting USE_ST_TIMESPEC when detected. You might > revise the message like this: > > Detect 'tv_nsec' field in 'struct stat' and set Makefile variable > NO_NSEC appropriately. > > A side-effect of the above detection is that we also determine > whether 'stat.st_mtimespec' is available, so, as a bonus, set the > Makefile variable USE_ST_TIMESPEC, as well. > > Also, your sign-off is missing (as mentioned in my previous review[1]). > > [1]: http://article.gmane.org/gmane.comp.version-control.git/261626 > >> --- >> configure.ac | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/configure.ac b/configure.ac >> index 6af9647..dcc4bf0 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval], >> [#include <sys/time.h>]) >> GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL]) >> # >> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist > > It would be slightly more accurate to drop the ".tv_nsec" bit from this comment. The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec. If I drop tv_nsec from the comment should I also drop it in the check? I thought it was better to be very explicit because the code using the check is using that .tv_nsec field...I figured the check may as well do exactly what the code is doing... > > Also: s/exist/exists./ > >> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist > > Perhaps wrap this long comment over two lines. > > Also: s/exist/exist./ > >> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec]) >> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec]) >> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then >> + USE_ST_TIMESPEC=YesPlease >> + GIT_CONF_SUBST([USE_ST_TIMESPEC]) >> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then >> + NO_NSEC=YesPlease >> + GIT_CONF_SUBST([NO_NSEC]) >> +fi >> +# >> # 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.2.0.68.g8f72f0c.dirty -- 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