Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Doc/library/os.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2807,6 +2807,16 @@ features:
This function can also support :ref:`paths relative to directory descriptors
<dir_fd>`.

On Linux, Android and MacOS, *path* can also be a file descriptor referring
to a symbolic link. In that case, *dir_fd* must be ``None``, and the return
value will be a string.
(On Linux and Android, such a file descriptor can be obtained through
:func:`os.open` with ``os.RDONLY | os.O_PATH | os.O_NOFOLLOW``.
On MacOS, this is possible by calling :func:`os.open` with
``os.O_RDONLY | os.O_SYMLINK``.)
On other operating systems, a ``NotImplementedError`` is raised if *path*
is an integer.

When trying to resolve a path that may contain links, use
:func:`~os.path.realpath` to properly handle recursion and platform
differences.
Expand All @@ -2829,6 +2839,10 @@ features:
substitution path (which typically includes ``\\?\`` prefix) rather
than the optional "print name" field that was previously returned.

.. versionchanged:: 3.16
Accepts file descriptors pointing to symbolic links as *path* on
Linux, Android and MacOS.

.. function:: remove(path, *, dir_fd=None)

Remove (delete) the file *path*. If *path* is a directory, an
Expand Down
4 changes: 4 additions & 0 deletions Doc/whatsnew/3.16.rst
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,10 @@ os
now raises :exc:`PermissionError` instead of the functions being unavailable.
(Contributed by Md Arif in :gh:`152936`.)

* :func:`os.readlink` now accepts a file descriptor referring to a symlink on
Linux, Android and MacOS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add Android to the other docs?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

(Contributed by OOTS in :gh:`157899`.)


pydoc
-----
Expand Down
3 changes: 3 additions & 0 deletions Lib/os.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ def _add(str, fn):
_add("HAVE_FSTATVFS", "statvfs")
if _exists("statx"):
_set.add(statx)
_add("HAVE_FREADLINK", "readlink")
if sys.platform in ["linux", "android"] and "O_PATH" in _globals:
_add("HAVE_READLINKAT", "readlink")
supports_fd = _set

_set = set()
Expand Down
82 changes: 82 additions & 0 deletions Lib/test/test_os/test_posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,77 @@ def test_readlink_dir_fd(self):
self.addCleanup(posix.unlink, fullname)
self.assertEqual(posix.readlink(name, dir_fd=dir_fd), 'symlink')


_support_readlink_with_fd = hasattr(os, 'readlink') and (
"HAVE_FREADLINK" in posix._have_functions # MacOS
or (
os.readlink in os.supports_dir_fd
and sys.platform in ["linux", "android"]
)
)
Comment on lines +1885 to +1891

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One open question is whether os.readlink() should be added to supports_fd. I don't have an opinion on that, happy to take others' lead here.

I'd say add them. O_SYMLINK/O_PATH|O_NOFOLLOW are the way to get fds referring to a symlink on their respective platform, so this can be just os.readlink in os.supports_dir_fd.
(Yes, I changed my mind after going through the code.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then, please also test that os.readlink in os.supports_dir_fd holds on MacOS, Linux & Android.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. os.readlink is now added automatically to os.supports_fd. I also added a test that checks presence/absence on all platforms where both os.readlink and os.supports_fd are defined.


def _open_symlink_as_fd(self, path):
open_flags = os.O_RDONLY
if hasattr(os, "O_SYMLINK"): # MacOS
open_flags |= os.O_SYMLINK
elif hasattr(os, "O_NOFOLLOW") and hasattr(os, "O_PATH"): # Linux
open_flags |= os.O_NOFOLLOW | os.O_PATH
else:
self.fail("lacking open flags for this test")
return os.open(path, open_flags)

@unittest.skipUnless(_support_readlink_with_fd,
"feature not supported on this platform")
def test_readlink_with_fd(self):
with self.prepare() as (dir_fd, name, fullname):
os.symlink("symlink", fullname)
self.addCleanup(posix.unlink, fullname)
fd = self._open_symlink_as_fd(fullname)
self.addCleanup(os.close, fd)
self.assertEqual(os.readlink(fd), "symlink")

@unittest.skipUnless(_support_readlink_with_fd,
"feature not supported on this platform")
def test_readlink_with_fd_not_referring_to_symlink_throws(self):
with self.prepare_file() as (dir_fd, name, fullname):
fd = os.open(fullname, os.O_RDONLY)
self.addCleanup(os.close, fd)
# on Linux/Android, readlinkat("", fd, ...) fails with ENOENT, which
# Python translates to a FileNotFoundError, a subclass of OSError.
# On MacOS, freadlink(fd, ...) fails with EINVAL, which gets raised
# as a OSError.
# So catching OSError here covers both cases.
with self.assertRaises(OSError):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My AI was obsessing a bit about this: Depending on circumstances (how you create the FD and the OS) it raises a bare OSError (due to EINVAL), or FileNotFoundError (due to ENOENT).

I agree that consistency would be nice, but I thought tinkering with the error handling is probably worse than this little inconsistency.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put this comment in the test code?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comment lines into the test.

os.readlink(fd)

@unittest.skipUnless(_support_readlink_with_fd,
"feature not supported on this platform")
def test_readlink_with_fd_throws_if_both_fd_and_dir_fd_are_given(self):
with self.prepare() as (dir_fd, name, fullname):
os.symlink("symlink", fullname)
self.addCleanup(posix.unlink, fullname)
fd = self._open_symlink_as_fd(fullname)
self.addCleanup(os.close, fd)
with self.assertRaises(ValueError):
os.readlink(fd, dir_fd=dir_fd)

@unittest.skipIf(_support_readlink_with_fd,
"feature is supported on this platform")
def test_readlink_with_fd_throws_not_implemented_error(self):
# on unsupported platforms, we may not even be able to get a
# file descriptor for a symlink, so use a fd for an ordinary file
os_helper.create_empty_file(os_helper.TESTFN)
self.addCleanup(os_helper.unlink, os_helper.TESTFN)
fd = os.open(os_helper.TESTFN, os.O_RDONLY)
self.addCleanup(os.close, fd)
with self.assertRaises(NotImplementedError):
os.readlink(fd)

@unittest.skipUnless(hasattr(os, "supports_fd") and hasattr(os, "readlink"),
"feature not supported on this platform")
def test_readlink_is_in_supports_fd_on_supported_platforms(self):
self.assertEqual(os.readlink in os.supports_fd, self._support_readlink_with_fd)

@unittest.skipUnless(os.rename in os.supports_dir_fd, "test needs dir_fd support in os.rename()")
def test_rename_dir_fd(self):
with self.prepare_file() as (dir_fd, name, fullname), \
Expand Down Expand Up @@ -2620,6 +2691,17 @@ def test_readlink(self):
with self.assertRaisesRegex(NotImplementedError, "dir_fd unavailable"):
os.readlink("path", dir_fd=0)

def test_freadlink(self):
self._verify_available("HAVE_FREADLINK")
if self.mac_ver >= (13, 0):
self.assertIn("HAVE_FREADLINK", posix._have_functions)

else:
self.assertNotIn("HAVE_FREADLINK", posix._have_functions)

with self.assertRaisesRegex(NotImplementedError, "readlink cannot read file descriptors on this platform"):
os.readlink(0)

def test_symlink(self):
self._verify_available("HAVE_SYMLINKAT")
if self.mac_ver >= (10, 10):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:func:`os.readlink` now accepts a file descriptor referring to a
symlink on Linux, Android and MacOS.
18 changes: 15 additions & 3 deletions Modules/clinic/posixmodule.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

102 changes: 87 additions & 15 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ static const unsigned int _Py_STATX_KNOWN = (STATX_BASIC_STATS | STATX_BTIME
# define HAVE_UNLINKAT_RUNTIME __builtin_available(macOS 10.10, iOS 8.0, *)
# define HAVE_OPENAT_RUNTIME __builtin_available(macOS 10.10, iOS 8.0, *)
# define HAVE_READLINKAT_RUNTIME __builtin_available(macOS 10.10, iOS 8.0, *)
# define HAVE_FREADLINK_RUNTIME __builtin_available(macOS 13.0, *)
# define HAVE_SYMLINKAT_RUNTIME __builtin_available(macOS 10.10, iOS 8.0, *)
# define HAVE_FUTIMENS_RUNTIME __builtin_available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)
# define HAVE_UTIMENSAT_RUNTIME __builtin_available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)
Expand Down Expand Up @@ -571,6 +572,10 @@ static const unsigned int _Py_STATX_KNOWN = (STATX_BASIC_STATS | STATX_BTIME
# define HAVE_READLINKAT_RUNTIME (readlinkat != NULL)
# endif

# ifdef _Py_HAVE_FREADLINK
# define HAVE_FREADLINK_RUNTIME (freadlink != NULL)
# endif

# ifdef HAVE_SYMLINKAT
# define HAVE_SYMLINKAT_RUNTIME (symlinkat != NULL)
# endif
Expand Down Expand Up @@ -10986,11 +10991,19 @@ os_unshare_impl(PyObject *module, int flags)
#endif



#if defined(HAVE_READLINK) || defined(MS_WINDOWS)

#if (defined(__linux__) || defined(__ANDROID__)) && defined(O_PATH)
// readlinkat(fd, "", ...) reads the symlink that fd refers to.
// supported since Linux 2.6.39 (same version that O_PATH was introduced).
#define _Py_READLINKAT_SUPPORTS_EMPTY_PATH
#endif

/*[clinic input]
os.readlink

path: path_t
path: path_t(allow_fd=True)
*
dir_fd: dir_fd(requires='readlinkat') = None

Expand All @@ -11002,45 +11015,90 @@ that directory.

dir_fd may not be implemented on your platform. If it is unavailable,
using it will raise a NotImplementedError.

On Linux, Android and MacOS, path may be a file descriptor referring to
a symlink. If it is, dir_fd must be None, and the return value will be a
bytes object. (File descriptors for symlinks can be obtained with

os.open(..., os.O_RDONLY | os.O_PATH | os.O_NOFOLLOW)

on Linux and Android, and

os.open(..., os.O_RDONLY | os.O_SYMLINK)

on MacOS.)
[clinic start generated code]*/

static PyObject *
os_readlink_impl(PyObject *module, path_t *path, int dir_fd)
/*[clinic end generated code: output=d21b732a2e814030 input=03d10130870dbca8]*/
/*[clinic end generated code: output=d21b732a2e814030 input=30272a2c5fba427c]*/
{
#if defined(HAVE_READLINK)
char buffer[MAXPATHLEN+1];
ssize_t length;
#ifdef HAVE_READLINKAT
int readlinkat_unavailable = 0;
#endif

Py_BEGIN_ALLOW_THREADS
if (path_and_dir_fd_invalid("readlink", path, dir_fd)) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message here isn't super descriptive - but I figured it was easier to just re-use the existing function. Let me know if you want a change.

return NULL;
}

if (path->is_fd) {
#if defined(_Py_HAVE_FREADLINK)
if (HAVE_FREADLINK_RUNTIME) {
Py_BEGIN_ALLOW_THREADS
length = freadlink(path->fd, buffer, MAXPATHLEN);
Py_END_ALLOW_THREADS
} else {
PyErr_SetString(PyExc_NotImplementedError,
"readlink cannot read file descriptors on this platform, "
"freadlink() is unavailable");
return NULL;
}
#elif defined(HAVE_READLINKAT) && defined(_Py_READLINKAT_SUPPORTS_EMPTY_PATH)
// linux/android:
// readlinkat(fd, "", ...) reads the link that fd refers to.
if (HAVE_READLINKAT_RUNTIME) {
Py_BEGIN_ALLOW_THREADS
length = readlinkat(path->fd, "", buffer, MAXPATHLEN);
Py_END_ALLOW_THREADS
} else {
// this should be unreachable:
// HAVE_READLINKAT_RUNTIME is always 1 on Linux/Android.
// Leaving it here as a safeguard.
PyErr_SetString(PyExc_NotImplementedError,
"readlink cannot read file descriptors on this platform, "
"readlinkat() is unavailable");
return NULL;
}
#else
PyErr_SetString(PyExc_NotImplementedError,
"readlink cannot read file descriptors on this platform");
return NULL;
Comment on lines +11073 to +11075

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the reason for having this twice (it's also on line 11113).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one below is in the windows branch. This one is inside the HAVE_READLINK branch of this function.

#endif
} else
#ifdef HAVE_READLINKAT
if (dir_fd != DEFAULT_DIR_FD) {
if (HAVE_READLINKAT_RUNTIME) {
Py_BEGIN_ALLOW_THREADS
length = readlinkat(dir_fd, path->narrow, buffer, MAXPATHLEN);
Py_END_ALLOW_THREADS
} else {
readlinkat_unavailable = 1;
argument_unavailable_error(NULL, "dir_fd");
return NULL;
}
} else
#endif
{
Py_BEGIN_ALLOW_THREADS
length = readlink(path->narrow, buffer, MAXPATHLEN);
Py_END_ALLOW_THREADS

#ifdef HAVE_READLINKAT
if (readlinkat_unavailable) {
argument_unavailable_error(NULL, "dir_fd");
return NULL;
Py_END_ALLOW_THREADS
}
#endif

if (length < 0) {
return path_error(path);
}
buffer[length] = '\0';

if (PyUnicode_Check(path->object))
if (path->is_fd || PyUnicode_Check(path->object))
return PyUnicode_DecodeFSDefaultAndSize(buffer, length);
else
return PyBytes_FromStringAndSize(buffer, length);
Expand All @@ -11052,6 +11110,12 @@ os_readlink_impl(PyObject *module, path_t *path, int dir_fd)
_Py_REPARSE_DATA_BUFFER *rdb = (_Py_REPARSE_DATA_BUFFER *)target_buffer;
PyObject *result = NULL;

if (path->is_fd) {
PyErr_SetString(PyExc_NotImplementedError,
"readlink cannot read file descriptors on this platform");
return NULL;
}

/* First get a handle to the reparse point */
Py_BEGIN_ALLOW_THREADS
reparse_point_handle = CreateFileW(
Expand Down Expand Up @@ -18881,6 +18945,10 @@ PROBE(probe_openat, HAVE_OPENAT_RUNTIME)
PROBE(probe_readlinkat, HAVE_READLINKAT_RUNTIME)
#endif

#ifdef _Py_HAVE_FREADLINK
PROBE(probe_freadlink, HAVE_FREADLINK_RUNTIME)
#endif

#ifdef HAVE_SYMLINKAT
PROBE(probe_symlinkat, HAVE_SYMLINKAT_RUNTIME)
#endif
Expand Down Expand Up @@ -18948,6 +19016,10 @@ static const struct have_function {
{ "HAVE_FPATHCONF", NULL },
#endif

#ifdef _Py_HAVE_FREADLINK
{ "HAVE_FREADLINK", probe_freadlink },
#endif

#ifdef HAVE_FSTATAT
{ "HAVE_FSTATAT", probe_fstatat },
#endif
Expand Down
Loading
Loading