Re: Unreadable directories aren't included in glob expansion

Re: Unreadable directories aren't included in glob expansion

[Jun. T]

(moved from users/24633⁩)

> 2020/01/11 2:09, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> 
> diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
(snip)
> + mkdir -m 000 glob.tmp/secret-d000
> + mkdir -m 111 glob.tmp/secret-d111
> + mkdir -m 444 glob.tmp/secret-d444
> + for 1 in 000 111 444 ; do ln -s secret-d$1 glob.tmp/secret-s$1; done
> + print -rC 2 -- glob.tmp/secret-*/ glob.tmp/secret-*(-/)
> +-f:unreadable directories can be globbed (users/24619, users/24626)
(snip)

This test fails on Cygwin, i.e., does not fail as expected.

This is because stat("glob.tmp/secret-d000/.") succeeds on Cygwin.
I asked about this in the cygwin mailing list and learned that
this is the default behavior of Windows and it is not easy for
Cygwin to override it.
I think just skipping this test on Cygwin is enough for now.



diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
index b0650c8c8..973dc2207 100644
--- a/Test/D02glob.ztst
+++ b/Test/D02glob.ztst
@@ -729,11 +729,15 @@
 >not/exist
 >exist
 
- mkdir -m 000 glob.tmp/secret-d000
- mkdir -m 111 glob.tmp/secret-d111
- mkdir -m 444 glob.tmp/secret-d444
- for 1 in 000 111 444 ; do ln -s secret-d$1 glob.tmp/secret-s$1; done
- print -rC 2 -- glob.tmp/secret-*/ glob.tmp/secret-*(-/)
+ if [[ $OSTYPE = cygwin* ]]; then
+   ZTST_skip='Cygwin ignores search permission of directories'
+ else
+   mkdir -m 000 glob.tmp/secret-d000
+   mkdir -m 111 glob.tmp/secret-d111
+   mkdir -m 444 glob.tmp/secret-d444
+   for 1 in 000 111 444 ; do ln -s secret-d$1 glob.tmp/secret-s$1; done
+   print -rC 2 -- glob.tmp/secret-*/ glob.tmp/secret-*(-/)
+ fi
 -f:unreadable directories can be globbed (users/24619, users/24626)
 >glob.tmp/secret-d000/  glob.tmp/secret-d000
 >glob.tmp/secret-d111/  glob.tmp/secret-d111

Re: Unreadable directories aren't included in glob expansion

[Daniel Shahaf]

Jun. T wrote on Thu, 27 Feb 2020 00:51 +0900:
> (moved from users/24633⁩)
> 
> > 2020/01/11 2:09, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > 
> > diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst  
> (snip)
> > + mkdir -m 000 glob.tmp/secret-d000
> > + mkdir -m 111 glob.tmp/secret-d111
> > + mkdir -m 444 glob.tmp/secret-d444
> > + for 1 in 000 111 444 ; do ln -s secret-d$1 glob.tmp/secret-s$1; done
> > + print -rC 2 -- glob.tmp/secret-*/ glob.tmp/secret-*(-/)
> > +-f:unreadable directories can be globbed (users/24619, users/24626)  
> (snip)
> 
> This test fails on Cygwin, i.e., does not fail as expected.
> This is because stat("glob.tmp/secret-d000/.") succeeds on Cygwin.
> I asked about this in the cygwin mailing list and learned that
> this is the default behavior of Windows and it is not easy for
> Cygwin to override it.
> I think just skipping this test on Cygwin is enough for now.

I assume some day we'll fix the C code to make the test pass on other
platforms.  When that happens, should the test then continue be skipped
on Cygwin?  If not, I suggest adding a comment reminding to remove the
skip when the test starts to pass on other platforms.

Cheers,

Daniel

Re: Unreadable directories aren't included in glob expansion

[Jun T]

> 2020/02/27 22:00, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> 
> Jun. T wrote on Thu, 27 Feb 2020 00:51 +0900:
>> 
>> I think just skipping this test on Cygwin is enough for now.
> 
> I assume some day we'll fix the C code to make the test pass on other
> platforms.  When that happens, should the test then continue be skipped
> on Cygwin?  If not, I suggest adding a comment reminding to remove the
> skip when the test starts to pass on other platforms.

Sorry, I was not following this thread; what kind of "fix" do you
have in mind?

Anyway, the only thing I know is
    both  access("d000/.", F_OK)  and  stat("d000/.", st)
succeed on Cygwin. If the revised C code depends on the failure of 
either of these calls then the test need be skipped on Cygwin,
I think.

Re: Unreadable directories aren't included in glob expansion

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

On Thu, Feb 27, 2020, 10:38 PM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:

>
> Anyway, the only thing I know is
>     both  access("d000/.", F_OK)  and  stat("d000/.", st)
> succeed on Cygwin.
>

What about "d000/.." ?

(I don't have the code handy, but why are we using F_OK instead of R_OK?)

>

Re: Unreadable directories aren't included in glob expansion

[Jun T]

> 2020/02/28 15:51, Bart Schaefer <schaefer@brasslantern.com> :
> 
> On Thu, Feb 27, 2020, 10:38 PM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
> 
> Anyway, the only thing I know is
>     both  access("d000/.", F_OK)  and  stat("d000/.", st)
> succeed on Cygwin.
> 
> What about "d000/.." ?

Both access(, F_OK) and stat() succeed for "d000/..".
If there is "afile" in d000/ then they also succeed for "d000/afile".

Re: Unreadable directories aren't included in glob expansion

[Daniel Shahaf]

Jun T wrote on Fri, 28 Feb 2020 15:37 +0900:
> > 2020/02/27 22:00, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > 
> > Jun. T wrote on Thu, 27 Feb 2020 00:51 +0900:  
> >> 
> >> I think just skipping this test on Cygwin is enough for now.  
> > 
> > I assume some day we'll fix the C code to make the test pass on other
> > platforms.  When that happens, should the test then continue be skipped
> > on Cygwin?  If not, I suggest adding a comment reminding to remove the
> > skip when the test starts to pass on other platforms.  
> 
> Sorry, I was not following this thread; what kind of "fix" do you
> have in mind?

I do not have a specific fix in mind.  I just assumed that at some
indefinite point in the future we will change the C code in a way that
makes the test produce the expected output, and at the same time remove
the 'f' flag from the test.

> Anyway, the only thing I know is
>     both  access("d000/.", F_OK)  and  stat("d000/.", st)
> succeed on Cygwin. If the revised C code depends on the failure of 
> either of these calls then the test need be skipped on Cygwin,
> I think.

Why would we need to skip the test on Cygwin after we fix it on other
platforms?  The only reason the test fails on Cygwin is because it's
marked 'f' (= expected to fail) and _does_ produce the expected
output.  Once we fix the C code so the test produces the expected
output, I think we'll be able to remove the 'f' flag _and_ the skip,
and then the test will be passing on all platforms, including Cygwin.
What am I missing?

Cheers,

Daniel

Re: Unreadable directories aren't included in glob expansion

[Daniel Shahaf]

Daniel Shahaf wrote on Fri, 28 Feb 2020 14:26 +0000:
> Jun T wrote on Fri, 28 Feb 2020 15:37 +0900:
> > > 2020/02/27 22:00, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > 
> > > Jun. T wrote on Thu, 27 Feb 2020 00:51 +0900:    
> > >> 
> > >> I think just skipping this test on Cygwin is enough for now.    
> > > 
> > > I assume some day we'll fix the C code to make the test pass on other
> > > platforms.  When that happens, should the test then continue be skipped
> > > on Cygwin?  If not, I suggest adding a comment reminding to remove the
> > > skip when the test starts to pass on other platforms.    
> > 
> > Sorry, I was not following this thread; what kind of "fix" do you
> > have in mind?  
> 
> I do not have a specific fix in mind.  I just assumed that at some
> indefinite point in the future we will change the C code in a way that
> makes the test produce the expected output, and at the same time remove
> the 'f' flag from the test.

Actually, it turns out that the test in question has already been fixed
on the 5.9 branch.  (I completely forgot about that before, sorry.  I only
ran into it now because I was trying to merge 5.9 into master.)

The fix is in commit edc04bd94656a152e17f06188bcef172d62a3ad4.

Could you check if D02 passes on the 5.9 branch on Cygwin?  If it does,
I'll merge master into 5.9, resolve the conflict on D02 by removing the
skip and leaving the test exit code and flags as "0:" (= pass), and then
merge 5.9 into master.

> > Anyway, the only thing I know is
> >     both  access("d000/.", F_OK)  and  stat("d000/.", st)
> > succeed on Cygwin. If the revised C code depends on the failure of 
> > either of these calls then the test need be skipped on Cygwin,
> > I think.  
> 
> Why would we need to skip the test on Cygwin after we fix it on other
> platforms?  The only reason the test fails on Cygwin is because it's
> marked 'f' (= expected to fail) and _does_ produce the expected
> output.  Once we fix the C code so the test produces the expected
> output, I think we'll be able to remove the 'f' flag _and_ the skip,
> and then the test will be passing on all platforms, including Cygwin.
> What am I missing?

Cheers,

Daniel

Re: Unreadable directories aren't included in glob expansion

[Jun T]

> 2020/02/28 23:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> 
> Daniel Shahaf wrote on Fri, 28 Feb 2020 14:26 +0000:
> 
> Actually, it turns out that the test in question has already been fixed
> on the 5.9 branch.  (I completely forgot about that before, sorry.  I only
> ran into it now because I was trying to merge 5.9 into master.)
> 
> The fix is in commit edc04bd94656a152e17f06188bcef172d62a3ad4.
> 
> Could you check if D02 passes on the 5.9 branch on Cygwin?  If it does,
> I'll merge master into 5.9, resolve the conflict on D02 by removing the
> skip and leaving the test exit code and flags as "0:" (= pass), and then
> merge 5.9 into master.

So you are going to fix the C code so that unreadable/unsearchable directory
does match with "*/".

On Cygwin, all of d000 d111 d444 match with */ on either master or 5.9
branch. So the test D02 passes on 5.9 (with flag '0:').

Re: Unreadable directories aren't included in glob expansion

[Daniel Shahaf]

Jun T wrote on Tue, 03 Mar 2020 01:57 +00:00:
> 
> > 2020/02/28 23:55, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > 
> > Daniel Shahaf wrote on Fri, 28 Feb 2020 14:26 +0000:
> > 
> > Actually, it turns out that the test in question has already been fixed
> > on the 5.9 branch.  (I completely forgot about that before, sorry.  I only
> > ran into it now because I was trying to merge 5.9 into master.)
> > 
> > The fix is in commit edc04bd94656a152e17f06188bcef172d62a3ad4.
> > 
> > Could you check if D02 passes on the 5.9 branch on Cygwin?  If it does,
> > I'll merge master into 5.9, resolve the conflict on D02 by removing the
> > skip and leaving the test exit code and flags as "0:" (= pass), and then
> > merge 5.9 into master.
> 
> So you are going to fix the C code so that unreadable/unsearchable directory
> does match with "*/".
> 

Yes.  If you have any reservations about that, don't hesitate to raise them.

> On Cygwin, all of d000 d111 d444 match with */ on either master or 5.9
> branch. So the test D02 passes on 5.9 (with flag '0:').

Thanks.  I'll merge when I have time, but I don't know when I'll have time.

Cheers,

Daniel