[PATCH] Cygwin has trustable filemode

[PATCH] Cygwin has trustable filemode

[Mark Levedahl]

The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Earlier versions of Cygwin (version 1.5 and prior) had 
various methods for supporting posix file modes on different file systems, 
often using extended attributes, and this support was optional.  Such 
versions of Cygwin are not available on any public mirror and are not 
supported by the Cygwin project. The currently available Cygwin supports 
POSIX file modes without exception - this is not an optional 
configuration. The support does depend upon the underlying file system 
(neither Linux nor Cygwin can set an execute bit on a FAT file system as 
FAT has no such support), but as this is no different than Linux, the
default should not treat Cygwin differently than Linux.  

Users who desire the non-POSIX mode of operation must explicitly set 
core.filemode=False, accepting non-interoperability with Linux.  

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..779d06a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

Re: [PATCH] Cygwin has trustable filemode

[Ramsay Jones]

Mark Levedahl wrote:
> The supported Cygwin distribution on supported Windows versions provides
> complete support for POSIX filemodes, so enable this by default. git as
> distributed by the Cygwin project is configured this way.
> 
> This fixes one testsuite failure:
> t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Huh? How is it running that test? Does cygwin 1.7 somehow allow tabs in
filenames? For me, on cygwin 1.5, that test reports:

    $ ./t3300-funny-names.sh
    1..0 # SKIP Your filesystem does not allow tabs in filenames
    $

> Historical notes: Earlier versions of Cygwin (version 1.5 and prior) had 
> various methods for supporting posix file modes on different file systems, 
> often using extended attributes, and this support was optional.  Such 
> versions of Cygwin are not available on any public mirror and are not 
> supported by the Cygwin project. The currently available Cygwin supports 
> POSIX file modes without exception - this is not an optional 
> configuration. The support does depend upon the underlying file system 
> (neither Linux nor Cygwin can set an execute bit on a FAT file system as 
> FAT has no such support), but as this is no different than Linux, the
> default should not treat Cygwin differently than Linux.  

The motivation for the original patch had more to do with "windows people"
using win32 text editors which set the executable bit inappropriately.
(see commit c869753e).

Since I use cygwin tools (vim), I don't have this problem. :-D

> Users who desire the non-POSIX mode of operation must explicitly set 
> core.filemode=False, accepting non-interoperability with Linux.  
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  config.mak.uname | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/config.mak.uname b/config.mak.uname
> index 7ac541e..779d06a 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
>  	NO_THREAD_SAFE_PREAD = YesPlease
>  	NEEDS_LIBICONV = YesPlease
>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> -	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>  	# There are conflicting reports about this.
>  	# On some boxes NO_MMAP is needed, and not so elsewhere.
> 

Should you revert commit c869753e ("Force core.filemode to false
on Cygwin.", 30-12-2006) instead?

ATB,
Ramsay Jones

Re: [PATCH] Cygwin has trustable filemode

[Mark Levedahl]

On 07/16/2013 05:20 PM, Ramsay Jones wrote:
> Mark Levedahl wrote:
>> The supported Cygwin distribution on supported Windows versions provides
>> complete support for POSIX filemodes, so enable this by default. git as
>> distributed by the Cygwin project is configured this way.
>>
>> This fixes one testsuite failure:
>> t3300 test 17 (diff-index -M -p with mode change quotes funny filename)
> Huh? How is it running that test? Does cygwin 1.7 somehow allow tabs in
> filenames? For me, on cygwin 1.5, that test reports:
>
>      $ ./t3300-funny-names.sh
>      1..0 # SKIP Your filesystem does not allow tabs in filenames
>      $
Cygwin 1.7 accesses the file system in a very different way than 
1.5/earlier, so handles funny names with alacrity.
>>   
> The motivation for the original patch had more to do with "windows people"
> using win32 text editors which set the executable bit inappropriately.
> (see commit c869753e).
>
> Since I use cygwin tools (vim), I don't have this problem. :-D
This is a perfect use for the pre-commit script. I've been doing this 
for years, changing line endings and executability based upon file type. 
This could no doubt also be handled by gitattributes now as well. 
(Almost) all windows editors are perfectly happy with \n line endings, 
and none I know of care about execute permissions. I strongly believe 
that making the file line ending mode and execute status conform to 
cross-platform standards is the "right" approach rather than ignoring 
these on Windows then making others on other platforms clean up the mess 
later.

Mark

[PATCH] Cygwin has trustable filemode

[Mark Levedahl]

The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus 
dropped support for all OS variants that lack NTFS and/or the full win32 
api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs 
by default.  Cygwin 1.5 supported OS variants that used FAT as the native 
file system, and had optional methods for providing POSIX file modes on 
top of FAT12/16 and NTFS, though not FAT32.  Also, support for POSIX modes 
on top of FAT were dropped later in 1.5.  Thus, POSIX filemode support 
could not be expected by default on a Cygwin 1.5 installation, but is 
expected by default on a 1.7 installation.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
Junio - The above notes are more accurate than in my previous commit message,
so if this commit survives into next/master, I would prefer this version as
opposed to the one now on pu (da875762)

Mark

 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 174703b..bf5db47 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -164,7 +164,6 @@ ifeq ($(uname_O),Cygwin)
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

Re: [PATCH] Cygwin has trustable filemode

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Junio - The above notes are more accurate than in my previous commit message,
> so if this commit survives into next/master, I would prefer this version as
> opposed to the one now on pu (da875762)

Thanks, will replace.

What do we want to do with the compat/regex build-time switch?

IIRC, this was only needed for 1.7 and not 1.5, and I also would
expect (without anything to back-up, so this is more a faith than
expectation) over time the "new library" would have a working regex
library.

>
> Mark
>
>  config.mak.uname | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 174703b..bf5db47 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -164,7 +164,6 @@ ifeq ($(uname_O),Cygwin)
>  	NO_THREAD_SAFE_PREAD = YesPlease
>  	NEEDS_LIBICONV = YesPlease
>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> -	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
>  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>  	# There are conflicting reports about this.
>  	# On some boxes NO_MMAP is needed, and not so elsewhere.

Re: [PATCH] Cygwin has trustable filemode

[Mark Levedahl]

On 07/19/2013 12:40 PM, Junio C Hamano wrote:
> Thanks, will replace.
>
> What do we want to do with the compat/regex build-time switch?
>
> IIRC, this was only needed for 1.7 and not 1.5, and I also would
> expect (without anything to back-up, so this is more a faith than
> expectation) over time the "new library" would have a working regex
> library.
>

The situation is that Cygwin uses newlib rather than glibc, and does so 
for licesnsing reasons (redhat sells licenses to developers allowing 
closed source applications built using Cygwin). So, there must be a 
compelling need to fix the library - git has a simple work around, so 
isn't the case. Also, Cygwin has a perl regex library for those 
demanding more complete / correct regex solution. So, I make no 
prediction on when the newlib regex functions are fixed.

Related: Should we have separate settings for 1.5 and 1.7 for several 
variables? Conflicts I see not reflected in config.mak.uname on pu:
     trustable filemode   (1.7 has, 1.5 does not)
     MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP 
utility is convolved in this)
     regex - 1.7 is broken, per Ramsay 1.5 works

If you think its worth it, I'll create a patch series with the above and 
justifications for the different settings that I know.

Mark

Re: [PATCH] Cygwin has trustable filemode

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> Related: Should we have separate settings for 1.5 and 1.7 for several
> variables? Conflicts I see not reflected in config.mak.uname on pu:
>     trustable filemode   (1.7 has, 1.5 does not)
>     MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP
> utility is convolved in this)
>     regex - 1.7 is broken, per Ramsay 1.5 works
>
> If you think its worth it, I'll create a patch series with the above
> and justifications for the different settings that I know.

I'd say that would be a sensible thing to do, given that the
alternative seems to be "let's drop 1.5 support right now, because
otherwise we cannot run Git on 1.7".

Re: [PATCH] Cygwin has trustable filemode

[Mark Levedahl]

On 07/19/2013 03:16 PM, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> Related: Should we have separate settings for 1.5 and 1.7 for several
>> variables? Conflicts I see not reflected in config.mak.uname on pu:
>>      trustable filemode   (1.7 has, 1.5 does not)
>>      MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP
>> utility is convolved in this)
>>      regex - 1.7 is broken, per Ramsay 1.5 works
>>
>> If you think its worth it, I'll create a patch series with the above
>> and justifications for the different settings that I know.
> I'd say that would be a sensible thing to do, given that the
> alternative seems to be "let's drop 1.5 support right now, because
> otherwise we cannot run Git on 1.7".
>
>
Ok, the following sequence builds up options for Cygwin 1.7 while 
leaving Cygwin 1.5 as-is. This series should replace

dad577f Cygwin has trustable filemode
174bb98 Use compat/regex on Cygwin

After merging the following into current pu, all tests that run by 
default pass on Cygwin 1.7, i.e.
     prove -j 8 t[0-9]*.sh
reports "All tests successful."
I've *never* had this happen on Cygwin before.

Mark


Mark

[PATCH 1/4] Cygwin 1.7 has trustable filemode

[Mark Levedahl]

The current Cygwin 1.7 distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way. Cygwin 1.5
installations are less likely to have this support, so leave the old
default in place for those.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus
dropped support for all OS variants that lack NTFS and/or the full win32
api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs
by default.  Cygwin 1.5 supports OS variants that use FAT as the native
file system, and had optional methods for providing POSIX file modes on
top of FAT12/16 and NTFS, though not FAT32.  Also, support for POSIX modes
on top of FAT were dropped later in 1.5.  Thus, POSIX filemode support
is not expected by default on a Cygwin 1.5 installation, but is expected
by default on a 1.7 installation.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..104dc44 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -158,12 +158,12 @@ ifeq ($(uname_O),Cygwin)
 		NO_MKSTEMPS = YesPlease
 		NO_SYMLINK_HEAD = YesPlease
 		NO_IPV6 = YesPlease
+		NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 		OLD_ICONV = UnfortunatelyYes
 	endif
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

[PATCH 2/4] Cygwin 1.7 needs compat/regex

[Mark Levedahl]

Cygwin v1.7 uses the regex library from newlib which does not pass git's
tests, so don't use it. This fixes failures in t4018 and t4034.

Continue to use the platform supplied regex library for earlier versions.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 104dc44..8652da9 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -160,6 +160,8 @@ ifeq ($(uname_O),Cygwin)
 		NO_IPV6 = YesPlease
 		NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 		OLD_ICONV = UnfortunatelyYes
+	else
+		NO_REGEX = UnfortunatelyYes
 	endif
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
-- 
1.8.3.2.0.13

[PATCH 3/4] Cygwin 1.7 has thread-safe pread

[Mark Levedahl]

Per http://cygwin.com/ml/cygwin/2012-07/msg00331.html , cygwin 1.7
was modified to explicitly support git's use of pread, so make this
the default. Do not affect earlier cygwin versions.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 8652da9..048c252 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -160,10 +160,10 @@ ifeq ($(uname_O),Cygwin)
 		NO_IPV6 = YesPlease
 		NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 		OLD_ICONV = UnfortunatelyYes
+		NO_THREAD_SAFE_PREAD = YesPlease
 	else
 		NO_REGEX = UnfortunatelyYes
 	endif
-	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-- 
1.8.3.2.0.13

[PATCH 4/4] Cygwin 1.7 supports mmap

[Mark Levedahl]

git has shipped for years with MMAP enabled in the stock distribution,
there are no reports of problems / failures on the list relating to
this. Leave the default as-is on v1.5 due to lack of knowlege of this
working on earlier Cygwin.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 config.mak.uname | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 048c252..32e8332 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -161,16 +161,16 @@ ifeq ($(uname_O),Cygwin)
 		NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 		OLD_ICONV = UnfortunatelyYes
 		NO_THREAD_SAFE_PREAD = YesPlease
+		# There are conflicting reports about this.
+		# On some boxes NO_MMAP is needed, and not so elsewhere.
+		# Try commenting this out if you suspect MMAP is more efficient
+		NO_MMAP = YesPlease
 	else
 		NO_REGEX = UnfortunatelyYes
 	endif
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-	# There are conflicting reports about this.
-	# On some boxes NO_MMAP is needed, and not so elsewhere.
-	# Try commenting this out if you suspect MMAP is more efficient
-	NO_MMAP = YesPlease
 	X = .exe
 	COMPAT_OBJS += compat/cygwin.o
 	UNRELIABLE_FSTAT = UnfortunatelyYes
-- 
1.8.3.2.0.13

Re: [PATCH] Cygwin has trustable filemode

[Jonathan Nieder]

Mark Levedahl wrote:

> After merging the following into current pu, all tests that run by
> default pass on Cygwin 1.7, i.e.
>     prove -j 8 t[0-9]*.sh
> reports "All tests successful."
> I've *never* had this happen on Cygwin before.

Nice.  Thanks for your hard work.

Re: [PATCH] Cygwin has trustable filemode

[Ramsay Jones]

Mark Levedahl wrote:
> On 07/19/2013 12:40 PM, Junio C Hamano wrote:
>> Thanks, will replace.
>>
>> What do we want to do with the compat/regex build-time switch?
>>
>> IIRC, this was only needed for 1.7 and not 1.5, and I also would
>> expect (without anything to back-up, so this is more a faith than
>> expectation) over time the "new library" would have a working regex
>> library.
>>
> 
> The situation is that Cygwin uses newlib rather than glibc, and does so 
> for licesnsing reasons (redhat sells licenses to developers allowing 
> closed source applications built using Cygwin). So, there must be a 
> compelling need to fix the library - git has a simple work around, so 
> isn't the case. Also, Cygwin has a perl regex library for those 
> demanding more complete / correct regex solution. So, I make no 
> prediction on when the newlib regex functions are fixed.
> 
> Related: Should we have separate settings for 1.5 and 1.7 for several 
> variables?

We already do.

>               Conflicts I see not reflected in config.mak.uname on pu:
>      trustable filemode   (1.7 has, 1.5 does not)

I see no need for any difference here. puzzled.

>      MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP 
> utility is convolved in this)

pread() is now thread-safe? great! (It must have been a fairly recent
change; last time I looked it was still not thread-safe on 1.7.)

>      regex - 1.7 is broken, per Ramsay 1.5 works

I don't see any reason not to use the compat/regex routines on both
cygwin 1.5 and 1.7.  However, I wouldn't object to restricting the use
of the compat routines to cygwin 1.7 either!

> If you think its worth it, I'll create a patch series with the above and 
> justifications for the different settings that I know.

As far as I can see, only the pread() and maybe MMAP and regex setting
need to change from the current setup.

ATB,
Ramsay Jones

Re: [PATCH] Cygwin has trustable filemode

[Ramsay Jones]

Mark Levedahl wrote:
> The supported Cygwin distribution on supported Windows versions provides
> complete support for POSIX filemodes, so enable this by default. git as
> distributed by the Cygwin project is configured this way.
> 
> This fixes one testsuite failure:
> t3300 test 17 (diff-index -M -p with mode change quotes funny filename)
> 
> Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus 
> dropped support for all OS variants that lack NTFS and/or the full win32 
> api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs 
> by default.  Cygwin 1.5 supported OS variants that used FAT as the native 
> file system, and had optional methods for providing POSIX file modes on 
> top of FAT12/16 and NTFS, though not FAT32.  Also, support for POSIX modes 
> on top of FAT were dropped later in 1.5.  Thus, POSIX filemode support 
> could not be expected by default on a Cygwin 1.5 installation, but is 
> expected by default on a 1.7 installation.
> 
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
> Junio - The above notes are more accurate than in my previous commit message,
> so if this commit survives into next/master, I would prefer this version as
> opposed to the one now on pu (da875762)

Again, I have to ask; should you not "revert" commit c869753e ("Force core.filemode
to false on Cygwin.", 30-12-2006)?  After this commit, there is no longer any user
of the NO_TRUSTABLE_FILEMODE build variable, and no real prospect of anyone else
wanting to use it.

ATB,
Ramsay Jones

Re: [PATCH] Cygwin has trustable filemode

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Mark Levedahl wrote:
>> The supported Cygwin distribution on supported Windows versions provides
>> complete support for POSIX filemodes, so enable this by default.
>> ...
>> Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus 
>> dropped support for all OS variants that lack NTFS and/or ...
>> ...  Thus, POSIX filemode support 
>> could not be expected by default on a Cygwin 1.5 installation, but is 
>> expected by default on a 1.7 installation.
>> 
> Again, I have to ask; should you not "revert" commit c869753e ("Force core.filemode
> to false on Cygwin.", 30-12-2006)?  After this commit, there is no longer any user
> of the NO_TRUSTABLE_FILEMODE build variable, and no real prospect of anyone else
> wanting to use it.

Thanks for raising this point.

Reading c869753e once again:

    The issue is that Cygwin and NTFS correctly supports the
    executable mode bit, and Git properly detected that, but most
    native Windows applications tend to create files such that
    Cygwin sees the executable bit set when it probably shouldn't
    be.

In other words, the reason why "NO_TRUSTABLE_FILEMODE" was added was
not because the Cygwin did not give us reliable filemodes.  It was
because tools outside the control of Git and/or Cygwin that users
use tend to misbehave, even when the working tree is on a filesystem
on which Cygwin can give us trustable filemodes.

So "1.7 always supports core.filemodes correctly because it no
longer works on filesystems without trustable filemodes" is not a
valid reason to justify Mark's change.

There are only three possible ways going forward, I think:

 (A) Drop Mark's patch, and do nothing else, because breakages of
     other people's programs are not fixed by Cygwin 1.7's improved
     filesystem support, and users still use those mode breaking
     programs written by others;

 (B) Drop Mark's patch, and revert c869753e, because it is not the
     business of our project to sweep breakages of other people's
     tools under the rug by crippling our software; or

 (C) Drop NO_TRUSTABLE_FILEMODE for _all_ versions of Cygwin,
     declaring that the spirit of c869753e to work around bugs in
     other people's software by crippling Git is justified, but that
     it is no longer necessary on Cygwin because people do not use
     such misbehaving third-party tools anymore.

These three each rely on its own precondition; I suspect it is
likely that (A)'s is the most accurate description of the real world.

Re: [PATCH] Cygwin has trustable filemode

[Mark Levedahl]

On 07/21/2013 05:56 PM, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> Mark Levedahl wrote:
>>> The supported Cygwin distribution on supported Windows versions provides
>>> complete support for POSIX filemodes, so enable this by default.
>>> ...
>>> Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus
>>> dropped support for all OS variants that lack NTFS and/or ...
>>> ...  Thus, POSIX filemode support
>>> could not be expected by default on a Cygwin 1.5 installation, but is
>>> expected by default on a 1.7 installation.
>>>
>> Again, I have to ask; should you not "revert" commit c869753e ("Force core.filemode
>> to false on Cygwin.", 30-12-2006)?  After this commit, there is no longer any user
>> of the NO_TRUSTABLE_FILEMODE build variable, and no real prospect of anyone else
>> wanting to use it.
> Thanks for raising this point.
>
> Reading c869753e once again:
>
>      The issue is that Cygwin and NTFS correctly supports the
>      executable mode bit, and Git properly detected that, but most
>      native Windows applications tend to create files such that
>      Cygwin sees the executable bit set when it probably shouldn't
>      be.
>
> In other words, the reason why "NO_TRUSTABLE_FILEMODE" was added was
> not because the Cygwin did not give us reliable filemodes.  It was
> because tools outside the control of Git and/or Cygwin that users
> use tend to misbehave, even when the working tree is on a filesystem
> on which Cygwin can give us trustable filemodes.
>
> So "1.7 always supports core.filemodes correctly because it no
> longer works on filesystems without trustable filemodes" is not a
> valid reason to justify Mark's change.
>
> There are only three possible ways going forward, I think:
>
>   (A) Drop Mark's patch, and do nothing else, because breakages of
>       other people's programs are not fixed by Cygwin 1.7's improved
>       filesystem support, and users still use those mode breaking
>       programs written by others;
>
>   (B) Drop Mark's patch, and revert c869753e, because it is not the
>       business of our project to sweep breakages of other people's
>       tools under the rug by crippling our software; or
>
>   (C) Drop NO_TRUSTABLE_FILEMODE for _all_ versions of Cygwin,
>       declaring that the spirit of c869753e to work around bugs in
>       other people's software by crippling Git is justified, but that
>       it is no longer necessary on Cygwin because people do not use
>       such misbehaving third-party tools anymore.
>
> These three each rely on its own precondition; I suspect it is
> likely that (A)'s is the most accurate description of the real world.
>

Perhaps the simplest approach is to just defer to the judgement of the 
Cygwin project maintainers here.

a) The Cygwin project has its stated objective of being as matching Linux.
b) The Cygwin project has always shipped git binaries built without 
NO_TRUSTABLE_FILEMODE

Also - users who do not want Cygwin's assumptions / environment are now 
free to use the msysgit version and frankly they should be so encouraged 
- it is faster than Cygwin's git. This option was not available in 2006.

Mark

Re: [PATCH] Cygwin has trustable filemode

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> On 07/21/2013 05:56 PM, Junio C Hamano wrote:
> ...
>> There are only three possible ways going forward, I think:
>>
>>   (A) Drop Mark's patch, and do nothing else, because breakages of
>>       other people's programs are not fixed by Cygwin 1.7's improved
>>       filesystem support, and users still use those mode breaking
>>       programs written by others;
>>
>>   (B) Drop Mark's patch, and revert c869753e, because it is not the
>>       business of our project to sweep breakages of other people's
>>       tools under the rug by crippling our software; or
>>
>>   (C) Drop NO_TRUSTABLE_FILEMODE for _all_ versions of Cygwin,
>>       declaring that the spirit of c869753e to work around bugs in
>>       other people's software by crippling Git is justified, but that
>>       it is no longer necessary on Cygwin because people do not use
>>       such misbehaving third-party tools anymore.
>>
>> These three each rely on its own precondition; I suspect it is
>> likely that (A)'s is the most accurate description of the real world.
>
> Perhaps the simplest approach is to just defer to the judgement of the
> Cygwin project maintainers here.
>
> a) The Cygwin project has its stated objective of being as matching Linux.

That does not say much.  On Linux, third-party "native Windows
applications" that necessitated c869753e (Force core.filemode to
false on Cygwin., 2006-12-30) to help users is not an issue.  On
Cygwin, it still is.

> b) The Cygwin project has always shipped git binaries built without
> NO_TRUSTABLE_FILEMODE

That is a fair point.  So let's do this instead.

-- >8 --
From: Mark Levedahl <mlevedahl@gmail.com>
Subject: [PATCH] cygwin: stop forcing core.filemode=false

We force core.filemode=false since c869753e (Force core.filemode to
false on Cygwin., 2006-12-30), even when the repository is on a
filesystem on which Cygwin can give us trustable filemodes, because
many native Windows applications the users use to edit files in the
working tree tend to (re)create files with executable bit randomly
set or reset.  However, binary distribution of Git that is supplied
by the downstream project to its users has been built without this
consideration.

Drop NO_TRUSTABLE_FILEMODE from our default configuration so that
hand-compiled Git out of box will match theirs.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..779d06a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
 	NO_THREAD_SAFE_PREAD = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.3-1030-g100bb15

Re: [PATCH] Cygwin has trustable filemode

[Mark Levedahl]

On 07/22/2013 01:02 AM, Junio C Hamano wrote:
>> b) The Cygwin project has always shipped git binaries built without
>> NO_TRUSTABLE_FILEMODE
> That is a fair point.  So let's do this instead.
>
> -- >8 --
> From: Mark Levedahl <mlevedahl@gmail.com>
> Subject: [PATCH] cygwin: stop forcing core.filemode=false
>
> We force core.filemode=false since c869753e (Force core.filemode to
> false on Cygwin., 2006-12-30), even when the repository is on a
> filesystem on which Cygwin can give us trustable filemodes, because
> many native Windows applications the users use to edit files in the
> working tree tend to (re)create files with executable bit randomly
> set or reset.  However, binary distribution of Git that is supplied
> by the downstream project to its users has been built without this
> consideration.
>
> Drop NO_TRUSTABLE_FILEMODE from our default configuration so that
> hand-compiled Git out of box will match theirs.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   config.mak.uname | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 7ac541e..779d06a 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
>   	NO_THREAD_SAFE_PREAD = YesPlease
>   	NEEDS_LIBICONV = YesPlease
>   	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> -	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
>   	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
>   	# There are conflicting reports about this.
>   	# On some boxes NO_MMAP is needed, and not so elsewhere.
ok by me.

Mark