[PATCH] D62718: [llvm-objcopy] Change handling of output file permissions

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I think this looks pretty good, but I'd like D63583 <https://reviews.llvm.org/D63583> to land first if possible.



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/mirror-permissions.test:1
+# RUN: touch %t
+# RUN: chmod 0777 %t
----------------
Add a comment at the top of this test that explains what the aim of the test is.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/mirror-permissions.test:12
+## exist on Windows.
+# RUN: if [[ "$OSTYPE" != "win32" ]]; then umask 0; fi
+
----------------
abrachet wrote:
> I know this won't be allowed, it was just to ask for help on how to do this. I don't want to make the entire test unsupported on Windows just because umask needs to be 0 on systems which have such a thing.  
I agree that this probably isn't the right way of doing things, and checking the permissions on Windows is important. How about having two tests, one for Windows and one for Linux? Effectively you already have that, just add `REQUIRES: windows` to this test and delete this block.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62718/new/

https://reviews.llvm.org/D62718