[PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin

[PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin

[Brian Gernhardt]

Not all OSes use st_ctim and st_mtim in their struct stat.  In
particular, it appears that OS X uses st_*timespec instead.  So add a
Makefile variable and #define called USE_ST_TIMESPEC to switch the
USE_NSEC defines to use st_*timespec.

This also turns it on by default for OS X (Darwin) machines.  Likely
this is a sane default for other BSD kernels as well, but I don't have
any to test that assumption on.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---

 This is on top of "next".

 Now time to go debug a Bus Error in git-grep that made this hard to find.

 Makefile          |    7 +++++++
 git-compat-util.h |    5 +++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 9a23aa5..4bdaad7 100644
--- a/Makefile
+++ b/Makefile
@@ -126,6 +126,9 @@ all::
 # randomly break unless your underlying filesystem supports those sub-second
 # times (my ext3 doesn't).
 #
+# Define USE_ST_TIMESPEC if your "struct stat" uses "st_ctimespec" instead of
+# "st_ctim"
+#
 # Define NO_NSEC if your "struct stat" does not have "st_ctim.tv_nsec"
 # available.  This automatically turns USE_NSEC off.
 #
@@ -660,6 +663,7 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_MEMMEM = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
+	USE_ST_TIMESPEC = YesPlease
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -925,6 +929,9 @@ endif
 ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
 	BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT
 endif
+ifdef USE_ST_TIMESPEC
+	BASIC_CFLAGS += -DUSE_ST_TIMESPEC
+endif
 ifdef NO_NSEC
 	BASIC_CFLAGS += -DNO_NSEC
 endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 83d8389..1906253 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,
 #define ST_CTIME_NSEC(st) 0
 #define ST_MTIME_NSEC(st) 0
 #else
+#ifdef USE_ST_TIMESPEC
+#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
+#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
+#else
 #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
 #define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
 #endif
+#endif
 
 #endif
-- 
1.6.2.221.g2411c.dirty

Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin

[Junio C Hamano]

Brian Gernhardt <benji@silverinsanity.com> writes:

> This also turns it on by default for OS X (Darwin) machines.  Likely
> this is a sane default for other BSD kernels as well, but I don't have
> any to test that assumption on.

Yeah, that was my initial reaction.  Any BSDers?

> diff --git a/git-compat-util.h b/git-compat-util.h
> index 83d8389..1906253 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,
>  #define ST_CTIME_NSEC(st) 0
>  #define ST_MTIME_NSEC(st) 0
>  #else
> +#ifdef USE_ST_TIMESPEC
> +#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
> +#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
> +#else
>  #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
>  #define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
>  #endif
> +#endif

Thanks.

I think this patch moves things in the right direction, but there are
other uses of "st_[cm]tim.tv_nsec" that do not use the ST_[CM]TIME_NSEC
macro.

$ git grep -n -e 'st_[cm]tim\.' --cached -- '*.[ch]'
builtin-fetch-pack.c:810:				|| st.st_mtim.tv_nsec != mtime.nsec
git-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
git-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
read-cache.c:207:	if (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)
read-cache.c:209:	if (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)

Probably we should apply the following patch as a fix, and then apply your
enhancement to support st_[cm]timespec systems?

 builtin-fetch-pack.c |    2 +-
 read-cache.c         |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 59b0b0a..1d7e023 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -807,7 +807,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
 				die("shallow file was removed during fetch");
 		} else if (st.st_mtime != mtime.sec
 #ifdef USE_NSEC
-				|| st.st_mtim.tv_nsec != mtime.nsec
+				|| ST_CTIME_NSEC(st) != mtime.nsec
 #endif
 			  )
 			die("shallow file was changed during fetch");
diff --git a/read-cache.c b/read-cache.c
index b819abb..7f74c8d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,9 +204,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 		changed |= CTIME_CHANGED;
 
 #ifdef USE_NSEC
-	if (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)
+	if (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))
 		changed |= MTIME_CHANGED;
-	if (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)
+	if (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
 		changed |= CTIME_CHANGED;
 #endif
 

Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin

[Brian Gernhardt]

On Mar 8, 2009, at 4:51 PM, Junio C Hamano wrote:

> I think this patch moves things in the right direction, but there are
> other uses of "st_[cm]tim.tv_nsec" that do not use the  
> ST_[CM]TIME_NSEC
> macro.
>
> $ git grep -n -e 'st_[cm]tim\.' --cached -- '*.[ch]'
> builtin-fetch-pack.c:810:				|| st.st_mtim.tv_nsec != mtime.nsec
> git-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int) 
> ((st).st_ctim.tv_nsec))
> git-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int) 
> ((st).st_mtim.tv_nsec))
> read-cache.c:207:	if (ce->ce_mtime.nsec != (unsigned int)st- 
> >st_mtim.tv_nsec)
> read-cache.c:209:	if (trust_ctime && ce->ce_ctime.nsec != (unsigned  
> int)st->st_ctim.tv_nsec)

Interesting.  I couldn't use git-grep due to other problems, but  
thought any other tim/timespec issues would have stopped my  
compilation.  Looking at the code, this is because everything other  
than the #defines in git-compat-util.h are surrounded by USE_NSEC  
which is not defined on my machine.

However, I noticed another breakage entirely.  Namely, that USE_NSEC  
is not defined anywhere.  There's a comment in the Makefile that says  
"define USE_NSEC below", but there is no code that checks for USE_NSEC  
and sets the appropriate compiler switch.  Trivial patch to follow.

> Probably we should apply the following patch as a fix, and then  
> apply your
> enhancement to support st_[cm]timespec systems?

Your patch looks sane to me.

~~ Brian

[PATCH] Makefile: Set compiler switch for USE_NSEC

[Brian Gernhardt]

The comments indicated that setting a Makefile variable USE_NSEC would
enable the code for sub-second [cm]times.  However, the Makefile
variable was never turned into a compiler switch so the code was never
enabled.  This patch allows USE_NSEC to be noticed by the compiler.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 4bdaad7..b96c2b3 100644
--- a/Makefile
+++ b/Makefile
@@ -929,6 +929,9 @@ endif
 ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
 	BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT
 endif
+ifdef USE_NSEC
+	BASIC_CFLAGS += -DUSE_NSEC
+endif
 ifdef USE_ST_TIMESPEC
 	BASIC_CFLAGS += -DUSE_ST_TIMESPEC
 endif
-- 
1.6.2.222.g01cbd