{"thread":{"id":"18215","subject":"[PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin","startedAt":"2009-03-08T20:04:28Z","lastAt":"2009-03-08T21:22:51Z","messageCount":4,"participants":["Brian Gernhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107396","messageId":"1236542668-83333-1-git-send-email-benji@silverinsanity.com","threadId":"18215","inReplyTo":null,"subject":"[PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2009-03-08T20:04:28Z","receivedAt":"2009-03-08T20:04:28Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"Not all OSes use st_ctim and st_mtim in their struct stat.  In\nparticular, it appears that OS X uses st_*timespec instead.  So add a\nMakefile variable and #define called USE_ST_TIMESPEC to switch the\nUSE_NSEC defines to use st_*timespec.\n\nThis also turns it on by default for OS X (Darwin) machines.  Likely\nthis is a sane default for other BSD kernels as well, but I don't have\nany to test that assumption on.\n\nSigned-off-by: Brian Gernhardt <benji@silverinsanity.com>\n---\n\n This is on top of \"next\".\n\n Now time to go debug a Bus Error in git-grep that made this hard to find.\n\n Makefile          |    7 +++++++\n git-compat-util.h |    5 +++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 9a23aa5..4bdaad7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -126,6 +126,9 @@ all::\n # randomly break unless your underlying filesystem supports those sub-second\n # times (my ext3 doesn't).\n #\n+# Define USE_ST_TIMESPEC if your \"struct stat\" uses \"st_ctimespec\" instead of\n+# \"st_ctim\"\n+#\n # Define NO_NSEC if your \"struct stat\" does not have \"st_ctim.tv_nsec\"\n # available.  This automatically turns USE_NSEC off.\n #\n@@ -660,6 +663,7 @@ ifeq ($(uname_S),Darwin)\n \tendif\n \tNO_MEMMEM = YesPlease\n \tTHREADED_DELTA_SEARCH = YesPlease\n+\tUSE_ST_TIMESPEC = YesPlease\n endif\n ifeq ($(uname_S),SunOS)\n \tNEEDS_SOCKET = YesPlease\n@@ -925,6 +929,9 @@ endif\n ifdef NO_ST_BLOCKS_IN_STRUCT_STAT\n \tBASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT\n endif\n+ifdef USE_ST_TIMESPEC\n+\tBASIC_CFLAGS += -DUSE_ST_TIMESPEC\n+endif\n ifdef NO_NSEC\n \tBASIC_CFLAGS += -DNO_NSEC\n endif\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 83d8389..1906253 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n #define ST_CTIME_NSEC(st) 0\n #define ST_MTIME_NSEC(st) 0\n #else\n+#ifdef USE_ST_TIMESPEC\n+#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n+#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))\n+#else\n #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\n #define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\n #endif\n+#endif\n \n #endif\n-- \n1.6.2.221.g2411c.dirty\n"},{"id":"107402","messageId":"7vhc23kaay.fsf@gitster.siamese.dyndns.org","threadId":"18215","inReplyTo":"1236542668-83333-1-git-send-email-benji@silverinsanity.com","subject":"Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-08T20:51:33Z","receivedAt":"2009-03-08T20:51:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt <benji@silverinsanity.com> writes:\n\n> This also turns it on by default for OS X (Darwin) machines.  Likely\n> this is a sane default for other BSD kernels as well, but I don't have\n> any to test that assumption on.\n\nYeah, that was my initial reaction.  Any BSDers?\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 83d8389..1906253 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n>  #define ST_CTIME_NSEC(st) 0\n>  #define ST_MTIME_NSEC(st) 0\n>  #else\n> +#ifdef USE_ST_TIMESPEC\n> +#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n> +#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))\n> +#else\n>  #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\n>  #define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\n>  #endif\n> +#endif\n\nThanks.\n\nI think this patch moves things in the right direction, but there are\nother uses of \"st_[cm]tim.tv_nsec\" that do not use the ST_[CM]TIME_NSEC\nmacro.\n\n$ git grep -n -e 'st_[cm]tim\\.' --cached -- '*.[ch]'\nbuiltin-fetch-pack.c:810:\t\t\t\t|| st.st_mtim.tv_nsec != mtime.nsec\ngit-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\ngit-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\nread-cache.c:207:\tif (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)\nread-cache.c:209:\tif (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)\n\nProbably we should apply the following patch as a fix, and then apply your\nenhancement to support st_[cm]timespec systems?\n\n builtin-fetch-pack.c |    2 +-\n read-cache.c         |    4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex 59b0b0a..1d7e023 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -807,7 +807,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,\n \t\t\t\tdie(\"shallow file was removed during fetch\");\n \t\t} else if (st.st_mtime != mtime.sec\n #ifdef USE_NSEC\n-\t\t\t\t|| st.st_mtim.tv_nsec != mtime.nsec\n+\t\t\t\t|| ST_CTIME_NSEC(st) != mtime.nsec\n #endif\n \t\t\t  )\n \t\t\tdie(\"shallow file was changed during fetch\");\ndiff --git a/read-cache.c b/read-cache.c\nindex b819abb..7f74c8d 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -204,9 +204,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \t\tchanged |= CTIME_CHANGED;\n \n #ifdef USE_NSEC\n-\tif (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)\n+\tif (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))\n \t\tchanged |= MTIME_CHANGED;\n-\tif (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)\n+\tif (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))\n \t\tchanged |= CTIME_CHANGED;\n #endif\n \n"},{"id":"107406","messageId":"70A401B0-C10D-4B4D-9DCC-D0968CE5EAF7@silverinsanity.com","threadId":"18215","inReplyTo":"7vhc23kaay.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2009-03-08T21:21:17Z","receivedAt":"2009-03-08T21:21:17Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Mar 8, 2009, at 4:51 PM, Junio C Hamano wrote:\n\n> I think this patch moves things in the right direction, but there are\n> other uses of \"st_[cm]tim.tv_nsec\" that do not use the  \n> ST_[CM]TIME_NSEC\n> macro.\n>\n> $ git grep -n -e 'st_[cm]tim\\.' --cached -- '*.[ch]'\n> builtin-fetch-pack.c:810:\t\t\t\t|| st.st_mtim.tv_nsec != mtime.nsec\n> git-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int) \n> ((st).st_ctim.tv_nsec))\n> git-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int) \n> ((st).st_mtim.tv_nsec))\n> read-cache.c:207:\tif (ce->ce_mtime.nsec != (unsigned int)st- \n> >st_mtim.tv_nsec)\n> read-cache.c:209:\tif (trust_ctime && ce->ce_ctime.nsec != (unsigned  \n> int)st->st_ctim.tv_nsec)\n\nInteresting.  I couldn't use git-grep due to other problems, but  \nthought any other tim/timespec issues would have stopped my  \ncompilation.  Looking at the code, this is because everything other  \nthan the #defines in git-compat-util.h are surrounded by USE_NSEC  \nwhich is not defined on my machine.\n\nHowever, I noticed another breakage entirely.  Namely, that USE_NSEC  \nis not defined anywhere.  There's a comment in the Makefile that says  \n\"define USE_NSEC below\", but there is no code that checks for USE_NSEC  \nand sets the appropriate compiler switch.  Trivial patch to follow.\n\n> Probably we should apply the following patch as a fix, and then  \n> apply your\n> enhancement to support st_[cm]timespec systems?\n\nYour patch looks sane to me.\n\n~~ Brian\n"},{"id":"107407","messageId":"1236547371-88742-1-git-send-email-benji@silverinsanity.com","threadId":"18215","inReplyTo":"70A401B0-C10D-4B4D-9DCC-D0968CE5EAF7@silverinsanity.com","subject":"[PATCH] Makefile: Set compiler switch for USE_NSEC","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2009-03-08T21:22:51Z","receivedAt":"2009-03-08T21:22:51Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"The comments indicated that setting a Makefile variable USE_NSEC would\nenable the code for sub-second [cm]times.  However, the Makefile\nvariable was never turned into a compiler switch so the code was never\nenabled.  This patch allows USE_NSEC to be noticed by the compiler.\n\nSigned-off-by: Brian Gernhardt <benji@silverinsanity.com>\n---\n Makefile |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4bdaad7..b96c2b3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -929,6 +929,9 @@ endif\n ifdef NO_ST_BLOCKS_IN_STRUCT_STAT\n \tBASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT\n endif\n+ifdef USE_NSEC\n+\tBASIC_CFLAGS += -DUSE_NSEC\n+endif\n ifdef USE_ST_TIMESPEC\n \tBASIC_CFLAGS += -DUSE_ST_TIMESPEC\n endif\n-- \n1.6.2.222.g01cbd\n"}]}