dir.c: regression fix for add_excludes with fscache #1407

jeffhostetler · 2017-12-19T17:09:53Z

Fix regression described in:
#1392
which was introduced in:
b235337

When the user has a .gitignore file that is a symlink, the fscache
optimization introduced above caused the stat-data from the symlink,
rather that of the target file, to be returned. Later when the ignore
file was read, the buffer length did not match the stat.st_size field
and we called die("cannot use as an exclude file")

Added code to replace the stat-data when the path is a symlink. This
preserves the effect of the performance gains for the normal case where
it is not a symlink.

Signed-off-by: Jeff Hostetler jeffhost@microsoft.com

dir.c

+		 * If the .gitignore file is successfully opened and in the
+		 * rare case that it is a symlink, we fstat it to replace the
+		 * original lstat stat-data to complete our illusion.
+		 */
 		if (lstat(fname, &st) < 0) {


compat/win32/fscache.c

@@ -257,7 +252,7 @@ static void fscache_clear(void)
 /*
 * Checks if the cache is enabled for the given path.
 */
-static inline int fscache_enabled(const char *path)
+int fscache_enabled(const char *path)


Make fscache_enabled() function public rather than static. Remove unneeded fscache_is_enabled() function. Change is_fscache_enabled() macro to call fscache_enabled(). is_fscache_enabled() now takes a pathname so that the answer is more precise and mean "is fscache enabled for this pathname", since fscache only stores repo-relative paths and not absolute paths, we can avoid attempting lookups for absolute paths. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>

Fix regression described in: git-for-windows#1392 which was introduced in: git-for-windows@b235337 Problem Symptoms ================ When the user has a .gitignore file that is a symlink, the fscache optimization introduced above caused the stat-data from the symlink, rather that of the target file, to be returned. Later when the ignore file was read, the buffer length did not match the stat.st_size field and we called die("cannot use <path> as an exclude file") Optimization Rationale ====================== The above optimization calls lstat() before open() primarily to ask fscache if the file exists. It gets the current stat-data as a side effect essentially for free (since we already have it in memory). If the file does not exist, it does not need to call open(). And since very few directories have .gitignore files, we can greatly reduce time spent in the filesystem. Discussion of Fix ================= The above optimization calls lstat() rather than stat() because the fscache only intercepts lstat() calls. Calls to stat() stay directed to the mingw_stat() completly bypassing fscache. Furthermore, calls to mingw_stat() always call {open, fstat, close} so that symlinks are properly dereferenced, which adds *additional* open/close calls on top of what the original code in dir.c is doing. Since the problem only manifests for symlinks, we add code to overwrite the stat-data when the path is a symlink. This preserves the effect of the performance gains provided by the fscache in the normal case. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>

dscho · 2017-12-20T16:56:14Z

Thank you so much!