[PATCH] D80838: [llvm-ar] Add more tests for errors in opening archives

Thu Jun 4 01:34:59 PDT 2020

jhenderson added a comment.

In D80838#2071252 <https://reviews.llvm.org/D80838#2071252>, @sameerarora101 wrote:

> @jhenderson I think you were right. I'll try running the test case with `# UNSUPPORTED: system-windows`. However, I still don't understand why other test cases, for eg. `read-only-archive.test` or https://github.com/llvm/llvm-project/blob/master/lld/test/ELF/driver-access.test , are able to run on windows with `chmod`. Is the issue with the error msg? I checked the logs too but they aren't that helpful :(

I can't say I fully understand it either, but I verified that it doesn't work on my Windows box either - it's actually successfully reading and printing the file contents. `chmod` does sort of work on Windows, but access permissions are different so it only is a rough approximation of the Unix equivalent. Thus, it is fairly trivial to mark things (to some degree at least) as read only (hence why those other tests work), but I'm guessing not as "unreadable". There's probably a way to do it, but not using chmod, and therefore unlikely to be in a portable fashion.

Arguably, the permission denied test isn't even needed in this case - the same code path at the tool level at least handles the "is directory" case too. I don't mind it being included though.

================
Comment at: llvm/test/tools/llvm-ar/error-opening-archive.test:1
+# UNSUPPORTED: system-windows
+
----------------
Rather than making all cases unsupported on Windows, I think it would be preferable to split this test into two, with the permission bit in its own test. You should also add a comment to that case saying why it is unsupported on Windows.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80838