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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGde0a22471157: Remove umask tests (authored by aganea). Changed prior to commit: https://reviews.llvm.org/D70854?vs=23340

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

rnk added a comment. I think we should remove it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70854/new/ https://reviews.llvm.org/D70854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

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

aganea added a comment. Yes this was added in rG18627115f4d2db5dc73207e0b5312f52536be7dd and rGe08b59f81d950bd5c8b8528fcb3ac4230c7b736c . It looks like it

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

rnk added a comment. 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

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

aganea updated this revision to Diff 233402. aganea added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. 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

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

aganea updated this revision to Diff 232100. aganea marked an inline comment as done. aganea added subscribers: tstellar, Meinersbur. aganea added a comment. 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

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

aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t rnk wrote: > aganea wrote: > > rnk wrote: > > > If you change this to `umask 022`,

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

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t aganea wrote: > rnk wrote: > > If you change thi

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

aganea marked 3 inline comments as done. aganea added inline comments. Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t rnk wrote: > If you change this to `umask 022`, does that result in `rw-r-`? T

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

rnk added inline comments. Comment at: clang/test/Misc/permissions.cpp:8 // RUN: umask 002 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t If you change this to `umask 022`, does that result in `rw-r-`? That would make the test meaningful on your system. Rep

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

aganea created this revision. aganea added reviewers: dblaikie, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. aganea retitled this revision from "[Clang] Do not always assume others permissions are set" to "[Clang] In tests, do not always assume others permissions ar