{"thread":{"id":"15863","subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","startedAt":"2008-10-11T18:54:40Z","lastAt":"2008-10-13T04:33:31Z","messageCount":11,"participants":["Mark Levedahl","Junio C Hamano","Dmitry Potapov"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"92805","messageId":"1223751280-2104-1-git-send-email-mlevedahl@gmail.com","threadId":"15863","inReplyTo":null,"subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-11T18:54:40Z","receivedAt":"2008-10-11T18:54:40Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Cygwin's POSIX emulation allows use of core.filemode true, unlike native\nWindow's implementation of stat / lstat, and Cygwin/git users who have\nconfigured core.filemode true in various repositories will be very\nunpleasantly surprised to find that git is no longer honoring that option.\nSo, this patch fores use of Cygwin's stat functions if core.filemode is\nset true, regardless of any other considerations.\n\nSigned-off-by: Mark Levedahl <mlevedahl@gmail.com>\n---\n Documentation/config.txt |    4 +++-\n compat/cygwin.c          |   10 +++++++++-\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7161597..a3a9495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::\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+\tnormal Cygwin l/stat() functions. True by default, unless core.filemode\n+\tis true, in which case ignoreCygwinFSTricks is ignored as Cygwin's\n+\tPOSIX emulation is required to support core.filemode.\n \n core.trustctime::\n \tIf false, the ctime differences between the index and the\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 423ff20..54028b3 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -91,13 +91,21 @@ static int cygwin_stat(const char *path, struct stat *buf)\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+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as\n+ * the Windows stat fuctions do not determine posix filemode.\n  */\n static int native_stat = 1;\n+static int core_filemode = 0;\n \n static int git_cygwin_config(const char *var, const char *value, void *cb)\n {\n+\tif (!strcmp(var, \"core.filemode\")) {\n+\t\t\tcore_filemode = git_config_bool(var, value);\n+\t\t\tnative_stat &= !core_filemode;\n+\t}\n \tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n-\t\tnative_stat = git_config_bool(var, value);\n+\t\tnative_stat = git_config_bool(var, value) &&\n+\t\t\t!core_filemode;\n \treturn 0;\n }\n \n-- \n1.6.0.2.535.g47d45.dirty\n"},{"id":"92806","messageId":"1223751859-3540-1-git-send-email-mlevedahl@gmail.com","threadId":"15863","inReplyTo":"1223751280-2104-1-git-send-email-mlevedahl@gmail.com","subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-11T19:04:19Z","receivedAt":"2008-10-11T19:04:19Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Cygwin's POSIX emulation allows use of core.filemode true, unlike native\nWindow's implementation of stat / lstat, and Cygwin/git users who have\nconfigured core.filemode true in various repositories will be very\nunpleasantly surprised to find that git is no longer honoring that option.\nSo, this patch fores use of Cygwin's stat functions if core.filemode is\nset true, regardless of any other considerations.\n\nSigned-off-by: Mark Levedahl <mlevedahl@gmail.com>\n---\nResend as I mangled Shawn's email address.\n\n Documentation/config.txt |    4 +++-\n compat/cygwin.c          |   10 +++++++++-\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7161597..a3a9495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::\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+\tnormal Cygwin l/stat() functions. True by default, unless core.filemode\n+\tis true, in which case ignoreCygwinFSTricks is ignored as Cygwin's\n+\tPOSIX emulation is required to support core.filemode.\n \n core.trustctime::\n \tIf false, the ctime differences between the index and the\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 423ff20..54028b3 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -91,13 +91,21 @@ static int cygwin_stat(const char *path, struct stat *buf)\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+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as\n+ * the Windows stat fuctions do not determine posix filemode.\n  */\n static int native_stat = 1;\n+static int core_filemode = 0;\n \n static int git_cygwin_config(const char *var, const char *value, void *cb)\n {\n+\tif (!strcmp(var, \"core.filemode\")) {\n+\t\t\tcore_filemode = git_config_bool(var, value);\n+\t\t\tnative_stat &= !core_filemode;\n+\t}\n \tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n-\t\tnative_stat = git_config_bool(var, value);\n+\t\tnative_stat = git_config_bool(var, value) &&\n+\t\t\t!core_filemode;\n \treturn 0;\n }\n \n-- \n1.6.0.2.535.g47d45.dirty\n"},{"id":"92810","messageId":"7v1vymke85.fsf@gitster.siamese.dyndns.org","threadId":"15863","inReplyTo":"1223751859-3540-1-git-send-email-mlevedahl@gmail.com","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-11T21:34:50Z","receivedAt":"2008-10-11T21:34:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> Cygwin's POSIX emulation allows use of core.filemode true, unlike native\n> Window's implementation of stat / lstat, and Cygwin/git users who have\n> configured core.filemode true in various repositories will be very\n> unpleasantly surprised to find that git is no longer honoring that option.\n> So, this patch fores use of Cygwin's stat functions if core.filemode is\n\ns/fores/forces/;\n\n>  static int native_stat = 1;\n> +static int core_filemode = 0;\n\nMakes me wonder why \"trust_executable_bit\" is unavailable here.\nPerhaps git_cygwin_config() does not fall back to git_default_config()\nfor a reason?\n\n>  static int git_cygwin_config(const char *var, const char *value, void *cb)\n>  {\n> +\tif (!strcmp(var, \"core.filemode\")) {\n> +\t\t\tcore_filemode = git_config_bool(var, value);\n> +\t\t\tnative_stat &= !core_filemode;\n> +\t}\n>  \tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n> -\t\tnative_stat = git_config_bool(var, value);\n> +\t\tnative_stat = git_config_bool(var, value) &&\n> +\t\t\t!core_filemode;\n>  \treturn 0;\n>  }\n\nIf you can safely determine if you would want to use cygwin_stat or not\nonly after you have read both core.filemode and core.ignorecygwinfstricks,\nperhaps keeping the config reader as is (this includes not falling back to\ngit_default_config()) and instead doing:\n\n\tstatic int init_stat(void)\n\t{\n\t\t...\n\t\tgit_config(git_sygwin_config, NULL);\n\t\tif (!core_filemode && native_stat) {\n\t\t\tcygwin_stat_fn = cygwin_stat;\n\t\t\tcygwin_lstat_fn = cygwin_lstat;\n\t\t} else {\n\t\t\tcygwin_stat_fn = stat;\n\t\t\tcygwin_lstat_fn = lstat;\n\t\t}\n\t\t...\n\nis less yucky?\n"},{"id":"92815","messageId":"1223765806-3332-1-git-send-email-mlevedahl@gmail.com","threadId":"15863","inReplyTo":"7v1vymke85.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-11T22:56:46Z","receivedAt":"2008-10-11T22:56:46Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Cygwin's POSIX emulation allows use of core.filemode true, unlike native\nWindow's implementation of stat / lstat, and Cygwin/git users who have\nconfigured core.filemode true in various repositories will be very\nunpleasantly surprised to find that git is no longer honoring that option.\nSo, this patch forces use of Cygwin's stat functions if core.filemode is\nset true, regardless of any other considerations.\n\nSigned-off-by: Mark Levedahl <mlevedahl@gmail.com>\n---\n Documentation/config.txt |    4 +++-\n compat/cygwin.c          |   18 +++++++++++++++---\n 2 files changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7161597..a3a9495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::\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+\tnormal Cygwin l/stat() functions. True by default, unless core.filemode\n+\tis true, in which case ignoreCygwinFSTricks is ignored as Cygwin's\n+\tPOSIX emulation is required to support core.filemode.\n \n core.trustctime::\n \tIf false, the ctime differences between the index and the\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 423ff20..1fed265 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -91,13 +91,20 @@ static int cygwin_stat(const char *path, struct stat *buf)\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+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as\n+ * the Windows stat fuctions do not determine posix filemode.\n  */\n static int native_stat = 1;\n+static int core_filemode = 0;\n \n static int git_cygwin_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n+\tif (!strcmp(var, \"core.filemode\"))\n+\t\tcore_filemode = git_config_bool(var, value);\n+\n+\telse if (!strcmp(var, \"core.ignorecygwinfstricks\"))\n \t\tnative_stat = git_config_bool(var, value);\n+\n \treturn 0;\n }\n \n@@ -105,8 +112,13 @@ 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\tif (!core_filemode && native_stat) {\n+\t\t\tcygwin_stat_fn = cygwin_stat;\n+\t\t\tcygwin_lstat_fn = cygwin_lstat;\n+\t\t} else {\n+\t\t\tcygwin_stat_fn = stat;\n+\t\t\tcygwin_lstat_fn = lstat;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn 1;\n-- \n1.6.0.2.536.ga36e\n"},{"id":"92838","messageId":"20081012133934.GB21650@dpotapov.dyndns.org","threadId":"15863","inReplyTo":"1223765806-3332-1-git-send-email-mlevedahl@gmail.com","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2008-10-12T13:39:34Z","receivedAt":"2008-10-12T13:39:34Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sat, Oct 11, 2008 at 06:56:46PM -0400, Mark Levedahl wrote:\n> diff --git a/compat/cygwin.c b/compat/cygwin.c\n> index 423ff20..1fed265 100644\n> --- a/compat/cygwin.c\n> +++ b/compat/cygwin.c\n> @@ -91,13 +91,20 @@ static int cygwin_stat(const char *path, struct stat *buf)\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> + * However, if core.filemode is true, we *must* use the Cygwin posix stat as\n> + * the Windows stat fuctions do not determine posix filemode.\n>   */\n>  static int native_stat = 1;\n> +static int core_filemode = 0;\n\nIs it a reason to add a new variable instead of using trust_executable_bit?\nAnd if so, why its default value is different? I know that this default does\nnot matter usually, because core.filemode is always stored in .git/config,\nbut still I don't like this discrapency.\n\nOtherwise, I don't have any objection to this patch.\n\nDmitry\n"},{"id":"92865","messageId":"1223837086-2864-1-git-send-email-mlevedahl@gmail.com","threadId":"15863","inReplyTo":"20081012133934.GB21650@dpotapov.dyndns.org","subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-12T18:44:46Z","receivedAt":"2008-10-12T18:44:46Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Cygwin's POSIX emulation allows use of core.filemode true, unlike native\nWindow's implementation of stat / lstat, and Cygwin/git users who have\nconfigured core.filemode true in various repositories will be very\nunpleasantly surprised to find that git is no longer honoring that option.\nSo, this patch forces use of Cygwin's stat functions if core.filemode is\nset true, regardless of any other considerations.\n\nSigned-off-by: Mark Levedahl <mlevedahl@gmail.com>\n---\nUpdated to use trust_executable_bit as suggested on the list.\n\n Documentation/config.txt |    4 +++-\n compat/cygwin.c          |   16 +++++++++++++---\n 2 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7161597..a3a9495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::\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+\tnormal Cygwin l/stat() functions. True by default, unless core.filemode\n+\tis true, in which case ignoreCygwinFSTricks is ignored as Cygwin's\n+\tPOSIX emulation is required to support core.filemode.\n\n core.trustctime::\n \tIf false, the ctime differences between the index and the\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 423ff20..221d2f5 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -91,12 +91,17 @@ static int cygwin_stat(const char *path, struct stat *buf)\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+ * However, if core.filemode is true, we *must* use the Cygwin posix stat as\n+ * the Windows stat fuctions do not determine posix filemode.\n  */\n static int native_stat = 1;\n+extern int trust_executable_bit;\n\n static int git_cygwin_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"core.ignorecygwinfstricks\"))\n+\tif (!strcmp(var, \"core.filemode\"))\n+\t\ttrust_executable_bit = git_config_bool(var, value);\n+\telse if (!strcmp(var, \"core.ignorecygwinfstricks\"))\n \t\tnative_stat = git_config_bool(var, value);\n \treturn 0;\n }\n@@ -105,8 +110,13 @@ 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\tif (!trust_executable_bit && native_stat) {\n+\t\t\tcygwin_stat_fn = cygwin_stat;\n+\t\t\tcygwin_lstat_fn = cygwin_lstat;\n+\t\t} else {\n+\t\t\tcygwin_stat_fn = stat;\n+\t\t\tcygwin_lstat_fn = lstat;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn 1;\n--\n1.6.0.2.535.g81f8.dirty\n"},{"id":"92868","messageId":"7vskr1fvys.fsf@gitster.siamese.dyndns.org","threadId":"15863","inReplyTo":"1223837086-2864-1-git-send-email-mlevedahl@gmail.com","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-12T19:35:07Z","receivedAt":"2008-10-12T19:35:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> Cygwin's POSIX emulation allows use of core.filemode true, unlike native\n> Window's implementation of stat / lstat, and Cygwin/git users who have\n> configured core.filemode true in various repositories will be very\n> unpleasantly surprised to find that git is no longer honoring that option.\n> So, this patch forces use of Cygwin's stat functions if core.filemode is\n> set true, regardless of any other considerations.\n>\n> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>\n> ---\n> Updated to use trust_executable_bit as suggested on the list.\n\nBefore you do that, can you explain why git_cygwin_config() should\nduplicate the setting of that variable instead of using the existing\nparser for that variable by calling git_default_config()?\n"},{"id":"92870","messageId":"48F25CB6.10702@gmail.com","threadId":"15863","inReplyTo":"7vskr1fvys.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-12T20:23:18Z","receivedAt":"2008-10-12T20:23:18Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> Before you do that, can you explain why git_cygwin_config() should\n> duplicate the setting of that variable instead of using the existing\n> parser for that variable by calling git_default_config()?\n>\n>   \n\nI don't like traversing the same list of configuration variables / files \ntwice, each time looking for a different variable just so that one of \nthose traversals can use a standard function, rather than just \ntraversing once and getting exactly what this routine needs; this seems \nwasteful to me. However, I am (obviously) not the maintainer of this \ncode so I will do this however it is desired.\n\nMark\n"},{"id":"92885","messageId":"7vd4i5fkny.fsf@gitster.siamese.dyndns.org","threadId":"15863","inReplyTo":"48F25CB6.10702@gmail.com","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-10-12T23:39:13Z","receivedAt":"2008-10-12T23:39:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl <mlevedahl@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>\n>> Before you do that, can you explain why git_cygwin_config() should\n>> duplicate the setting of that variable instead of using the existing\n>> parser for that variable by calling git_default_config()?\n>\n> I don't like traversing the same list of configuration variables /\n> files twice, each time looking for a different variable just so that\n> one of those traversals can use a standard function, rather than just\n> traversing once and getting exactly what this routine needs; this\n> seems wasteful to me. However, I am (obviously) not the maintainer of\n> this code so I will do this however it is desired.\n\nI am not sure what you meant by \"traversing twice\"; when the first call to\nstat or lstat is made, you call init_stat() and read the config file,\nfeeding each and every variable definition to the callback functions.  And\nthen whenever the command implementations call git_config() to getting the\nconfiguration necessary for their own use, they read the config, again\nfeeding each and every variable definition to the callback functions.\nThis is \"traversing twice\" which is unfortunate, but that is not something\nyou can control from this codepath.\n\nBut in any case, I was worried about something different.\n\nIf you call git_default_config() as a fallback from git_cygwin_config(),\nwhich is what all the other standard configuration callback functions do,\nand/or set trust_executable_bit, command implementations that do not call\ngit_default_config() on their own would see different trust_executable_bit\nvalue when running on non-Cygwin platform and on Cygwin.\n\nI was wondering if git_cygwin_config() was originally written not to call\ngit_default_config() because some command implementations do not to want\nto call git_default_core_config() (and/or read trust_executable_bit\nvariable from the configuration) for some reason (which would be just\nhiding bugs in other parts of the system, I suspect).\n\nIf that is the case, we would have to fix such broken parts of the system,\nbut until that happens your original patch to use a separate variable and\nkeeping trust_executable_bit untouched would be much safer than this\nlatest patch.  Hence the question.\n"},{"id":"92900","messageId":"48F2CF39.5040000@gmail.com","threadId":"15863","inReplyTo":"7vd4i5fkny.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-13T04:31:53Z","receivedAt":"2008-10-13T04:31:53Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n> I was wondering if git_cygwin_config() was originally written not to call\n> git_default_config() because some command implementations do not to want\n> to call git_default_core_config() (and/or read trust_executable_bit\n> variable from the configuration) for some reason (which would be just\n> hiding bugs in other parts of the system, I suspect).\n>\n> If that is the case, we would have to fix such broken parts of the system,\n> but until that happens your original patch to use a separate variable and\n> keeping trust_executable_bit untouched would be much safer than this\n> latest patch.  Hence the question.\n>   \nI was worried about altering the startup code, which is why I tried to \nintroduce as little change as possible. However, having rewritten the \npatch using git_default_config() everything seems fine (the testsuite \npasses, or at least as well as it usually does under Cygwin, and I \nassume that is sufficient as this is Cygiwn specific). So, that patch \nfollows.\n\nMark\n"},{"id":"92901","messageId":"1223872411-3328-1-git-send-email-mlevedahl@gmail.com","threadId":"15863","inReplyTo":"7vd4i5fkny.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] compat/cygwin.c - Use cygwin's stat if core.filemode == true","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2008-10-13T04:33:31Z","receivedAt":"2008-10-13T04:33:31Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Cygwin's POSIX emulation allows use of core.filemode true, unlike native\nWindow's implementation of stat / lstat, and Cygwin/git users who have\nconfigured core.filemode true in various repositories will be very\nunpleasantly surprised to find that git is no longer honoring that option.\nSo, this patch forces use of Cygwin's stat functions if core.filemode is\nset true, regardless of any other considerations.\n\nSigned-off-by: Mark Levedahl <mlevedahl@gmail.com>\n---\n Documentation/config.txt |    4 +++-\n compat/cygwin.c          |   20 +++++++++++++++-----\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7161597..a3a9495 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -124,7 +124,9 @@ core.ignoreCygwinFSTricks::\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+\tnormal Cygwin l/stat() functions. True by default, unless core.filemode\n+\tis true, in which case ignoreCygwinFSTricks is ignored as Cygwin's\n+\tPOSIX emulation is required to support core.filemode.\n \n core.trustctime::\n \tIf false, the ctime differences between the index and the\ndiff --git a/compat/cygwin.c b/compat/cygwin.c\nindex 423ff20..780375e 100644\n--- a/compat/cygwin.c\n+++ b/compat/cygwin.c\n@@ -91,22 +91,32 @@ static int cygwin_stat(const char *path, struct stat *buf)\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+ * However, if the trust_executable_bit is set, we must use the Cygwin posix\n+ * stat/lstat as the Windows stat fuctions do not determine posix filemode.\n  */\n static int native_stat = 1;\n+extern int trust_executable_bit;\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+\tif (!strcmp(var, \"core.ignorecygwinfstricks\")) {\n+\t\t\tnative_stat = git_config_bool(var, value);\n+\t\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value, cb);\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\tif (!trust_executable_bit && native_stat) {\n+\t\t\tcygwin_stat_fn = cygwin_stat;\n+\t\t\tcygwin_lstat_fn = cygwin_lstat;\n+\t\t} else {\n+\t\t\tcygwin_stat_fn = stat;\n+\t\t\tcygwin_lstat_fn = lstat;\n+\t\t}\n \t\treturn 0;\n \t}\n \treturn 1;\n-- \n1.6.0.2.535.ga11fa.dirty\n"}]}