[libcxx-commits] [PATCH] D140183: [libc++][Android] Disable test_no_resolve_symlink_on_symlink on Android

[Ryan Prichard via Phabricator via libcxx-commits]

rprichard added a comment.

In D140183#4009462 <https://reviews.llvm.org/D140183#4009462>, @ldionne wrote:

> Would it be better to instead `XFAIL` it using Lit machinery? Right now you're disabling the test even when it would succeed, which means you're losing that coverage.
>
> I'm fine with the patch, but I think you may want to do something less drastic to retain coverage on your platform.

I think the last bullet point is the correct behavior (no-op + operation_not_supported).

I'm not quite sure what this test ought to cover. Should it:

- Just verify that libc++ translates std::filesystem APIs properly to the Linux fchmodat API, or
- Also verify that fchmodat itself works.

If we want to cover both, then we'd need to define when fchmodat is expected to work correctly, which is complicated. Currently, this appears to be:

- If /data is f2fs, then fchmodat appears to work for Android M and up.
- If /data is any other FS, then fchmodat is currently broken, but it might be fixed in a new Android platform (libc.so) and/or a fixed Android kernel.

Maybe we can add a features.py test that looks for a broken fchmodat on Android? It would need to repeat parts of the symlink test but using fchmodat instead of std::filesystem. If we want to test fchmodat itself, then we could carve out the cases that are expected to work:

- Parse /proc/mounts and look for a /data of type f2fs.
- Use android_get_device_api_level to detect a platform that should work.
- If there's a kernel patch, then parse the kernel version and look for ones that are guaranteed to have the fix. (Older versions could have a backported fix?)

FWIW, I have another WIP patch that tries to fix the rest of std::filesystem for Android. It disables tests for hard links, FIFOs, and domain sockets, because all of these are blocked by SELinux policy, at least for the shell (non-root) user. Unfortunately, I suspect non-emulator L devices can't run any of the std::filesystem tests, because they can't create symlinks, and I haven't figured out yet how to accommodate that yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140183