Apply suggestions from code review

Co-authored-by: Victor Stinner <[email protected]>

Author: jbosboom

Doc/library/os.rst

@@ -3383,7 +3383,7 @@ features:
       Added the :attr:`st_birthtime` member on Windows.
 
 
-.. function:: statx(path, mask, *, dir_fd=None, follow_symlinks=True, sync=None)
+.. function:: statx(path, mask, flags=0, *, dir_fd=None, follow_symlinks=True)
 
    Get the status of a file or file descriptor by performing a :c:func:`!statx`
    system call on the given path.
@@ -3392,18 +3392,12 @@ features:
    indirectly through the :class:`PathLike` interface -- or as an open file
    descriptor. *mask* is a combination of the module-level
    :const:`STATX_* <STATX_TYPE>` constants specifying the information to
-   retrieve. Returns a :class:`statx_result` object whose
+   retrieve. *flags* is a combination of the module-level
+   :const:`AT_STATX_* <AT_STATX_FORCE_SYNC>` constants and/or
+   :const:`AT_NO_AUTOMOUNT`. Returns a :class:`statx_result` object whose
    :attr:`~os.statx_result.stx_mask` attribute specifies the information
    actually retrieved (which may differ from *mask*).
 
-   The optional parameter *sync* controls the freshness of the returned
-   information.  ``sync=True`` requests that the kernel return up-to-date
-   information, even when doing so is expensive (for example, requiring a
-   round trip to the server for a file on a network filesystem).
-   ``sync=False`` requests that the kernel return cached information if
-   available.  ``sync=None`` expresses no preference, in which case the kernel
-   will return information as fresh as :func:`~os.stat` does.
-
    This function supports :ref:`specifying a file descriptor <path_fd>`,
    :ref:`paths relative to directory descriptors <dir_fd>`, and
    :ref:`not following symlinks <follow_symlinks>`.
@@ -3515,6 +3509,49 @@ features:
 
    .. versionadded:: next
 
+.. data:: AT_STATX_FORCE_SYNC
+
+   A flag for the :func:`os.statx` function.  Requests that the kernel return
+   up-to-date information even when doing so is expensive (for example,
+   requiring a round trip to the server for a file on a network filesystem).
+
+   .. availability:: Linux >= 4.11 with glibc >= 2.28.
+
+   .. versionadded:: next
+
+.. data:: AT_STATX_DONT_SYNC
+
+   A flag for the :func:`os.statx` function.  Requests that the kernel return
+   cached information if possible.
+
+   .. availability:: Linux >= 4.11 with glibc >= 2.28.
+
+   .. versionadded:: next
+
+.. data:: AT_STATX_SYNC_AS_STAT
+
+   A flag for the :func:`os.statx` function.  This flag is defined as ``0``, so
+   it has no effect, but it can be used to explicitly indicate neither
+   :data:`AT_STATX_FORCE_SYNC` nor :data:`AT_STATX_DONT_SYNC` is being passed.
+   In the absence of the other two flags, the kernel will generally return
+   information as fresh as :func:`os.stat` would return.
+
+   .. availability:: Linux >= 4.11 with glibc >= 2.28.
+
+   .. versionadded:: next
+
+
+.. data:: AT_NO_AUTOMOUNT
+
+   If the final component of a path is an automount point, operate on the
+   automount point instead of performing the automount.  (On Linux,
+   :func:`os.stat`, :func:`os.fstat` and :func:`os.lstat` always behave this
+   way.)
+
+   .. availability:: Linux.
+
+   .. versionadded:: next
+
 
 .. function:: statvfs(path)
 

Include/internal/pycore_global_objects_fini_generated.h

@@ -785,7 +785,6 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(sub_key)
         STRUCT_FOR_ID(subcalls)
         STRUCT_FOR_ID(symmetric_difference_update)
-        STRUCT_FOR_ID(sync)
         STRUCT_FOR_ID(tabsize)
         STRUCT_FOR_ID(tag)
         STRUCT_FOR_ID(target)

Include/internal/pycore_global_strings.h

@@ -812,14 +812,6 @@ def test_statx_attributes_bytes(self):
     def test_statx_attributes_pathlike(self):
         self.check_statx_attributes(FakePath(self.fname))
 
-    @unittest.skipUnless(hasattr(os, 'statx'), 'test needs os.statx()')
-    def test_statx_sync(self):
-        # Test sync= kwarg parsing.  (We can't predict if or how the result
-        # will change.)
-        for sync in (False, True):
-            with self.subTest(sync=sync):
-                os.statx(self.fname, os.STATX_BASIC_STATS, sync=sync)
-
     @unittest.skipUnless(hasattr(os, 'statvfs'), 'test needs os.statvfs()')
     def test_statvfs_attributes(self):
         result = os.statvfs(self.fname)

Include/internal/pycore_runtime_init_generated.h

@@ -668,22 +668,65 @@ def test_fstat(self):
         finally:
             fp.close()
 
-    def test_stat(self):
-        self.assertTrue(posix.stat(os_helper.TESTFN))
-        self.assertTrue(posix.stat(os.fsencode(os_helper.TESTFN)))
+    def check_statlike_path(self, func):
+        self.assertTrue(func(os_helper.TESTFN))
+        self.assertTrue(func(os.fsencode(os_helper.TESTFN)))
+        self.assertTrue(func(os_helper.FakePath(os_helper.TESTFN)))
 
         self.assertRaisesRegex(TypeError,
                 'should be string, bytes, os.PathLike or integer, not',
-                posix.stat, bytearray(os.fsencode(os_helper.TESTFN)))
+                func, bytearray(os.fsencode(os_helper.TESTFN)))
         self.assertRaisesRegex(TypeError,
                 'should be string, bytes, os.PathLike or integer, not',
-                posix.stat, None)
+                func, None)
         self.assertRaisesRegex(TypeError,
                 'should be string, bytes, os.PathLike or integer, not',
-                posix.stat, list(os_helper.TESTFN))
+                func, list(os_helper.TESTFN))
         self.assertRaisesRegex(TypeError,
                 'should be string, bytes, os.PathLike or integer, not',
-                posix.stat, list(os.fsencode(os_helper.TESTFN)))
+                func, list(os.fsencode(os_helper.TESTFN)))
+
+    def test_stat(self):
+        self.check_statlike_path(posix.stat)
+
+    @unittest.skipUnless(hasattr(posix, 'statx'), 'test needs posix.statx()')
+    def test_statx(self):
+        def func(path, **kwargs):
+            return posix.statx(path, posix.STATX_BASIC_STATS, **kwargs)
+        self.check_statlike_path(func)
+
+    @unittest.skipUnless(hasattr(posix, 'statx'), 'test needs posix.statx()')
+    def test_statx_flags(self):
+        # glibc's fallback implementation of statx via the stat family fails
+        # with EINVAL on the (nonzero) sync flags.  If you see this failure,
+        # update your kernel and/or seccomp syscall filter.
+        valid_flag_names = ('AT_NO_AUTOMOUNT', 'AT_STATX_SYNC_AS_STAT',
+                            'AT_STATX_FORCE_SYNC', 'AT_STATX_DONT_SYNC')
+        for flag_name in valid_flag_names:
+            flag = getattr(posix, flag_name)
+            with self.subTest(msg=flag_name, flags=flag):
+                posix.statx(os_helper.TESTFN, posix.STATX_BASIC_STATS,
+                            flags=flag)
+
+        # These flags are not exposed to Python because their functionality is
+        # implemented via kwargs instead.
+        kwarg_equivalent_flags = (
+            (0x0100, 'AT_SYMLINK_NOFOLLOW', 'follow_symlinks'),
+            (0x0400, 'AT_SYMLINK_FOLLOW', 'follow_symlinks'),
+            (0x1000, 'AT_EMPTY_PATH', 'dir_fd'),
+        )
+        for flag, flag_name, kwarg_name in kwarg_equivalent_flags:
+            with self.subTest(msg=flag_name, flags=flag):
+                with self.assertRaisesRegex(ValueError, kwarg_name):
+                    posix.statx(os_helper.TESTFN, posix.STATX_BASIC_STATS,
+                                flags=flag)
+
+        with self.subTest(msg="AT_STATX_FORCE_SYNC | AT_STATX_DONT_SYNC"):
+            with self.assertRaises(OSError) as ctx:
+                flags = posix.AT_STATX_FORCE_SYNC | posix.AT_STATX_DONT_SYNC
+                posix.statx(os_helper.TESTFN, posix.STATX_BASIC_STATS,
+                            flags=flags)
+            self.assertEqual(ctx.exception.errno, errno.EINVAL)
 
     @unittest.skipUnless(hasattr(posix, 'mkfifo'), "don't have mkfifo()")
     def test_mkfifo(self):