[Clang] In tests, do not always assume others permissions are set

[aganea]

Summary

On my Linux Ubuntu 18.04, because of default ACL settings, permissions for "others" are always unset/empty. The tests below used to fail without this patch.

[rnk]

clang/test/Misc/permissions.cpp

0

If you change this to umask 022, does that result in rw-r-----? That would make the test meaningful on your system.

[aganea]

clang/test/Misc/permissions.cpp

0

No, using umask 022 has no effect, still yields 'rw':

$ umask
0000
$ umask 0077
$ touch test
$ ls -l
-rw-rw---- 1 aganea sudosgroup    0 Dec  3 11:02 test

So this seems to be related to the interaction between ACL and umask. The umask doc says they should interact, but that doesn't seem to work on my Ubuntu 18.04.01. No matter what I set in the umask mode, creating a new file inherits the default ACL.
All my folders have ACL enabled:

$ ls -l /mnt/
drwxrws--x+ 8 root sudosgroup 4096 Dec  3 17:07 f       <-- note the + which indicates ACL is being used

I could give 'rw' permissions to others:

$ setfacl -R -d -m o::rw /mnt/f

However even with that, the test fails (because umask has no effect).
I'm not sure what the right fix would be here. I can investigate other things. Any suggestions?

[rnk]

lgtm

clang/test/Misc/permissions.cpp

0

I'm not sure what the right fix would be here. I can investigate other things. Any suggestions?

I don't think it's worth it. Let's go with your fix and make the test pass.

[aganea]

clang/test/Misc/permissions.cpp

0

Good. I'll add a comment above in the test to explain the situation.

[aganea]

Just after I hit Submit last night, I found a better way, by disabling ACL for that folder.
Ping @tstellar @Meinersbur in case they have an opinion.
Please let me know which solution you think is better.

[aganea]

Ensure the access-rights commands (setfacl and chmod) won't fail the tests if the user doesn't have the appropriate rights to change permissions.
Added llvm/test/Other/umask.ll because it was also failing.

[rnk]

I see there is no setfacl on Mac. Personally, I feel like this test doesn't have much value. Rafael added it long ago when we changed how we opened files in some way that I guess affected umask. I'm not sure we really need this regression test given how unportable it seems to be.

[aganea]

Yes this was added in rG18627115f4d2db5dc73207e0b5312f52536be7dd and rGe08b59f81d950bd5c8b8528fcb3ac4230c7b736c. It looks like it was only there to validate that specific refactoring.
Should I remove these two files, or simply commit the previous diff? (ie. -rw-rw----)

[rnk]

I think we should remove it.

clang/test/Misc/permissions.cpp

-// REQUIRES: shell
-
-// RUN: umask 000
-// RUN: %clang_cc1 -emit-llvm-bc %s -o %t
-// RUN: ls -l %t | FileCheck --check-prefix=CHECK000 %s
-// CHECK000: rw-rw-rw-
-
-// RUN: umask 002
-// RUN: %clang_cc1 -emit-llvm-bc %s -o %t
-// RUN: ls -l %t | FileCheck --check-prefix=CHECK002 %s
-// CHECK002: rw-rw-r--

llvm/test/Other/umask.ll

-; REQUIRES: shell
-; XFAIL: windows-gnu
-
-; RUN: umask 000
-; RUN: rm -f %t.000
-; RUN: llvm-as %s -o %t.000
-; RUN: ls -l %t.000 | FileCheck --check-prefix=CHECK000 %s
-; CHECK000: rw-rw-rw
-
-; RUN: umask 002
-; RUN: rm -f %t.002
-; RUN: llvm-as %s -o %t.002
-; RUN: ls -l %t.002 | FileCheck --check-prefix=CHECK002 %s
-; CHECK002: rw-rw-r-