{"thread":{"id":"15689","subject":"[PATCH 4/4] cygwin: Use native Win32 API for stat","startedAt":"2008-09-27T08:43:49Z","lastAt":"2008-09-30T20:26:25Z","messageCount":11,"participants":["Dmitry Potapov","Johannes Sixt","Alex Riesen","Shawn O. Pearce","Marcus Griep"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"91737","messageId":"20080927084349.GC21650@dpotapov.dyndns.org","threadId":"15689","inReplyTo":null,"subject":"[PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-09-27T08:43:49Z","receivedAt":"2008-09-27T08:43:49Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"lstat/stat functions in Cygwin are very slow, because they try to emulate\nsome *nix things that Git does not actually need. This patch adds Win32\nspecific implementation of these functions for Cygwin.\n\nThis implementation handles most situation directly but in some rare cases\nit falls back on the implementation provided for Cygwin. This is necessary\nfor two reasons:\n\n- Cygwin has its own file hierarchy, so absolute paths used in Cygwin is\n  not suitable to be used Win32 API. cygwin_conv_to_win32_path can not be\n  used because it automatically dereference Cygwin symbol links, also it\n  causes extra syscall. Fortunately Git rarely use absolute paths, so we\n  always use Cygwin implementation for absolute paths.\n\n- Support of symbol links. Cygwin stores symbol links as ordinary using\n  one of two possible formats. Therefore, the fast implementation falls\n  back to Cygwin functions if it detects potential use of symbol links.\n\nThe speed of this implementation should be the same as mingw_lstat for\ncommon cases, but it is considerable slower when the specified file name\ndoes not exist.\n\nDespite all efforts to make the fast implementation as robust as possible,\nit may not work well for some very rare situations. I am aware only one\nsituation: use Cygwin mount to bind unrelated paths inside repository\ntogether.  Therefore, the core.cygwinnativestat configuration option is\nprovided, which controls whether native or Cygwin version of stat is used.\n\nSigned-off-by: Dmitry Potapov <dpotapov@gmail.com>\n---\n Documentation/config.txt |    9 +++\n Makefile                 |    4 ++\n compat/cygwin.c          |  125 ++++++++++++++++++++++++++++++++++++++++++++++\n compat/cygwin.h          |    9 +++\n git-compat-util.h        |    1 +\n 5 files changed, 148 insertions(+), 0 deletions(-)\n create mode 100644 compat/cygwin.c\n create mode 100644 compat/cygwin.h\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bea867d..c198bc0 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -117,6 +117,15 @@ core.fileMode::\n \tthe working copy are ignored; useful on broken filesystems like FAT.\n \tSee linkgit:git-update-index[1]. True by default.\n \n+core.cygwinNativeStat::\n+\tThis option is only used by Cygwin implementation of Git. If false,\n+\tthe Cygwin stat() and lstat() functions are used. This may be useful\n+\tif your repository consists of a few separate directories joined in\n+\tone hierarchy using Cygwin mount. If true, Git uses native Win32 API\n+\twhenever it is possible and falls back to Cygwin functions only to\n+\thandle symbol links. The native mode is more than twice faster than\n+\tnormal Cygwin l/stat() functions. True by default.\n+\n core.trustctime::\n \tIf false, the ctime differences between the index and the\n \tworking copy are ignored; useful when the inode change time\ndiff --git a/Makefile b/Makefile\nindex 3c0664a..0708390 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -347,6 +347,7 @@ LIB_H += cache.h\n LIB_H += cache-tree.h\n LIB_H += commit.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/cygwin.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -747,6 +748,9 @@ ifeq ($(uname_S),HP-UX)\n \tNO_SYS_SELECT_H = YesPlease\n \tSNPRINTF_RETURNS_BOGUS = YesPlease\n endif\n+ifneq (,$(findstring CYGWIN,$(uname_S)))\n+\tCOMPAT_OBJS += compat/cygwin.o\n+endif\n ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_MMAP = YesPlease\n \tNO_PREAD = YesPlease\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nnew file mode 100644\nindex 0000000..ad09b17\n--- /dev/null\n+++ b/compat/cygwin.c\n@@ -0,0 +1,125 @@\n+#define WIN32_LEAN_AND_MEAN\n+#include \"../git-compat-util.h\"\n+#include \"win32.h\"\n+#include \"../cache.h\" /* to read configuration */\n+\n+static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)\n+{\n+\tlong long winTime = ((long long)ft->dwHighDateTime << 32) + ft->dwLowDateTime;\n+\twinTime -= 116444736000000000LL; /* Windows to Unix Epoch conversion */\n+\tts->tv_sec = (time_t)(winTime/10000000); /* 100-nanosecond interval to seconds */\n+\tts->tv_nsec = (long)(winTime - ts->tv_sec*10000000LL) * 100; /* nanoseconds */\n+}\n+\n+#define size_to_blocks(s) (((s)+511)/512)\n+\n+/* do_stat is a common implementation for cygwin_lstat and cygwin_stat.\n+ *\n+ * To simplify its logic, in the case of cygwin symlinks, this implementation\n+ * falls back to the cygwin version of stat/lstat, which is provided as the\n+ * last argument.\n+ */\n+static int do_stat(const char *file_name, struct stat *buf, stat_fn_t cygstat)\n+{\n+\tWIN32_FILE_ATTRIBUTE_DATA fdata;\n+\n+\tif (file_name[0] == '/')\n+\t\treturn cygstat (file_name, buf);\n+\n+\tif (!(errno = get_file_attr(file_name, &fdata))) {\n+\t\t/*\n+\t\t * If the system attribute is set and it is not a directory then\n+\t\t * it could be a symbol link created in the nowinsymlinks mode.\n+\t\t * Normally, Cygwin works in the winsymlinks mode, so this situation\n+\t\t * is very unlikely. For the sake of simplicity of our code, let's\n+\t\t * Cygwin to handle it.\n+\t\t */\n+\t\tif ((fdata.dwFileAttributes & FILE_ATTRIBUTE_SYSTEM) &&\n+\t\t    !(fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))\n+\t\t\treturn cygstat (file_name, buf);\n+\n+\t\t/* fill out the stat structure */\n+\t\tbuf->st_dev = buf->st_rdev = 0; /* not used by Git */\n+\t\tbuf->st_ino = 0;\n+\t\tbuf->st_mode = file_attr_to_st_mode (fdata.dwFileAttributes);\n+\t\tbuf->st_nlink = 1;\n+\t\tbuf->st_uid = buf->st_gid = 0;\n+#ifdef __CYGWIN_USE_BIG_TYPES__\n+\t\tbuf->st_size = ((_off64_t)fdata.nFileSizeHigh << 32) +\n+\t\t\tfdata.nFileSizeLow;\n+#else\n+\t\tbuf->st_size = (off_t)fdata.nFileSizeLow;\n+#endif\n+\t\tbuf->st_blocks = size_to_blocks(buf->st_size);\n+\t\tfiletime_to_timespec(&fdata.ftLastAccessTime, &buf->st_atim);\n+\t\tfiletime_to_timespec(&fdata.ftLastWriteTime, &buf->st_mtim);\n+\t\tfiletime_to_timespec(&fdata.ftCreationTime, &buf->st_ctim);\n+\t\treturn 0;\n+\t} else if (errno == ENOENT) {\n+\t\t/*\n+\t\t * In the winsymlinks mode (which is the default), Cygwin\n+\t\t * emulates symbol links using Windows shortcut files. These\n+\t\t * files are formed by adding .lnk extension. So, if we have\n+\t\t * not found the specified file name, it could be that it is\n+\t\t * a symbol link. Let's Cygwin to deal with that.\n+\t\t */\n+\t\treturn cygstat (file_name, buf);\n+\t}\n+\treturn -1;\n+}\n+\n+/* We provide our own lstat/stat functions, since the provided Cygwin versions\n+ * of these functions are too slow. These stat functions are tailored for Git's\n+ * usage, and therefore they are not meant to be complete and correct emulation\n+ * of lstat/stat functionality.\n+ */\n+static int cygwin_lstat(const char *path, struct stat *buf)\n+{\n+\treturn do_stat(path, buf, lstat);\n+}\n+\n+static int cygwin_stat(const char *path, struct stat *buf)\n+{\n+\treturn do_stat(path, buf, stat);\n+}\n+\n+\n+/*\n+ * At start up, we are trying to determine whether Win32 API or cygwin stat\n+ * functions should be used. The choice is determined by core.cygwinnativestat.\n+ * Reading this option is not always possible immediately as git_dir may be\n+ * not be set yet. So until it is set, use cygwin lstat/stat functions.\n+ */\n+static int native_stat = 1;\n+\n+static int git_cygwin_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"core.cygwinnativestat\"))\n+\t\tnative_stat = git_config_bool(var, value);\n+\treturn 0;\n+}\n+\n+static int init_stat(void)\n+{\n+\tif (have_git_dir()) {\n+\t\tgit_config(git_cygwin_config, NULL);\n+\t\tcygwin_stat_fn = native_stat ? cygwin_stat : stat;\n+\t\tcygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+static int cygwin_stat_stub(const char *file_name, struct stat *buf)\n+{\n+\treturn (init_stat() ? stat : *cygwin_stat_fn)(file_name, buf);\n+}\n+\n+static int cygwin_lstat_stub(const char *file_name, struct stat *buf)\n+{\n+\treturn (init_stat() ? lstat : *cygwin_lstat_fn)(file_name, buf);\n+}\n+\n+stat_fn_t cygwin_stat_fn = cygwin_stat_stub;\n+stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;\n+\ndiff --git a/compat/cygwin.h b/compat/cygwin.h\nnew file mode 100644\nindex 0000000..a3229f5\n--- /dev/null\n+++ b/compat/cygwin.h\n@@ -0,0 +1,9 @@\n+#include <sys/types.h>\n+#include <sys/stat.h>\n+\n+typedef int (*stat_fn_t)(const char*, struct stat*);\n+extern stat_fn_t cygwin_stat_fn;\n+extern stat_fn_t cygwin_lstat_fn;\n+\n+#define stat(path, buf) (*cygwin_stat_fn)(path, buf)\n+#define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex db2836f..cd9752c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -85,6 +85,7 @@\n #undef _XOPEN_SOURCE\n #include <grp.h>\n #define _XOPEN_SOURCE 600\n+#include \"compat/cygwin.h\"\n #else\n #undef _ALL_SOURCE /* AIX 5.3L defines a struct list with _ALL_SOURCE. */\n #include <grp.h>\n-- \n1.6.0.2.237.g0297e5\n"},{"id":"91743","messageId":"20080927163349.GE21650@dpotapov.dyndns.org","threadId":"15689","inReplyTo":"347507080809270851y79764dbcgba1ef5a1d58bdd3e@mail.gmail.com","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-09-27T16:33:50Z","receivedAt":"2008-09-27T16:33:50Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sat, Sep 27, 2008 at 11:51:24AM -0400, Marcus Griep wrote:\n> \n> Overall, looks good, though Alex's comment of using \"cygwin.nativestat\" may\n> be a better descriptor for the config flag.\n\nPerhaps... but I am not sure about policy for options in config file.\ncore.cygwinnativestat was proposed by Shawn, so I would like to hear\nhis opinion.\n\n> I also think there is more refactoring that could be done, with there being\n> common code paths still existing in MinGW and Cygwin.\n\nI am not sure that there is much common code left: stat structures are\ndifferent in MinGW and Cygwin (different fields and their types), and I\ndo not think having one function with a lot of #ifdef __CYGWIN__ in it\nis actual improvement. But you can send a patch on top of mine and let\nother people decide if it is a worty goal.\n\nDmitry\n"},{"id":"91746","messageId":"200809272035.03833.johannes.sixt@telecom.at","threadId":"15689","inReplyTo":"20080927084349.GC21650@dpotapov.dyndns.org","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-09-27T18:35:03Z","receivedAt":"2008-09-27T18:35:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 27. September 2008, Dmitry Potapov wrote:\n> lstat/stat functions in Cygwin are very slow, because they try to emulate\n> some *nix things that Git does not actually need. This patch adds Win32\n> specific implementation of these functions for Cygwin.\n>\n> This implementation handles most situation directly but in some rare cases\n> it falls back on the implementation provided for Cygwin.\n\nEven though I was concerned about code duplication earlier, with the \nfactorization that you do in this series this is acceptable, in particular, \nsince working out at a solution that deals with the time_t vs. timespec \ndifference we would need dirty tricks that are not worth it.\n\n(But see my comment about get_file_attr() in a separate mail.)\n\n> +core.cygwinNativeStat::\n\nThis name is *really* odd, for two reasons:\n\n- If I read \"native\" in connection with Windows, I would understand Windows's \nimplementation as \"native\". Cygwin is not native - it's a bolted-on feature.\n\n- This name talks about the implementation, not about its effect.\n\nPerhaps a better name would be core.ignoreCygwinFSFeatures, and the \ndescription would only mention that setting this to true (the default) makes \nmany operations much faster, but makes it impossible to use File System \nFeatures A and B and C in the repository. \"If you need one of these features, \nset this to false.\"\n\n(And after writing above paragraphs I notice, that you actually really meant \nWindows's \"native\" stat; see how confusing the name is?)\n\n> +static inline void filetime_to_timespec(const FILETIME *ft, struct\n> timespec *ts) +{\n> +\tlong long winTime = ((long long)ft->dwHighDateTime << 32) +\n> ft->dwLowDateTime; +\twinTime -= 116444736000000000LL; /* Windows to Unix\n> Epoch conversion */ +\tts->tv_sec = (time_t)(winTime/10000000); /*\n> 100-nanosecond interval to seconds */ +\tts->tv_nsec = (long)(winTime -\n> ts->tv_sec*10000000LL) * 100; /* nanoseconds */ +}\n\nShorter lines in this function would be appreciated (and not just because my \nMUA can't deal with them ;).\n\n-- Hannes\n"},{"id":"91750","messageId":"20080927215406.GG21650@dpotapov.dyndns.org","threadId":"15689","inReplyTo":"200809272035.03833.johannes.sixt@telecom.at","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-09-27T21:54:06Z","receivedAt":"2008-09-27T21:54:06Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sat, Sep 27, 2008 at 08:35:03PM +0200, Johannes Sixt wrote:\n> \n> > +core.cygwinNativeStat::\n> \n> This name is *really* odd, for two reasons:\n> \n> - If I read \"native\" in connection with Windows, I would understand Windows's \n> implementation as \"native\". Cygwin is not native - it's a bolted-on feature.\n> \n> - This name talks about the implementation, not about its effect.\n> \n> Perhaps a better name would be core.ignoreCygwinFSFeatures, and the \n> description would only mention that setting this to true (the default) makes \n> many operations much faster, but makes it impossible to use File System \n> Features A and B and C in the repository. \"If you need one of these features, \n> set this to false.\"\n> \n> (And after writing above paragraphs I notice, that you actually really meant \n> Windows's \"native\" stat; see how confusing the name is?)\n\nIt was Shawn's suggestion. I don't care much about the name as long as\nit is explained in the documentation... Therefore, I accepted what Shawn\nsaid without giving it any thought. Now, when you bring this name to my\nattention, I believe core.useCygwinStat (in the opposite to the current\ncore.cygwinNativeStat) would be a better name. Your name is okay too,\nbut a bit too long for my taste and not specific enough (I suppose\nCygwin does many FS related tricks). Anyway, I don't have a strong\nopinion here, so just whatever most people like is fine with me :)\n\n> \n> > +static inline void filetime_to_timespec(const FILETIME *ft, struct\n> > timespec *ts) +{\n> > +\tlong long winTime = ((long long)ft->dwHighDateTime << 32) +\n> > ft->dwLowDateTime; +\twinTime -= 116444736000000000LL; /* Windows to Unix\n> > Epoch conversion */ +\tts->tv_sec = (time_t)(winTime/10000000); /*\n> > 100-nanosecond interval to seconds */ +\tts->tv_nsec = (long)(winTime -\n> > ts->tv_sec*10000000LL) * 100; /* nanoseconds */ +}\n> \n> Shorter lines in this function would be appreciated (and not just because my \n> MUA can't deal with them ;).\n\nI am sorry, I did not notice that the line got longer than 80 columns.\nI will resent the patch once the issue with the name of the option is\nresolved.\n\nDmitry\n"},{"id":"91777","messageId":"200809281124.08364.johannes.sixt@telecom.at","threadId":"15689","inReplyTo":"20080927215406.GG21650@dpotapov.dyndns.org","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-09-28T09:24:08Z","receivedAt":"2008-09-28T09:24:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 27. September 2008, Dmitry Potapov wrote:\n> On Sat, Sep 27, 2008 at 08:35:03PM +0200, Johannes Sixt wrote:\n> > > +core.cygwinNativeStat::\n> >\n> > This name is *really* odd, for two reasons:\n...\n> It was Shawn's suggestion. I don't care much about the name as long as\n> it is explained in the documentation... Therefore, I accepted what Shawn\n> said without giving it any thought.\n\nShawn is an importen git-o-maniac, but it's certainly not blasphemy to \nquestion his words of wisdom ;)\n\n> Now, when you bring this name to my \n> attention, I believe core.useCygwinStat (in the opposite to the current\n> core.cygwinNativeStat) would be a better name. Your name is okay too,\n> but a bit too long for my taste and not specific enough (I suppose\n> Cygwin does many FS related tricks). Anyway, I don't have a strong\n> opinion here, so just whatever most people like is fine with me :)\n\nMy point is that emphasis on \"stat\" in the name is wrong: That's about \nimplementation, but not about the effect. Why wouldn't 'ignoreCygwinFSTricks' \nbe specific enough? By using a native stat implementation, *all* of them are \nignored. Yes, you fall back to Cygwin's stat sometimes, but these are cases \nwhere the *effect* is not that relevant. (And the length of the name doesn't \nworry me, considering how many people would want to change the default.)\n\n-- Hannes\n"},{"id":"91778","messageId":"20080928095045.GA3746@blimp.localhost","threadId":"15689","inReplyTo":"20080927084349.GC21650@dpotapov.dyndns.org","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-09-28T09:50:45Z","receivedAt":"2008-09-28T09:50:45Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Dmitry Potapov, Sat, Sep 27, 2008 10:43:49 +0200:\n> Despite all efforts to make the fast implementation as robust as possible,\n> it may not work well for some very rare situations. I am aware only one\n> situation: use Cygwin mount to bind unrelated paths inside repository\n> together.  Therefore, the core.cygwinnativestat configuration option is\n> provided, which controls whether native or Cygwin version of stat is used.\n\ncygwin.tryWindowsState? (I think cygwin has to get its own section)\n\n> +static int do_stat(const char *file_name, struct stat *buf, stat_fn_t cygstat)\n> +{\n> +\tWIN32_FILE_ATTRIBUTE_DATA fdata;\n> +\n> +\tif (file_name[0] == '/')\n> +\t\treturn cygstat (file_name, buf);\n> +\n> +\tif (!(errno = get_file_attr(file_name, &fdata))) {\n> +\t\t/*\n> +\t\t * If the system attribute is set and it is not a directory then\n> +\t\t * it could be a symbol link created in the nowinsymlinks mode.\n> +\t\t * Normally, Cygwin works in the winsymlinks mode, so this situation\n> +\t\t * is very unlikely. For the sake of simplicity of our code, let's\n> +\t\t * Cygwin to handle it.\n> +\t\t */\n> +\t\tif ((fdata.dwFileAttributes & FILE_ATTRIBUTE_SYSTEM) &&\n> +\t\t    !(fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))\n> +\t\t\treturn cygstat (file_name, buf);\n\nformatting: space after function name.\n\n> +\n> +\t\t/* fill out the stat structure */\n> +\t\tbuf->st_dev = buf->st_rdev = 0; /* not used by Git */\n> +\t\tbuf->st_ino = 0;\n> +\t\tbuf->st_mode = file_attr_to_st_mode (fdata.dwFileAttributes);\n> +\t\tbuf->st_nlink = 1;\n> +\t\tbuf->st_uid = buf->st_gid = 0;\n> +#ifdef __CYGWIN_USE_BIG_TYPES__\n> +\t\tbuf->st_size = ((_off64_t)fdata.nFileSizeHigh << 32) +\n> +\t\t\tfdata.nFileSizeLow;\n> +#else\n> +\t\tbuf->st_size = (off_t)fdata.nFileSizeLow;\n> +#endif\n> +\t\tbuf->st_blocks = size_to_blocks(buf->st_size);\n> +\t\tfiletime_to_timespec(&fdata.ftLastAccessTime, &buf->st_atim);\n> +\t\tfiletime_to_timespec(&fdata.ftLastWriteTime, &buf->st_mtim);\n> +\t\tfiletime_to_timespec(&fdata.ftCreationTime, &buf->st_ctim);\n> +\t\treturn 0;\n> +\t} else if (errno == ENOENT) {\n> +\t\t/*\n> +\t\t * In the winsymlinks mode (which is the default), Cygwin\n> +\t\t * emulates symbol links using Windows shortcut files. These\n> +\t\t * files are formed by adding .lnk extension. So, if we have\n> +\t\t * not found the specified file name, it could be that it is\n> +\t\t * a symbol link. Let's Cygwin to deal with that.\n> +\t\t */\n> +\t\treturn cygstat (file_name, buf);\n> +\t}\n> +\treturn -1;\n> +}\n\nI like it and will be keeping in my tree. Thanks!\n"},{"id":"91854","messageId":"20080929153400.GJ17584@spearce.org","threadId":"15689","inReplyTo":"200809281124.08364.johannes.sixt@telecom.at","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-09-29T15:34:00Z","receivedAt":"2008-09-29T15:34:00Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt <johannes.sixt@telecom.at> wrote:\n> On Samstag, 27. September 2008, Dmitry Potapov wrote:\n> > On Sat, Sep 27, 2008 at 08:35:03PM +0200, Johannes Sixt wrote:\n> > > > +core.cygwinNativeStat::\n> > >\n> > > This name is *really* odd, for two reasons:\n> ...\n> > It was Shawn's suggestion. I don't care much about the name as long as\n> > it is explained in the documentation... Therefore, I accepted what Shawn\n> > said without giving it any thought.\n> \n> Shawn is an importen git-o-maniac, but it's certainly not blasphemy to \n> question his words of wisdom ;)\n\nAs Hannes points out, blindly accepting anything I say might not\nbe a good idea.  I have my moments of sanity, but I'm far, far\nfrom perfect.  ;-)\n\n> My point is that emphasis on \"stat\" in the name is wrong: That's about \n> implementation, but not about the effect. Why wouldn't 'ignoreCygwinFSTricks' \n> be specific enough?\n\nI like this a lot better.  I could see us also bypassing other Cygwin\nfunctions like open() in order to get faster system calls for Git.\nSince it would be byassing the same Cygwin path name translation\ncode it should be controlled by the same flag.\n\n> (And the length of the name doesn't \n> worry me, considering how many people would want to change the default.)\n\nAgreed.  Most people setting it would copy and paste from the\ndocumentation anyway.\n\nI wonder though if we can't automatically implement it by grabbing\na copy of the Cygwin mount table and comparing those paths to\n$GIT_DIR or $GIT_WORK_TREE.  If any mount table entry is contained\nwithin either of them then we know we can't use the native stat.\nIts rather common for neither of these to contain a mount point,\nand it is therefore easy to enable the native stat.\n\n-- \nShawn.\n"},{"id":"91869","messageId":"20080929182600.GI21650@dpotapov.dyndns.org","threadId":"15689","inReplyTo":"20080929153400.GJ17584@spearce.org","subject":"Re: [PATCH 4/4] cygwin: Use native Win32 API for stat","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-09-29T18:26:00Z","receivedAt":"2008-09-29T18:26:00Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Mon, Sep 29, 2008 at 08:34:00AM -0700, Shawn O. Pearce wrote:\n> Johannes Sixt <johannes.sixt@telecom.at> wrote:\n> \n> > My point is that emphasis on \"stat\" in the name is wrong: That's about \n> > implementation, but not about the effect. Why wouldn't 'ignoreCygwinFSTricks' \n> > be specific enough?\n> \n> I like this a lot better.  I could see us also bypassing other Cygwin\n> functions like open() in order to get faster system calls for Git.\n\nIf you think that it may be useful to bypass some other functions, and\nyou want to use the same option to control that then a general name like\nthat makes sense. Personally, I don't believe that we may want to bypass\nsomething like open() as it is not performance critical, but I said\nabove I don't care about the name much, so I am going to change my patch\nto use ignoreCygwinFSTricks.\n\nDmitry\n"},{"id":"91949","messageId":"20080930135347.GK21650@dpotapov.dyndns.org","threadId":"15689","inReplyTo":"20080929153400.GJ17584@spearce.org","subject":"[PATCH 4/4 v2] cygwin: Use native Win32 API for stat","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-09-30T13:53:47Z","receivedAt":"2008-09-30T13:53:47Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"lstat/stat functions in Cygwin are very slow, because they try to emulate\nsome *nix things that Git does not actually need. This patch adds Win32\nspecific implementation of these functions for Cygwin.\n\nThis implementation handles most situation directly but in some rare cases\nit falls back on the implementation provided for Cygwin. This is necessary\nfor two reasons:\n\n- Cygwin has its own file hierarchy, so absolute paths used in Cygwin is\n  not suitable to be used Win32 API. cygwin_conv_to_win32_path can not be\n  used because it automatically dereference Cygwin symbol links, also it\n  causes extra syscall. Fortunately Git rarely use absolute paths, so we\n  always use Cygwin implementation for absolute paths.\n\n- Support of symbol links. Cygwin stores symbol links as ordinary using\n  one of two possible formats. Therefore, the fast implementation falls\n  back to Cygwin functions if it detects potential use of symbol links.\n\nThe speed of this implementation should be the same as mingw_lstat for\ncommon cases, but it is considerable slower when the specified file name\ndoes not exist.\n\nDespite all efforts to make the fast implementation as robust as possible,\nit may not work well for some very rare situations. I am aware only one\nsituation: use Cygwin mount to bind unrelated paths inside repository\ntogether.  Therefore, the core.ignoreCygwinFSTricks configuration option is\nprovided, which controls whether native or Cygwin version of stat is used.\n\nSigned-off-by: Dmitry Potapov <dpotapov@gmail.com>\n---\n\nThis version of patch has the following correction:\n\n1. cygwinNativeStat renamed as ignoreCygwinFSTricks\n2. lines in filetime_to_timespec are reformatted to fit in 80 columns\n3. extra spaces after function names are removed\n\n\n Documentation/config.txt |    9 +++\n Makefile                 |    4 ++\n compat/cygwin.c          |  127 ++++++++++++++++++++++++++++++++++++++++++++++\n compat/cygwin.h          |    9 +++\n git-compat-util.h        |    1 +\n 5 files changed, 150 insertions(+), 0 deletions(-)\n create mode 100644 compat/cygwin.c\n create mode 100644 compat/cygwin.h\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bea867d..61437fe 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -117,6 +117,15 @@ core.fileMode::\n \tthe working copy are ignored; useful on broken filesystems like FAT.\n \tSee linkgit:git-update-index[1]. True by default.\n \n+core.ignoreCygwinFSTricks::\n+\tThis option is only used by Cygwin implementation of Git. If false,\n+\tthe Cygwin stat() and lstat() functions are used. This may be useful\n+\tif your repository consists of a few separate directories joined in\n+\tone hierarchy using Cygwin mount. If true, Git uses native Win32 API\n+\twhenever it is possible and falls back to Cygwin functions only to\n+\thandle symbol links. The native mode is more than twice faster than\n+\tnormal Cygwin l/stat() functions. True by default.\n+\n core.trustctime::\n \tIf false, the ctime differences between the index and the\n \tworking copy are ignored; useful when the inode change time\ndiff --git a/Makefile b/Makefile\nindex 3c0664a..0708390 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -347,6 +347,7 @@ LIB_H += cache.h\n LIB_H += cache-tree.h\n LIB_H += commit.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/cygwin.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -747,6 +748,9 @@ ifeq ($(uname_S),HP-UX)\n \tNO_SYS_SELECT_H = YesPlease\n \tSNPRINTF_RETURNS_BOGUS = YesPlease\n endif\n+ifneq (,$(findstring CYGWIN,$(uname_S)))\n+\tCOMPAT_OBJS += compat/cygwin.o\n+endif\n ifneq (,$(findstring MINGW,$(uname_S)))\n \tNO_MMAP = YesPlease\n \tNO_PREAD = YesPlease\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nnew file mode 100644\nindex 0000000..423ff20\n--- /dev/null\n+++ b/compat/cygwin.c\n@@ -0,0 +1,127 @@\n+#define WIN32_LEAN_AND_MEAN\n+#include \"../git-compat-util.h\"\n+#include \"win32.h\"\n+#include \"../cache.h\" /* to read configuration */\n+\n+static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)\n+{\n+\tlong long winTime = ((long long)ft->dwHighDateTime << 32) +\n+\t\t\tft->dwLowDateTime;\n+\twinTime -= 116444736000000000LL; /* Windows to Unix Epoch conversion */\n+\t/* convert 100-nsecond interval to seconds and nanoseconds */\n+\tts->tv_sec = (time_t)(winTime/10000000);\n+\tts->tv_nsec = (long)(winTime - ts->tv_sec*10000000LL) * 100;\n+}\n+\n+#define size_to_blocks(s) (((s)+511)/512)\n+\n+/* do_stat is a common implementation for cygwin_lstat and cygwin_stat.\n+ *\n+ * To simplify its logic, in the case of cygwin symlinks, this implementation\n+ * falls back to the cygwin version of stat/lstat, which is provided as the\n+ * last argument.\n+ */\n+static int do_stat(const char *file_name, struct stat *buf, stat_fn_t cygstat)\n+{\n+\tWIN32_FILE_ATTRIBUTE_DATA fdata;\n+\n+\tif (file_name[0] == '/')\n+\t\treturn cygstat (file_name, buf);\n+\n+\tif (!(errno = get_file_attr(file_name, &fdata))) {\n+\t\t/*\n+\t\t * If the system attribute is set and it is not a directory then\n+\t\t * it could be a symbol link created in the nowinsymlinks mode.\n+\t\t * Normally, Cygwin works in the winsymlinks mode, so this situation\n+\t\t * is very unlikely. For the sake of simplicity of our code, let's\n+\t\t * Cygwin to handle it.\n+\t\t */\n+\t\tif ((fdata.dwFileAttributes & FILE_ATTRIBUTE_SYSTEM) &&\n+\t\t    !(fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))\n+\t\t\treturn cygstat(file_name, buf);\n+\n+\t\t/* fill out the stat structure */\n+\t\tbuf->st_dev = buf->st_rdev = 0; /* not used by Git */\n+\t\tbuf->st_ino = 0;\n+\t\tbuf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);\n+\t\tbuf->st_nlink = 1;\n+\t\tbuf->st_uid = buf->st_gid = 0;\n+#ifdef __CYGWIN_USE_BIG_TYPES__\n+\t\tbuf->st_size = ((_off64_t)fdata.nFileSizeHigh << 32) +\n+\t\t\tfdata.nFileSizeLow;\n+#else\n+\t\tbuf->st_size = (off_t)fdata.nFileSizeLow;\n+#endif\n+\t\tbuf->st_blocks = size_to_blocks(buf->st_size);\n+\t\tfiletime_to_timespec(&fdata.ftLastAccessTime, &buf->st_atim);\n+\t\tfiletime_to_timespec(&fdata.ftLastWriteTime, &buf->st_mtim);\n+\t\tfiletime_to_timespec(&fdata.ftCreationTime, &buf->st_ctim);\n+\t\treturn 0;\n+\t} else if (errno == ENOENT) {\n+\t\t/*\n+\t\t * In the winsymlinks mode (which is the default), Cygwin\n+\t\t * emulates symbol links using Windows shortcut files. These\n+\t\t * files are formed by adding .lnk extension. So, if we have\n+\t\t * not found the specified file name, it could be that it is\n+\t\t * a symbol link. Let's Cygwin to deal with that.\n+\t\t */\n+\t\treturn cygstat(file_name, buf);\n+\t}\n+\treturn -1;\n+}\n+\n+/* We provide our own lstat/stat functions, since the provided Cygwin versions\n+ * of these functions are too slow. These stat functions are tailored for Git's\n+ * usage, and therefore they are not meant to be complete and correct emulation\n+ * of lstat/stat functionality.\n+ */\n+static int cygwin_lstat(const char *path, struct stat *buf)\n+{\n+\treturn do_stat(path, buf, lstat);\n+}\n+\n+static int cygwin_stat(const char *path, struct stat *buf)\n+{\n+\treturn do_stat(path, buf, stat);\n+}\n+\n+\n+/*\n+ * At start up, we are trying to determine whether Win32 API or cygwin stat\n+ * functions should be used. The choice is determined by core.ignorecygwinfstricks.\n+ * Reading this option is not always possible immediately as git_dir may be\n+ * not be set yet. So until it is set, use cygwin lstat/stat functions.\n+ */\n+static int native_stat = 1;\n+\n+static int git_cygwin_config(const char *var, const char *value, void *cb)\n+{\n+\tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n+\t\tnative_stat = git_config_bool(var, value);\n+\treturn 0;\n+}\n+\n+static int init_stat(void)\n+{\n+\tif (have_git_dir()) {\n+\t\tgit_config(git_cygwin_config, NULL);\n+\t\tcygwin_stat_fn = native_stat ? cygwin_stat : stat;\n+\t\tcygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+static int cygwin_stat_stub(const char *file_name, struct stat *buf)\n+{\n+\treturn (init_stat() ? stat : *cygwin_stat_fn)(file_name, buf);\n+}\n+\n+static int cygwin_lstat_stub(const char *file_name, struct stat *buf)\n+{\n+\treturn (init_stat() ? lstat : *cygwin_lstat_fn)(file_name, buf);\n+}\n+\n+stat_fn_t cygwin_stat_fn = cygwin_stat_stub;\n+stat_fn_t cygwin_lstat_fn = cygwin_lstat_stub;\n+\ndiff --git a/compat/cygwin.h b/compat/cygwin.h\nnew file mode 100644\nindex 0000000..a3229f5\n--- /dev/null\n+++ b/compat/cygwin.h\n@@ -0,0 +1,9 @@\n+#include <sys/types.h>\n+#include <sys/stat.h>\n+\n+typedef int (*stat_fn_t)(const char*, struct stat*);\n+extern stat_fn_t cygwin_stat_fn;\n+extern stat_fn_t cygwin_lstat_fn;\n+\n+#define stat(path, buf) (*cygwin_stat_fn)(path, buf)\n+#define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex db2836f..cd9752c 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -85,6 +85,7 @@\n #undef _XOPEN_SOURCE\n #include <grp.h>\n #define _XOPEN_SOURCE 600\n+#include \"compat/cygwin.h\"\n #else\n #undef _ALL_SOURCE /* AIX 5.3L defines a struct list with _ALL_SOURCE. */\n #include <grp.h>\n-- \n1.6.0\n"},{"id":"91951","messageId":"48E23E5B.7020404@griep.us","threadId":"15689","inReplyTo":"20080930135347.GK21650@dpotapov.dyndns.org","subject":"Re: [PATCH 4/4 v2] cygwin: Use native Win32 API for stat","fromName":"Marcus Griep","fromEmail":"marcus@griep.us","sentAt":"2008-09-30T14:57:31Z","receivedAt":"2008-09-30T14:57:31Z","isPatch":true,"sender":{"key":"marcus@griep.us","avatar":"https://gravatar.com/avatar/0a841f2aad3f9a38c9bcf87a567c2a1751cb08924bae1bc46ee171395bcf9794?d=mp&s=160"},"body":"Dmitry Potapov wrote:\n> lstat/stat functions in Cygwin are very slow, because they try to emulate\n> some *nix things that Git does not actually need. This patch adds Win32\n> specific implementation of these functions for Cygwin.\n\nCan't wait to see this patch in next or master!  If you recall my benchmarks\nfrom earlier, the speed-up is pretty good for cygwin users working with\nlarge repositories.\n\n> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n\nThanks for the work, Dmitry!\n\n-- \nMarcus Griep\nGPG Key ID: 0x5E968152\n——\nhttp://www.boohaunt.net\nאת.ψο´\n\n"},{"id":"91984","messageId":"20080930202625.GM21310@spearce.org","threadId":"15689","inReplyTo":"48E23E5B.7020404@griep.us","subject":"Re: [PATCH 4/4 v2] cygwin: Use native Win32 API for stat","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-09-30T20:26:25Z","receivedAt":"2008-09-30T20:26:25Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Marcus Griep <marcus@griep.us> wrote:\n> Dmitry Potapov wrote:\n> > lstat/stat functions in Cygwin are very slow, because they try to emulate\n> > some *nix things that Git does not actually need. This patch adds Win32\n> > specific implementation of these functions for Cygwin.\n> \n> Can't wait to see this patch in next or master!  If you recall my benchmarks\n> from earlier, the speed-up is pretty good for cygwin users working with\n> large repositories.\n> \n> > Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>\n> \n> Thanks for the work, Dmitry!\n\nThanks folks.  I'm scheduling this for 'next'.  Lets see how\nit goes...\n\n-- \nShawn.\n"}]}