[PATCH 0/3] Win32: nanosecond-precision file times

[PATCH 0/3] Win32: nanosecond-precision file times

[Karsten Blees]

This patch series was inspired by the problem that Git does not
detect changed file content if st_size, st_mtime and st_ctime
are unchanged. This was apparently caused by VSS2Git resetting
mtime to a value in the past. [1]

I believe (or rather hope) that all involved in the discussion
agree that Git cannot reasonably be expected to detect changed
file content if file time(s) are reset on purpose.

However, some users have expressed concerns that 'same size and
mtime' [2] may theoretically happen by chance in daily operation.

This patch series adopts POSIX 2013 'struct timespec' file times
to make this practically impossible, at least on NTFS with 100ns
file time resolution.

Cheers,
Karsten

[1] https://github.com/msysgit/git/issues/312
[2] Note that st_ctime of a file never changes on Windows, as it
    means 'creation time' rather than 'change status time'.

Karsten Blees (3):
  Win32: make FILETIME conversion functions public
  Win32: replace MSVCRT's fstat() with a Win32-based implementation
  Win32: implement nanosecond-precision file times

 compat/mingw.c   | 56 +++++++++++++++++++++++++-------------------------------
 compat/mingw.h   | 55 +++++++++++++++++++++++++++++++++++++++++++++----------
 config.mak.uname |  4 ++--
 3 files changed, 72 insertions(+), 43 deletions(-)

-- 
2.3.0.3.ge7778af

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 1/3] Win32: make FILETIME conversion functions public

[Karsten Blees]

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 compat/mingw.c | 16 ----------------
 compat/mingw.h | 16 ++++++++++++++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 70f3191..ba3cfb0 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -419,22 +419,6 @@ int mingw_chmod(const char *filename, int mode)
 	return _wchmod(wfilename, mode);
 }
 
-/*
- * The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
- * Returns the 100-nanoseconds ("hekto nanoseconds") since the epoch.
- */
-static inline long long filetime_to_hnsec(const FILETIME *ft)
-{
-	long long winTime = ((long long)ft->dwHighDateTime << 32) + ft->dwLowDateTime;
-	/* Windows to Unix Epoch conversion */
-	return winTime - 116444736000000000LL;
-}
-
-static inline time_t filetime_to_time_t(const FILETIME *ft)
-{
-	return (time_t)(filetime_to_hnsec(ft) / 10000000);
-}
-
 /* We keep the do_lstat code in a separate function to avoid recursion.
  * When a path ends with a slash, the stat will fail with ENOENT. In
  * this case, we strip the trailing slashes and stat again.
diff --git a/compat/mingw.h b/compat/mingw.h
index 5e499cf..f2a78b4 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -283,6 +283,22 @@ static inline int getrlimit(int resource, struct rlimit *rlp)
 }
 
 /*
+ * The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
+ * Returns the 100-nanoseconds ("hekto nanoseconds") since the epoch.
+ */
+static inline long long filetime_to_hnsec(const FILETIME *ft)
+{
+	long long winTime = ((long long)ft->dwHighDateTime << 32) + ft->dwLowDateTime;
+	/* Windows to Unix Epoch conversion */
+	return winTime - 116444736000000000LL;
+}
+
+static inline time_t filetime_to_time_t(const FILETIME *ft)
+{
+	return (time_t)(filetime_to_hnsec(ft) / 10000000);
+}
+
+/*
  * Use mingw specific stat()/lstat()/fstat() implementations on Windows.
  */
 #define off_t off64_t
-- 
2.3.0.3.ge7778af


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2/3] Win32: replace MSVCRT's fstat() with a Win32-based implementation

[Karsten Blees]

fstat() is the only stat-related CRT function for which we don't have a
full replacement yet (and thus the only reason to stick with MSVCRT's
'struct stat' definition).

Fully implement fstat(), in preparation of implementing a POSIX 2013
compatible 'struct stat' with nanosecond-precision file times.

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 compat/mingw.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index ba3cfb0..6d73a3d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -532,28 +532,38 @@ int mingw_fstat(int fd, struct stat *buf)
 {
 	HANDLE fh = (HANDLE)_get_osfhandle(fd);
 	BY_HANDLE_FILE_INFORMATION fdata;
+	DWORD avail;
 
 	if (fh == INVALID_HANDLE_VALUE) {
 		errno = EBADF;
 		return -1;
 	}
-	/* direct non-file handles to MS's fstat() */
-	if (GetFileType(fh) != FILE_TYPE_DISK)
-		return _fstati64(fd, buf);
 
-	if (GetFileInformationByHandle(fh, &fdata)) {
-		buf->st_ino = 0;
-		buf->st_gid = 0;
-		buf->st_uid = 0;
-		buf->st_nlink = 1;
+	/* initialize stat fields */
+	memset(buf, 0, sizeof(*buf));
+	buf->st_nlink = 1;
+
+	switch (GetFileType(fh) & ~FILE_TYPE_REMOTE) {
+	case FILE_TYPE_DISK:
+		if (!GetFileInformationByHandle(fh, &fdata))
+			break;
 		buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
 		buf->st_size = fdata.nFileSizeLow |
 			(((off_t)fdata.nFileSizeHigh)<<32);
-		buf->st_dev = buf->st_rdev = 0; /* not used by Git */
 		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
 		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
 		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
 		return 0;
+
+	case FILE_TYPE_CHAR:
+		buf->st_mode = _S_IFCHR;
+		return 0;
+
+	case FILE_TYPE_PIPE:
+		buf->st_mode = _S_IFIFO;
+		if (PeekNamedPipe(fh, NULL, 0, NULL, &avail, NULL))
+			buf->st_size = avail;
+		return 0;
 	}
 	errno = EBADF;
 	return -1;
-- 
2.3.0.3.ge7778af


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] Win32: implement nanosecond-precision file times

[Karsten Blees]

We no longer use any of MSVCRT's stat-functions, so there's no need to
stick to a CRT-compatible 'struct stat' either.

Define and use our own POSIX-2013-compatible 'struct stat' with nanosecond-
precision file times.

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 compat/mingw.c   | 12 ++++++------
 compat/mingw.h   | 43 +++++++++++++++++++++++++++++++------------
 config.mak.uname |  4 ++--
 3 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 6d73a3d..e4d5e3f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -442,9 +442,9 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)
 		buf->st_size = fdata.nFileSizeLow |
 			(((off_t)fdata.nFileSizeHigh)<<32);
 		buf->st_dev = buf->st_rdev = 0; /* not used by Git */
-		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
-		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
-		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
+		filetime_to_timespec(&(fdata.ftLastAccessTime), &(buf->st_atim));
+		filetime_to_timespec(&(fdata.ftLastWriteTime), &(buf->st_mtim));
+		filetime_to_timespec(&(fdata.ftCreationTime), &(buf->st_ctim));
 		if (fdata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
 			WIN32_FIND_DATAW findbuf;
 			HANDLE handle = FindFirstFileW(wfilename, &findbuf);
@@ -550,9 +550,9 @@ int mingw_fstat(int fd, struct stat *buf)
 		buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
 		buf->st_size = fdata.nFileSizeLow |
 			(((off_t)fdata.nFileSizeHigh)<<32);
-		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
-		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
-		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
+		filetime_to_timespec(&(fdata.ftLastAccessTime), &(buf->st_atim));
+		filetime_to_timespec(&(fdata.ftLastWriteTime), &(buf->st_mtim));
+		filetime_to_timespec(&(fdata.ftCreationTime), &(buf->st_ctim));
 		return 0;
 
 	case FILE_TYPE_CHAR:
diff --git a/compat/mingw.h b/compat/mingw.h
index f2a78b4..8dee9c9 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -293,22 +293,48 @@ static inline long long filetime_to_hnsec(const FILETIME *ft)
 	return winTime - 116444736000000000LL;
 }
 
-static inline time_t filetime_to_time_t(const FILETIME *ft)
+struct timespec {
+	time_t tv_sec;
+	long tv_nsec;
+};
+
+static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
 {
-	return (time_t)(filetime_to_hnsec(ft) / 10000000);
+	long long hnsec = filetime_to_hnsec(ft);
+	ts->tv_sec = (time_t)(hnsec / 10000000);
+	ts->tv_nsec = (hnsec % 10000000) * 100;
 }
 
 /*
- * Use mingw specific stat()/lstat()/fstat() implementations on Windows.
+ * Use mingw specific stat()/lstat()/fstat() implementations on Windows,
+ * including our own struct stat with 64 bit st_size and nanosecond-precision
+ * file times.
  */
 #define off_t off64_t
 #define lseek _lseeki64
 
-/* use struct stat with 64 bit st_size */
+struct mingw_stat {
+    _dev_t st_dev;
+    _ino_t st_ino;
+    _mode_t st_mode;
+    short st_nlink;
+    short st_uid;
+    short st_gid;
+    _dev_t st_rdev;
+    off64_t st_size;
+    struct timespec st_atim;
+    struct timespec st_mtim;
+    struct timespec st_ctim;
+};
+
+#define st_atime st_atim.tv_sec
+#define st_mtime st_mtim.tv_sec
+#define st_ctime st_ctim.tv_sec
+
 #ifdef stat
 #undef stat
 #endif
-#define stat _stati64
+#define stat mingw_stat
 int mingw_lstat(const char *file_name, struct stat *buf);
 int mingw_stat(const char *file_name, struct stat *buf);
 int mingw_fstat(int fd, struct stat *buf);
@@ -321,13 +347,6 @@ int mingw_fstat(int fd, struct stat *buf);
 #endif
 #define lstat mingw_lstat
 
-#ifndef _stati64
-# define _stati64(x,y) mingw_stat(x,y)
-#elif defined (_USE_32BIT_TIME_T)
-# define _stat32i64(x,y) mingw_stat(x,y)
-#else
-# define _stat64(x,y) mingw_stat(x,y)
-#endif
 
 int mingw_utime(const char *file_name, const struct utimbuf *times);
 #define utime mingw_utime
diff --git a/config.mak.uname b/config.mak.uname
index b64b63c..a18a4cc 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -346,7 +346,7 @@ ifeq ($(uname_S),Windows)
 	NO_SVN_TESTS = YesPlease
 	RUNTIME_PREFIX = YesPlease
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-	NO_NSEC = YesPlease
+	USE_NSEC = YesPlease
 	USE_WIN32_MMAP = YesPlease
 	# USE_NED_ALLOCATOR = YesPlease
 	UNRELIABLE_FSTAT = UnfortunatelyYes
@@ -498,7 +498,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_PERL_MAKEMAKER = YesPlease
 	RUNTIME_PREFIX = YesPlease
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-	NO_NSEC = YesPlease
+	USE_NSEC = YesPlease
 	USE_WIN32_MMAP = YesPlease
 	USE_NED_ALLOCATOR = YesPlease
 	UNRELIABLE_FSTAT = UnfortunatelyYes
-- 
2.3.0.3.ge7778af


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 3/3] Win32: implement nanosecond-precision file times

[Thomas Braun]

Am 12.02.2015 um 00:53 schrieb Karsten Blees:
> We no longer use any of MSVCRT's stat-functions, so there's no need to
> stick to a CRT-compatible 'struct stat' either.
> 
> Define and use our own POSIX-2013-compatible 'struct stat' with nanosecond-
> precision file times.
> 
> Signed-off-by: Karsten Blees <blees@dcon.de>
> ---
>  compat/mingw.c   | 12 ++++++------
>  compat/mingw.h   | 43 +++++++++++++++++++++++++++++++------------
>  config.mak.uname |  4 ++--
>  3 files changed, 39 insertions(+), 20 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 6d73a3d..e4d5e3f 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -442,9 +442,9 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)
>  		buf->st_size = fdata.nFileSizeLow |
>  			(((off_t)fdata.nFileSizeHigh)<<32);
>  		buf->st_dev = buf->st_rdev = 0; /* not used by Git */
> -		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
> -		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
> -		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
> +		filetime_to_timespec(&(fdata.ftLastAccessTime), &(buf->st_atim));
> +		filetime_to_timespec(&(fdata.ftLastWriteTime), &(buf->st_mtim));
> +		filetime_to_timespec(&(fdata.ftCreationTime), &(buf->st_ctim));
>  		if (fdata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
>  			WIN32_FIND_DATAW findbuf;
>  			HANDLE handle = FindFirstFileW(wfilename, &findbuf);
> @@ -550,9 +550,9 @@ int mingw_fstat(int fd, struct stat *buf)
>  		buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
>  		buf->st_size = fdata.nFileSizeLow |
>  			(((off_t)fdata.nFileSizeHigh)<<32);
> -		buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
> -		buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
> -		buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
> +		filetime_to_timespec(&(fdata.ftLastAccessTime), &(buf->st_atim));
> +		filetime_to_timespec(&(fdata.ftLastWriteTime), &(buf->st_mtim));
> +		filetime_to_timespec(&(fdata.ftCreationTime), &(buf->st_ctim));
>  		return 0;
>  
>  	case FILE_TYPE_CHAR:
> diff --git a/compat/mingw.h b/compat/mingw.h
> index f2a78b4..8dee9c9 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -293,22 +293,48 @@ static inline long long filetime_to_hnsec(const FILETIME *ft)
>  	return winTime - 116444736000000000LL;
>  }
>  
> -static inline time_t filetime_to_time_t(const FILETIME *ft)
> +struct timespec {
> +	time_t tv_sec;
> +	long tv_nsec;
> +};
> +
> +static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
>  {
> -	return (time_t)(filetime_to_hnsec(ft) / 10000000);
> +	long long hnsec = filetime_to_hnsec(ft);
> +	ts->tv_sec = (time_t)(hnsec / 10000000);
> +	ts->tv_nsec = (hnsec % 10000000) * 100;
>  }
>  
>  /*
> - * Use mingw specific stat()/lstat()/fstat() implementations on Windows.
> + * Use mingw specific stat()/lstat()/fstat() implementations on Windows,
> + * including our own struct stat with 64 bit st_size and nanosecond-precision
> + * file times.
>   */
>  #define off_t off64_t
>  #define lseek _lseeki64
>  
> -/* use struct stat with 64 bit st_size */
> +struct mingw_stat {
> +    _dev_t st_dev;
> +    _ino_t st_ino;
> +    _mode_t st_mode;
> +    short st_nlink;
> +    short st_uid;
> +    short st_gid;
> +    _dev_t st_rdev;
> +    off64_t st_size;
> +    struct timespec st_atim;
> +    struct timespec st_mtim;
> +    struct timespec st_ctim;
> +};
> +
> +#define st_atime st_atim.tv_sec
> +#define st_mtime st_mtim.tv_sec
> +#define st_ctime st_ctim.tv_sec
> +
>  #ifdef stat
>  #undef stat
>  #endif
> -#define stat _stati64
> +#define stat mingw_stat
>  int mingw_lstat(const char *file_name, struct stat *buf);
>  int mingw_stat(const char *file_name, struct stat *buf);
>  int mingw_fstat(int fd, struct stat *buf);
> @@ -321,13 +347,6 @@ int mingw_fstat(int fd, struct stat *buf);
>  #endif
>  #define lstat mingw_lstat
>  
> -#ifndef _stati64
> -# define _stati64(x,y) mingw_stat(x,y)
> -#elif defined (_USE_32BIT_TIME_T)
> -# define _stat32i64(x,y) mingw_stat(x,y)
> -#else
> -# define _stat64(x,y) mingw_stat(x,y)
> -#endif
>  
>  int mingw_utime(const char *file_name, const struct utimbuf *times);
>  #define utime mingw_utime
> diff --git a/config.mak.uname b/config.mak.uname
> index b64b63c..a18a4cc 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -346,7 +346,7 @@ ifeq ($(uname_S),Windows)
>  	NO_SVN_TESTS = YesPlease
>  	RUNTIME_PREFIX = YesPlease
>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
> -	NO_NSEC = YesPlease
> +	USE_NSEC = YesPlease
>  	USE_WIN32_MMAP = YesPlease
>  	# USE_NED_ALLOCATOR = YesPlease
>  	UNRELIABLE_FSTAT = UnfortunatelyYes
> @@ -498,7 +498,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>  	NO_PERL_MAKEMAKER = YesPlease
>  	RUNTIME_PREFIX = YesPlease
>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
> -	NO_NSEC = YesPlease
> +	USE_NSEC = YesPlease
>  	USE_WIN32_MMAP = YesPlease
>  	USE_NED_ALLOCATOR = YesPlease
>  	UNRELIABLE_FSTAT = UnfortunatelyYes
> 

Why not also enable it in our special msysgit section?

diff --git a/config.mak.uname b/config.mak.uname
index b64b63c..6326794 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -535,6 +535,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
        INTERNAL_QSORT = YesPlease
        HAVE_LIBCHARSET_H = YesPlease
        NO_GETTEXT = YesPlease
+       USE_NSEC = YesPlease
 else
        NO_CURL = YesPlease
 endif

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 3/3] Win32: implement nanosecond-precision file times

[Karsten Blees]

Am 13.02.2015 um 00:15 schrieb Thomas Braun:
> Am 12.02.2015 um 00:53 schrieb Karsten Blees:
>> diff --git a/config.mak.uname b/config.mak.uname
>> index b64b63c..a18a4cc 100644
>> --- a/config.mak.uname
>> +++ b/config.mak.uname
>> @@ -346,7 +346,7 @@ ifeq ($(uname_S),Windows)
>>  	NO_SVN_TESTS = YesPlease
>>  	RUNTIME_PREFIX = YesPlease
>>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>> -	NO_NSEC = YesPlease
>> +	USE_NSEC = YesPlease
>>  	USE_WIN32_MMAP = YesPlease
>>  	# USE_NED_ALLOCATOR = YesPlease
>>  	UNRELIABLE_FSTAT = UnfortunatelyYes
>> @@ -498,7 +498,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
>>  	NO_PERL_MAKEMAKER = YesPlease
>>  	RUNTIME_PREFIX = YesPlease
>>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>> -	NO_NSEC = YesPlease
>> +	USE_NSEC = YesPlease
>>  	USE_WIN32_MMAP = YesPlease
>>  	USE_NED_ALLOCATOR = YesPlease
>>  	UNRELIABLE_FSTAT = UnfortunatelyYes
>>
> 
> Why not also enable it in our special msysgit section?
> 
> diff --git a/config.mak.uname b/config.mak.uname
> index b64b63c..6326794 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -535,6 +535,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))
>         INTERNAL_QSORT = YesPlease
>         HAVE_LIBCHARSET_H = YesPlease
>         NO_GETTEXT = YesPlease
> +       USE_NSEC = YesPlease
>  else
>         NO_CURL = YesPlease
>  endif
> 

The msysgit section is within MINGW (i.e. already covered by the 2nd hunk), don't let the indentation fool you.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> This patch series was inspired by the problem that Git does not
> detect changed file content if st_size, st_mtime and st_ctime
> are unchanged. This was apparently caused by VSS2Git resetting
> mtime to a value in the past. [1]
>
> I believe (or rather hope) that all involved in the discussion
> agree that Git cannot reasonably be expected to detect changed
> file content if file time(s) are reset on purpose.
>
> However, some users have expressed concerns that 'same size and
> mtime' [2] may theoretically happen by chance in daily operation.

Hmph.

Haven't we already accepted that it is not just "may theoretically
happen" and had counter-measures in racy-git detection machinery
for quite some time?

Re: Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Johannes Schindelin]

Hi,

On 2015-02-12 20:48, Junio C Hamano wrote:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> This patch series was inspired by the problem that Git does not
>> detect changed file content if st_size, st_mtime and st_ctime
>> are unchanged. This was apparently caused by VSS2Git resetting
>> mtime to a value in the past. [1]
>>
>> I believe (or rather hope) that all involved in the discussion
>> agree that Git cannot reasonably be expected to detect changed
>> file content if file time(s) are reset on purpose.
>>
>> However, some users have expressed concerns that 'same size and
>> mtime' [2] may theoretically happen by chance in daily operation.
> 
> Hmph.
> 
> Haven't we already accepted that it is not just "may theoretically
> happen" and had counter-measures in racy-git detection machinery
> for quite some time?

I agree that the "racy-git" reference is more of a red herring; it appears that the report Karsten referred to is based on a different understanding of the mtime than mine (it is purported that the mtime of a file should reflect the time when it entered the repository, rather than the time when the file was written to disk).

Please let that not distract you.

This patch series has merits on the basis of populating st_mtim.tv_nsec already. We can provide this information in the common case (i.e. NTFS, *not* FAT), so we should.

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Karsten Blees]

Am 12.02.2015 um 20:48 schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> However, some users have expressed concerns that 'same size and
>> mtime' [2] may theoretically happen by chance in daily operation.
> 
> Hmph.
> 
> Haven't we already accepted that it is not just "may theoretically
> happen" and had counter-measures in racy-git detection machinery
> for quite some time?
> 

Racy-git only triggers for files that are modified at the same time
as .git/index (i.e. we don't know if the stat cache is up to date).

This is more about copying 'old' things around, which usually also
copies mtime on Windows. E.g.:

  # create two files with slightly different mtime
  for i in {1..10}; do (echo "v1" >> test); done &&
  for i in {1..10}; do (echo "v2" >> test2); done
  # wait a bit so that '.git/index' is always newer than 'test' / 'test2'
  sleep 1
  git add test
  git commit -m v1
  # copy test2 over test (similar to 'cp -p', but native 'copy' also
  # copies mtime nanoseconds)
  cmd //c "copy /y test2 test"
  git add test
  git commit -m v2

Without these patches, git does not detect the change, and the second
git add / git commit are noops.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> This is more about copying 'old' things around, which usually also
> copies mtime on Windows. E.g.:
>
>   # create two files with slightly different mtime
>   for i in {1..10}; do (echo "v1" >> test); done &&
>   for i in {1..10}; do (echo "v2" >> test2); done
>   # wait a bit so that '.git/index' is always newer than 'test' / 'test2'
>   sleep 1
>   git add test
>   git commit -m v1
>   # copy test2 over test (similar to 'cp -p', but native 'copy' also
>   # copies mtime nanoseconds)
>   cmd //c "copy /y test2 test"
>   git add test
>   git commit -m v2
>
> Without these patches, git does not detect the change, and the second
> git add / git commit are noops.

We do have sec/nsec fields in cache_time structure, so I have
nothing against updating the msysGit port to fill that value.

I was and am just reacting to the fact that this is sold as if it
"fixes" something.  It doesn't fundamentally change the fact that
mtime that does not follow the semantics Dscho mentioned in his
earlier message does not work well with Git.

Having said that, even with such a patch, as long as the system is
sufficiently fast, test and test2 will have nonoseconds identical
timestamp and you would have the same issue, no?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Karsten Blees]

Am 13.02.2015 um 00:38 schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> This is more about copying 'old' things around, which usually also
>> copies mtime on Windows. E.g.:
>>
>>   # create two files with slightly different mtime
>>   for i in {1..10}; do (echo "v1" >> test); done &&
>>   for i in {1..10}; do (echo "v2" >> test2); done
>>   # wait a bit so that '.git/index' is always newer than 'test' / 'test2'
>>   sleep 1
>>   git add test
>>   git commit -m v1
>>   # copy test2 over test (similar to 'cp -p', but native 'copy' also
>>   # copies mtime nanoseconds)
>>   cmd //c "copy /y test2 test"
>>   git add test
>>   git commit -m v2
>>
>> Without these patches, git does not detect the change, and the second
>> git add / git commit are noops.
> 
> We do have sec/nsec fields in cache_time structure, so I have
> nothing against updating the msysGit port to fill that value.
> 
> I was and am just reacting to the fact that this is sold as if it
> "fixes" something.

Sorry, that must have been a misunderstanding. This series does
NOT fix the problem with VSS2Git, nor any other tool that abuses
mtime for the author's birthday or whatever.

The issue that two files may accidentally have the same size and
mtime was just brought up in this discussion.

> It doesn't fundamentally change the fact that
> mtime that does not follow the semantics Dscho mentioned in his
> earlier message does not work well with Git.
> 
> Having said that, even with such a patch, as long as the system is
> sufficiently fast, test and test2 will have nonoseconds identical
> timestamp and you would have the same issue, no?
> 

Right. Where "sufficiently fast" would mean opening and closing a
file ten times in less than 100ns...on Windows... ;-)

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> Am 13.02.2015 um 00:38 schrieb Junio C Hamano:
>> 
>> We do have sec/nsec fields in cache_time structure, so I have
>> nothing against updating the msysGit port to fill that value.

Having said that, we do not enable the NSEC stuff by default on Unix
for a reason.  I'd expect those who know Windows filesystems well to
pick the default there wisely ;-)

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Karsten Blees]

Am 13.02.2015 um 20:28 schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> Am 13.02.2015 um 00:38 schrieb Junio C Hamano:
>>>
>>> We do have sec/nsec fields in cache_time structure, so I have
>>> nothing against updating the msysGit port to fill that value.
> 
> Having said that, we do not enable the NSEC stuff by default on Unix
> for a reason.  I'd expect those who know Windows filesystems well to
> pick the default there wisely ;-)
> 

Now I'm a bit confused about the discrepancy between racy-git.txt and
the Makefile.

Racy-git.txt explains that the nsec-part may be dropped when an inode
is flushed to disk if the file system doesn't support nsec resolution.
This was supposedly an issue with the Linux kernel fixed back in 2005.

In my understanding, this means that git would see the file as
changed and re-check the content (i.e. it will hurt performance).

IOW: Git may be slow if the file system cache has better file time
resolution than the on-disk file system representation.


However, the Makefile has this to say on the subject:

# Define USE_NSEC below if you want git to care about sub-second file mtimes
# and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
# it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
# randomly break unless your underlying filesystem supports those sub-second
# times (my ext3 doesn't).

Am I missing something? Is there anything in Git that will actually
"break" with USE_NSEC if the OS / file system doesn't support it
(rather than just being slow)?

History:
* The Makefile comment was added in 2005 (bdd4da59), along with a
  comment in read-cache.c explaining the issue (i.e. flushing to disk
  will clear the nsec field).
* The comment in read-cache.c was removed in 2008 (7a51ed66),
  seemingly dropping USE_NSEC support entirely.
* USE_NSEC support was re-added (without the read-cache.c comment) in
  2009 (fba2f38a).


Regarding the Windows situation: I've just verified (on my Win7 x64
box) that file times obtained through a variety of APIs (GetFileTime,
GetFileAttributesEx, GetFileInformationByHandle, FindFirstFile) are
consistent and properly rounded to the file system's resolution (e.g.
10ms / 2s for FAT). This is even if the file is still open and I try
to SetFileTime() to unrounded values.

So I think enabling USE_NSEC should be fine on Windows.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

> However, the Makefile has this to say on the subject:
>
> # Define USE_NSEC below if you want git to care about sub-second file mtimes
> # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
> # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
> # randomly break unless your underlying filesystem supports those sub-second
> # times (my ext3 doesn't).
>
> Am I missing something?

I think "it would break" is about show-diff which wanted to use the
cached stat information for freshness.

	>foo
	git update-index --add foo
        sleep 2
        >foo
        git diff-files ;# modern counterpart of show-diff

would say that "foo" is *different*, because the plumbing commands
like diff-files expect you to refresh the index before you call
them.

And if you did "git update-index --refresh" after touching "foo" the
last time before running "git diff-files" in the above sequence, you
should expect that it does not say "foo" is different, no matter how
much time passes between the time you run that "refresh" and
"diff-files" (or between the time you last touched "foo" and you run
"refresh", for that matter), as long as you do not touch "foo" in
the meantime.  The following should say "foo" is *not* different,
that is:

	>foo
	git update-index --add foo
        sleep 2
        >foo
        sleep arbitrary
        git update-index --refresh
        sleep arbitrary
        git diff-files ;# modern counterpart of show-diff

If you use NSEC, however, and "refresh" grabbed a subsecond time and
then later "diff-files" learned a truncated/rounded time because the
filesystem later purged the cached inodes and re-read it from the
underlying filesystem with no subsecond time resolution, the times
would not match so you will again see "diff-files" report that "foo"
is now different.

That is what the comment you cited is about.

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 0/3] Win32: nanosecond-precision file times

[Karsten Blees]

Am 16.02.2015 um 23:10 schrieb Junio C Hamano:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> However, the Makefile has this to say on the subject:
>>
>> # Define USE_NSEC below if you want git to care about sub-second file mtimes
>> # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
>> # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
>> # randomly break unless your underlying filesystem supports those sub-second
>> # times (my ext3 doesn't).
>>
>> Am I missing something?
> 
[...]
> 
> If you use NSEC, however, and "refresh" grabbed a subsecond time and
> then later "diff-files" learned a truncated/rounded time because the
> filesystem later purged the cached inodes and re-read it from the
> underlying filesystem with no subsecond time resolution, the times
> would not match so you will again see "diff-files" report that "foo"
> is now different.
> 
> That is what the comment you cited is about.
> 

OK, so it all boils down to the "inode cache doesn't round to on-disk
resolution" issue after all, as explained in racy-git.txt.

But then the Makefile comment is quite misleading. Enabling USE_NSEC
will not unconditionally "BREAK YOUR LOCAL DIFFS". Show-diff / diff-files
will also not "break", but may report false positives instead (which may
be worse than failing hard...).

It also seems to me that this is a Linux-only issue which is only remotely
related to the USE_NSEC setting or file systems' timestamp resolutions.

The kernel patch referenced in racy-git.txt only addresses sub-second
resolutions. So even if USE_NSEC is *disabled*, the diff-files issue will
bite you on e.g. FAT32-formatted flash-drives on Linux, at least on
re-mount ("sync && echo 2>/proc/sys/vm/drop_caches" didn't seem to trigger
the rounding, though).

I also suspect that the sub-second rounding function of that patch
(timespec_trunc()) takes some invalid shortcuts - if you configure the
kernel for 300 jiffies per second (i.e. 3,333,333 ns per tick), UDF, NTFS,
SMBFS and CIFS file times will most likely not be properly rounded in the
inode cache. Haven't tested this, though.

So the only file systems with reliable file times on Linux seem to be
those with exactly 1s or 1ns resolution...?

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.