[libc++][Android] Disable test_no_resolve_symlink_on_symlink on Android

[rprichard]

Summary

The test is intended to ensure a particular behavior, but different
versions of Android (even just L and up) have multiple possible
behaviors.

Starting with Android M, fchmodat appears to successfully modify the
permissions of a symlink on an ext4 filesystem (but not f2fs). With
Android R, fchmodat then fails with operation_not_supported, but still
modifies the symlink's permissions. This issue is tracked internally at
http://b/262631136.

[Mordante]

Looks good from libc++'s point of view. I really like @danalbert review to see whether the comments are correct.

[rprichard]

In D140183#4003855, @Mordante wrote:

Looks good from libc++'s point of view. I really like @danalbert review to see whether the comments are correct.

Sounds good. @danalbert is going to be on vacation until next year. Maybe I can find someone else to review them?

[ldionne]

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.

[rprichard]

In 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.

[rprichard]

I reported a kernel bug related to this issue, https://bugzilla.kernel.org/show_bug.cgi?id=216834.

XFAIL would remove test coverage of the rest of this test file, which I think is more interesting than what test_no_resolve_symlink_on_symlink is testing. But it should be easy to split test_no_resolve_symlink_on_symlink into its own test file?

I think the condition we want for the XFAIL today is "/data/local/tmp is on an ext4 filesystem and/or the device's API is 21 or 22". The latter could be determined using android_get_device_api_level() and added to a general feature like android-device-api=$API. We could determine the filesystem type by dumping and parsing /proc/mounts. We may have to revisit that if the kernel gets fixed in a way that fixes this test for Bionic. (IIRC, I think other libc's implement fchmodat differently, in a way that doesn't expose the kernel behavior.)

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.permissions/permissions.pass.cpp

for (auto const& TC : cases) {
           permissions(TC.p, TC.set_perms, ec);
           TEST_CHECK(!ec);
           auto pp = status(TC.p).permissions();
           TEST_CHECK(pp == NormalizeExpectedPerms(TC.expected));
         }
     }
 }
 
-#ifndef _WIN32
+#if !defined(_WIN32) && !defined(__ANDROID__)
 // This test isn't currently meaningful on Windows; the Windows file
 // permissions visible via std::filesystem doesn't show any difference
 // between owner/group/others.
+//
+// Skip the test on Android because the behavior varies widely. A device
+// may do one of these things:
+//  - Ignore nofollow and modify the target (e.g. L emulator).
+//  - Refuse to let the shell user create a symlink (e.g. physical L device,
+//    volantis or Nexus 7 2013).
+//  - Change the symlink's permissions successfully (e.g. emulators from
+//    Android M to Q, ext4 filesystem).
+//  - Change the symlink's permissions, then fail with operation_not_supported
+//    anyway (e.g. R emulator and later, ext4 filesystem, see Google internal
+//    bug http://b/262631136).
+//  - Do nothing and fail with operation_not_supported (e.g. devices using f2fs
+//    for /data/local/tmp).
 TEST_CASE(test_no_resolve_symlink_on_symlink)
 {
     scoped_test_env env;
     const path file = env.create_file("file", 42);
     const path sym = env.create_symlink(file, "sym");
     const auto file_perms = status(file).permissions();
 
     struct TestCase {