git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
@ 2013-03-23 12:40 Torsten Bögershausen
  2013-03-24  2:49 ` Eric Sunshine
  0 siblings, 1 reply; 12+ messages in thread
From: Torsten Bögershausen @ 2013-03-23 12:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: j6t, git, ramsay, tboegi

On 19.03.13 22:03, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> Use a compile switch IGNORECYGWINFSTRICKS to disable the usage
>> of cygwin_lstat_fn() only in path.c
> 
> The analysis of the problem and the basic idea to disable the
> fast-but-lying fstricks in the code that matters may be good, but
> the implementation is questionable.
> 
> What if we later need to move functions around, etc., so that some
> other calls in path.c still do want to use the fstricks bit while
> the existing ones in the file want the real lstat() information?
> 
> Actually, that already is the case.  The call to lstat() in
> validate_headref() only cares about the S_ISXXX() type and can
> afford to use the fast-and-lying one, no?
> 
> How about doing something like this in the generic codepath, and
> implement your own cygwin_true_mode_bits() function at the cygwin
> compatibility layer, and add
> 
>     #define true_mode_bits cygwin_true_mode_bits
> 
> in the compat/cygwin.h file?  The change has the documentation value
> to clarify what each lstat() is used for, too.
> 
> Ideally, the "#ifndef true_mode_bits" part may want to become a
> generic helper function if there are lstat() calls in other files
> that cygwin wants to use the real lstat() not the fast-but-lying
> one, but one step at a time.
> 
> Hrm?
> 
>  path.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/path.c b/path.c
> index d3d3f8b..d0b31e5 100644
> --- a/path.c
> +++ b/path.c
> @@ -14,6 +14,21 @@
>  #include "strbuf.h"
>  #include "string-list.h"
>  
> +#ifndef true_mode_bits
> +/*
> + * The replacement lstat(2) we use on Cygwin is incomplete and
> + * lies about permission bits; most of the time we do not care,
> + * but the callsites of this wrpper do care.
> + */
> +static int true_mode_bits(const char *path, int *mode)
> +{
> +	struct stat st;
> +	if (lstat(path, &st) < 0)
> +		return -1;
> +	return st.st_mode;
> +}
> +#endif
> +
>  static char bad_path[] = "/bad-path/";
>  
>  static char *get_pathname(void)
> @@ -400,9 +415,8 @@ int set_shared_perm(const char *path, int mode)
>  		return 0;
>  	}
>  	if (!mode) {
> -		if (lstat(path, &st) < 0)
> +		if (true_mode_bits(path, &mode) < 0)
>  			return -1;
> -		mode = st.st_mode;
>  		orig_mode = mode;
>  	} else
>  		orig_mode = 0;
> 
Thanks for review, I Changed the function name into get_st_mode_bits(),
but that is only a suggestion.

-- >8 --
Subject: [PATCH] Make core.sharedRepository work under cygwin 1.7

When core.sharedRepository is used, set_shared_perm() in path.c
needs lstat() to return the correct POSIX permissions.

The default for cygwin is core.ignoreCygwinFSTricks = false, which
means that the fast implementation in do_stat() is used instead of lstat().

lstat() under cygwin uses the Windows security model to implement
POSIX-like permissions.
The user, group or everyone bits can be set individually.

do_stat() simplifes the file permission bits, and may return a wrong value:
The read-only attribute of a file is used to calculate
the permissions, resulting in either rw-r--r-- or r--r--r--

One effect of the simplified do_stat() is that t1301 failes.

Add a function cygwin_get_st_mode_bits() which returns the POSIX permissions.
When not compiling for cygwin, true_mode_bits() in path.c is used.

Side note:
t1301 passes under cygwin 1.5.
The "user write" bit is synchronized with the "read only" attribute of a file:

$ chmod 444 x
$ attrib x
A    R     C:\temp\pt\x

cygwin 1.7 would show
A          C:\temp\pt\x

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 compat/cygwin.c   | 13 +++++++++++++
 compat/cygwin.h   |  5 +++++
 git-compat-util.h |  1 +
 path.c            | 20 +++++++++++++++++---
 4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/compat/cygwin.c b/compat/cygwin.c
index 5428858..871b41d 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,3 +1,4 @@
+#define CYGWIN_C
 #define WIN32_LEAN_AND_MEAN
 #ifdef CYGWIN_V15_WIN32API
 #include "../git-compat-util.h"
@@ -10,6 +11,18 @@
 #endif
 #include "../cache.h" /* to read configuration */
 
+/*
+ * Return POSIX permission bits, regardless of core.ignorecygwinfstricks
+ */
+int cygwin_get_st_mode_bits(const char *path, int *mode)
+{
+	struct stat st;
+	if (lstat(path, &st) < 0)
+		return -1;
+	*mode = st.st_mode;
+	return 0;
+}
+
 static inline void filetime_to_timespec(const FILETIME *ft, struct timespec *ts)
 {
 	long long winTime = ((long long)ft->dwHighDateTime << 32) +
diff --git a/compat/cygwin.h b/compat/cygwin.h
index a3229f5..c04965a 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -4,6 +4,11 @@
 typedef int (*stat_fn_t)(const char*, struct stat*);
 extern stat_fn_t cygwin_stat_fn;
 extern stat_fn_t cygwin_lstat_fn;
+int cygwin_get_st_mode_bits(const char *path, int *mode);
 
+#define get_st_mode_bits(p,m) cygwin_get_st_mode_bits((p),(m))
+#ifndef CYGWIN_C
+/* cygwin.c needs the original lstat() */
 #define stat(path, buf) (*cygwin_stat_fn)(path, buf)
 #define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)
+#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index b7eaaa9..04a67ad 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -160,6 +160,7 @@
 typedef long intptr_t;
 typedef unsigned long uintptr_t;
 #endif
+int get_st_mode_bits(const char *path, int *mode);
 #if defined(__CYGWIN__)
 #undef _XOPEN_SOURCE
 #include <grp.h>
diff --git a/path.c b/path.c
index d3d3f8b..2fdccc2 100644
--- a/path.c
+++ b/path.c
@@ -14,6 +14,22 @@
 #include "strbuf.h"
 #include "string-list.h"
 
+#ifndef get_st_mode_bits
+/*
+ * The replacement lstat(2) we use on Cygwin is incomplete and
+ * may return wrong permission bits. Most of the time we do not care,
+ * but the callsites of this wrapper do care.
+ */
+int get_st_mode_bits(const char *path, int *mode)
+{
+	struct stat st;
+	if (lstat(path, &st) < 0)
+		return -1;
+	*mode = st.st_mode;
+	return 0;
+}
+#endif
+
 static char bad_path[] = "/bad-path/";
 
 static char *get_pathname(void)
@@ -391,7 +407,6 @@ const char *enter_repo(const char *path, int strict)
 
 int set_shared_perm(const char *path, int mode)
 {
-	struct stat st;
 	int tweak, shared, orig_mode;
 
 	if (!shared_repository) {
@@ -400,9 +415,8 @@ int set_shared_perm(const char *path, int mode)
 		return 0;
 	}
 	if (!mode) {
-		if (lstat(path, &st) < 0)
+		if (get_st_mode_bits(path, &mode) < 0)
 			return -1;
-		mode = st.st_mode;
 		orig_mode = mode;
 	} else
 		orig_mode = 0;
-- 
1.8.2.341.g543621f

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-03-23 12:40 [RFC] test-lib.sh: No POSIXPERM for cygwin Torsten Bögershausen
@ 2013-03-24  2:49 ` Eric Sunshine
  2013-03-25 15:53   ` Torsten Bögershausen
  0 siblings, 1 reply; 12+ messages in thread
From: Eric Sunshine @ 2013-03-24  2:49 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Junio C Hamano, j6t, Git List, ramsay

On Sat, Mar 23, 2013 at 8:40 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> Subject: [PATCH] Make core.sharedRepository work under cygwin 1.7
>
> When core.sharedRepository is used, set_shared_perm() in path.c
> needs lstat() to return the correct POSIX permissions.
>
> The default for cygwin is core.ignoreCygwinFSTricks = false, which
> means that the fast implementation in do_stat() is used instead of lstat().
>
> lstat() under cygwin uses the Windows security model to implement
> POSIX-like permissions.
> The user, group or everyone bits can be set individually.
>
> do_stat() simplifes the file permission bits, and may return a wrong value:
> The read-only attribute of a file is used to calculate
> the permissions, resulting in either rw-r--r-- or r--r--r--
>
> One effect of the simplified do_stat() is that t1301 failes.

s/failes/fails/

> Add a function cygwin_get_st_mode_bits() which returns the POSIX permissions.
> When not compiling for cygwin, true_mode_bits() in path.c is used.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-03-24  2:49 ` Eric Sunshine
@ 2013-03-25 15:53   ` Torsten Bögershausen
  0 siblings, 0 replies; 12+ messages in thread
From: Torsten Bögershausen @ 2013-03-25 15:53 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Torsten Bögershausen, Junio C Hamano, j6t, Git List, ramsay

On 24.03.13 03:49, Eric Sunshine wrote:
> On Sat, Mar 23, 2013 at 8:40 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> Subject: [PATCH] Make core.sharedRepository work under cygwin 1.7
[..]
> 
> s/failes/fails/
> 
Thanks for review,
I will send a new patch in a minute.

It is actually 2 patches,
- The fix as discussed here
- A small refactoring of set_shared_perm() in path.c,
  which I found while digging in the code.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-03-19 21:03 ` Junio C Hamano
@ 2013-03-19 21:10   ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2013-03-19 21:10 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Ramsay Jones, git, j6t

Junio C Hamano <gitster@pobox.com> writes:

> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Use a compile switch IGNORECYGWINFSTRICKS to disable the usage
>> of cygwin_lstat_fn() only in path.c
>
> The analysis of the problem and the basic idea to disable the
> fast-but-lying fstricks in the code that matters may be good, but
> the implementation is questionable.
>
> What if we later need to move functions around, etc., so that some
> other calls in path.c still do want to use the fstricks bit while
> the existing ones in the file want the real lstat() information?
>
> Actually, that already is the case.  The call to lstat() in
> validate_headref() only cares about the S_ISXXX() type and can
> afford to use the fast-and-lying one, no?
>
> How about doing something like this in the generic codepath, and
> implement your own cygwin_true_mode_bits() function at the cygwin
> compatibility layer, and add
>
>     #define true_mode_bits cygwin_true_mode_bits
>
> in the compat/cygwin.h file?  The change has the documentation value
> to clarify what each lstat() is used for, too.
>
> Ideally, the "#ifndef true_mode_bits" part may want to become a
> generic helper function if there are lstat() calls in other files
> that cygwin wants to use the real lstat() not the fast-but-lying
> one, but one step at a time.
>
> Hrm?
>
>  path.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index d3d3f8b..d0b31e5 100644
> --- a/path.c
> +++ b/path.c
> @@ -14,6 +14,21 @@
>  #include "strbuf.h"
>  #include "string-list.h"
>  
> +#ifndef true_mode_bits
> +/*
> + * The replacement lstat(2) we use on Cygwin is incomplete and
> + * lies about permission bits; most of the time we do not care,
> + * but the callsites of this wrpper do care.
> + */
> +static int true_mode_bits(const char *path, int *mode)
> +{
> +	struct stat st;
> +	if (lstat(path, &st) < 0)
> +		return -1;
> +	return st.st_mode;

Of course this should be more like

	*mode = st.st_mode;
        return 0;

but I think you got the idea ;-)

> +}
> +#endif
> +
>  static char bad_path[] = "/bad-path/";
>  
>  static char *get_pathname(void)
> @@ -400,9 +415,8 @@ int set_shared_perm(const char *path, int mode)
>  		return 0;
>  	}
>  	if (!mode) {
> -		if (lstat(path, &st) < 0)
> +		if (true_mode_bits(path, &mode) < 0)
>  			return -1;
> -		mode = st.st_mode;
>  		orig_mode = mode;
>  	} else
>  		orig_mode = 0;

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-03-19 19:49 Torsten Bögershausen
@ 2013-03-19 21:03 ` Junio C Hamano
  2013-03-19 21:10   ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2013-03-19 21:03 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Ramsay Jones, git, j6t

Torsten Bögershausen <tboegi@web.de> writes:

> Use a compile switch IGNORECYGWINFSTRICKS to disable the usage
> of cygwin_lstat_fn() only in path.c

The analysis of the problem and the basic idea to disable the
fast-but-lying fstricks in the code that matters may be good, but
the implementation is questionable.

What if we later need to move functions around, etc., so that some
other calls in path.c still do want to use the fstricks bit while
the existing ones in the file want the real lstat() information?

Actually, that already is the case.  The call to lstat() in
validate_headref() only cares about the S_ISXXX() type and can
afford to use the fast-and-lying one, no?

How about doing something like this in the generic codepath, and
implement your own cygwin_true_mode_bits() function at the cygwin
compatibility layer, and add

    #define true_mode_bits cygwin_true_mode_bits

in the compat/cygwin.h file?  The change has the documentation value
to clarify what each lstat() is used for, too.

Ideally, the "#ifndef true_mode_bits" part may want to become a
generic helper function if there are lstat() calls in other files
that cygwin wants to use the real lstat() not the fast-but-lying
one, but one step at a time.

Hrm?

 path.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/path.c b/path.c
index d3d3f8b..d0b31e5 100644
--- a/path.c
+++ b/path.c
@@ -14,6 +14,21 @@
 #include "strbuf.h"
 #include "string-list.h"
 
+#ifndef true_mode_bits
+/*
+ * The replacement lstat(2) we use on Cygwin is incomplete and
+ * lies about permission bits; most of the time we do not care,
+ * but the callsites of this wrpper do care.
+ */
+static int true_mode_bits(const char *path, int *mode)
+{
+	struct stat st;
+	if (lstat(path, &st) < 0)
+		return -1;
+	return st.st_mode;
+}
+#endif
+
 static char bad_path[] = "/bad-path/";
 
 static char *get_pathname(void)
@@ -400,9 +415,8 @@ int set_shared_perm(const char *path, int mode)
 		return 0;
 	}
 	if (!mode) {
-		if (lstat(path, &st) < 0)
+		if (true_mode_bits(path, &mode) < 0)
 			return -1;
-		mode = st.st_mode;
 		orig_mode = mode;
 	} else
 		orig_mode = 0;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
@ 2013-03-19 19:49 Torsten Bögershausen
  2013-03-19 21:03 ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Torsten Bögershausen @ 2013-03-19 19:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ramsay Jones, git, j6t, tboegi

On Friday 08 February 2013 07:08:14 Torsten Bögershausen wrote:
> On 07.02.13 20:35, Junio C Hamano wrote:
> > Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> >
> >> Torsten Bögershausen wrote:
> >>> t0070 and t1301 fail when running the test suite under cygwin.
> >>> Skip the failing tests by unsetting POSIXPERM.
> >> t1301 does not fail for me. (WIN XP (SP3) on NTFS)
> > Others run Cygwin with vfat or some other filesystem, and some of
> > them do not cope will with POSIXPERM, perhaps?
> >
> > Not having POSIXPERM by default for Cygwin may be a saner default
> > than having one, if we have to pick one.
> >
> > It may be debatable to have this default as platform attribute,
> > though.
> >
> Yes, 1301 passes on cygwin 1.5, but not on 1.7.
> And it breaks on VFAT, for all kind of OS.
> 
> Thanks for comments, I'll put more investigations on my todo stack.
> /Torsten

It turns out that cygwin 1.7 handles the read only attribute different then
cygwin 1.5.
Under cygwin 1.5 the "read only" attribute is synchronized with the
"user write" bit from the permissions.

tb@PC /cygdrive/c/temp/pt
$ echo x >x
$ chmod 444 x
$ attrib x
A    R     C:\temp\pt\x
Cygwin 1.7 does not do the synchronization, which means that some files
seem to be writable when do_stat() is used, but the permissions are r--r--r--

We can improve Git to work for cygwin 1.7, a suggestion for a patch
may look like this:

-- >8 --
Subject: [PATCH] Make core.sharedRepository work under cygwin 1.7

When core.sharedRepository is used, set_shared_perm() in path.c
needs lstat() to return the correct POSIX permissions.

The default for cygwin is core.ignoreCygwinFSTricks = false, which
means that a simplified and fast implementation of lstat() is used.

Especially the file permission bits are wrong in cygwin_lstat_fn():
The read-only attribute of a file is used to calculate
the permissions, resulting in either rw-r--r-- or r--r--r--

Use a compile switch IGNORECYGWINFSTRICKS to disable the usage
of cygwin_lstat_fn() only in path.c

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 compat/cygwin.h | 2 ++
 path.c          | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/compat/cygwin.h b/compat/cygwin.h
index a3229f5..984efbe 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -5,5 +5,7 @@ typedef int (*stat_fn_t)(const char*, struct stat*);
 extern stat_fn_t cygwin_stat_fn;
 extern stat_fn_t cygwin_lstat_fn;
 
+#ifndef IGNORECYGWINFSTRICKS
 #define stat(path, buf) (*cygwin_stat_fn)(path, buf)
 #define lstat(path, buf) (*cygwin_lstat_fn)(path, buf)
+#endif
diff --git a/path.c b/path.c
index d3d3f8b..0acfabf 100644
--- a/path.c
+++ b/path.c
@@ -10,6 +10,8 @@
  *
  * which is what it's designed for.
  */
+#define IGNORECYGWINFSTRICKS
+
 #include "cache.h"
 #include "strbuf.h"
 #include "string-list.h"
-- 
1.8.2.341.g543621f

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-02-07 19:35   ` Junio C Hamano
@ 2013-02-08  6:08     ` Torsten Bögershausen
  0 siblings, 0 replies; 12+ messages in thread
From: Torsten Bögershausen @ 2013-02-08  6:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ramsay Jones, Torsten Bögershausen, git, j6t

On 07.02.13 20:35, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> Torsten Bögershausen wrote:
>>> t0070 and t1301 fail when running the test suite under cygwin.
>>> Skip the failing tests by unsetting POSIXPERM.
>> t1301 does not fail for me. (WIN XP (SP3) on NTFS)
> Others run Cygwin with vfat or some other filesystem, and some of
> them do not cope will with POSIXPERM, perhaps?
>
> Not having POSIXPERM by default for Cygwin may be a saner default
> than having one, if we have to pick one.
>
> It may be debatable to have this default as platform attribute,
> though.
>
Yes, 1301 passes on cygwin 1.5, but not on 1.7.
And it breaks on VFAT, for all kind of OS.

Thanks for comments, I'll put more investigations on my todo stack.
/Torsten

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-02-07 18:25 ` Ramsay Jones
@ 2013-02-07 19:35   ` Junio C Hamano
  2013-02-08  6:08     ` Torsten Bögershausen
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2013-02-07 19:35 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Torsten Bögershausen, git, j6t

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

> Torsten Bögershausen wrote:
>> t0070 and t1301 fail when running the test suite under cygwin.
>> Skip the failing tests by unsetting POSIXPERM.
>
> t1301 does not fail for me. (WIN XP (SP3) on NTFS)

Others run Cygwin with vfat or some other filesystem, and some of
them do not cope will with POSIXPERM, perhaps?

Not having POSIXPERM by default for Cygwin may be a saner default
than having one, if we have to pick one.

It may be debatable to have this default as platform attribute,
though.

>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>>  t/test-lib.sh | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 1a6c4ab..94b097e 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -669,7 +669,6 @@ case $(uname -s) in
>>  	test_set_prereq SED_STRIPS_CR
>>  	;;
>>  *CYGWIN*)
>> -	test_set_prereq POSIXPERM
>>  	test_set_prereq EXECKEEPSPID
>>  	test_set_prereq NOT_MINGW
>>  	test_set_prereq SED_STRIPS_CR
>> 
>
> So, I'm not in favour of this, FWIW.
>
> ATB,
> Ramsay Jones

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-01-27 14:57 Torsten Bögershausen
  2013-02-06  9:34 ` Erik Faye-Lund
@ 2013-02-07 18:25 ` Ramsay Jones
  2013-02-07 19:35   ` Junio C Hamano
  1 sibling, 1 reply; 12+ messages in thread
From: Ramsay Jones @ 2013-02-07 18:25 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git, j6t

Torsten Bögershausen wrote:
> t0070 and t1301 fail when running the test suite under cygwin.
> Skip the failing tests by unsetting POSIXPERM.

t1301 does not fail for me. (WIN XP (SP3) on NTFS)
[It's so long since I looked, but I'm pretty sure that the failure
in t0070 is caused by *git*, not by cygwin not supporting POSIXPERM]

> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
>  t/test-lib.sh | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1a6c4ab..94b097e 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -669,7 +669,6 @@ case $(uname -s) in
>  	test_set_prereq SED_STRIPS_CR
>  	;;
>  *CYGWIN*)
> -	test_set_prereq POSIXPERM
>  	test_set_prereq EXECKEEPSPID
>  	test_set_prereq NOT_MINGW
>  	test_set_prereq SED_STRIPS_CR
> 

So, I'm not in favour of this, FWIW.

ATB,
Ramsay Jones

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-02-06  9:34 ` Erik Faye-Lund
@ 2013-02-06 20:16   ` Torsten Bögershausen
  0 siblings, 0 replies; 12+ messages in thread
From: Torsten Bögershausen @ 2013-02-06 20:16 UTC (permalink / raw)
  To: kusmabite; +Cc: Torsten Bögershausen, ramsay, git, j6t

Am 2013-02-06 10:34, schrieb Erik Faye-Lund:
> On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> t0070 and t1301 fail when running the test suite under cygwin.
>> Skip the failing tests by unsetting POSIXPERM.
>>
>
> But is this the real reason? I thought Cygwin implemented POSIX permissions...?
t0070:
  'mktemp to unwritable directory prints filename'
   mkdir cannotwrite &&
   chmod -w cannotwrite &&
   test_when_finished "chmod +w cannotwrite" &&
   test_must_fail test-mktemp cannotwrite/testXXXXXX 2>err &&
   grep "cannotwrite/test" err

When a directory under Linux/*nix has no write permission,
it is not allowed to create another directory (or file..) here.
This is not working under cygwin, a directory/file can be created
even if the parent directory has chmod 0.
-------------
tb@PC /cygdrive/c/temp
$ mkdir ttt

tb@PC /cygdrive/c/temp
$ chmod 0 ttt

tb@PC /cygdrive/c/temp
$ ls -ld ttt
d---------+ 1 tb None 0 Feb  6 20:33 ttt

tb@PC /cygdrive/c/temp
$ touch ttt/x

tb@PC /cygdrive/c/temp
$ ls -ld ttt
d---------+ 1 tb None 0 Feb  6 20:33 ttt

tb@PC /cygdrive/c/temp
$ ls -l ttt
total 0
-rw-r--r--+ 1 tb None 0 Feb  6 20:33 x
-------------------------------------------

If this is POSIX compliant? I'm not an expert here.
On the other hand:
This test case does not test git, but rather the file system,
so we can probaly remove it?

About 1301:
Some resereach needs to be done, to find out the connection between
umask, cygwin and the mount options.

On my system I have:
$mount
C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto)

/Torsten

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
  2013-01-27 14:57 Torsten Bögershausen
@ 2013-02-06  9:34 ` Erik Faye-Lund
  2013-02-06 20:16   ` Torsten Bögershausen
  2013-02-07 18:25 ` Ramsay Jones
  1 sibling, 1 reply; 12+ messages in thread
From: Erik Faye-Lund @ 2013-02-06  9:34 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: ramsay, git, j6t

On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> t0070 and t1301 fail when running the test suite under cygwin.
> Skip the failing tests by unsetting POSIXPERM.
>

But is this the real reason? I thought Cygwin implemented POSIX permissions...?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC] test-lib.sh: No POSIXPERM for cygwin
@ 2013-01-27 14:57 Torsten Bögershausen
  2013-02-06  9:34 ` Erik Faye-Lund
  2013-02-07 18:25 ` Ramsay Jones
  0 siblings, 2 replies; 12+ messages in thread
From: Torsten Bögershausen @ 2013-01-27 14:57 UTC (permalink / raw)
  To: ramsay, git; +Cc: j6t, tboegi

t0070 and t1301 fail when running the test suite under cygwin.
Skip the failing tests by unsetting POSIXPERM.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab..94b097e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -669,7 +669,6 @@ case $(uname -s) in
 	test_set_prereq SED_STRIPS_CR
 	;;
 *CYGWIN*)
-	test_set_prereq POSIXPERM
 	test_set_prereq EXECKEEPSPID
 	test_set_prereq NOT_MINGW
 	test_set_prereq SED_STRIPS_CR
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-03-25 15:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-23 12:40 [RFC] test-lib.sh: No POSIXPERM for cygwin Torsten Bögershausen
2013-03-24  2:49 ` Eric Sunshine
2013-03-25 15:53   ` Torsten Bögershausen
  -- strict thread matches above, loose matches on Subject: below --
2013-03-19 19:49 Torsten Bögershausen
2013-03-19 21:03 ` Junio C Hamano
2013-03-19 21:10   ` Junio C Hamano
2013-01-27 14:57 Torsten Bögershausen
2013-02-06  9:34 ` Erik Faye-Lund
2013-02-06 20:16   ` Torsten Bögershausen
2013-02-07 18:25 ` Ramsay Jones
2013-02-07 19:35   ` Junio C Hamano
2013-02-08  6:08     ` Torsten Bögershausen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).