Skip to content

Add dir_fd to os.path.lexists() & os.path.isdir() #117967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nineteendo opened this issue Apr 17, 2024 · 8 comments
Closed

Add dir_fd to os.path.lexists() & os.path.isdir() #117967

nineteendo opened this issue Apr 17, 2024 · 8 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@nineteendo
Copy link
Contributor

nineteendo commented Apr 17, 2024

Feature or enhancement

Proposal:

glob currently needs its own implementation of os.path.lexists() & os.path.isdir() to support dir_fd:

cpython/Lib/glob.py

Lines 201 to 222 in f74e512

def _lexists(pathname, dir_fd):
# Same as os.path.lexists(), but with dir_fd
if dir_fd is None:
return os.path.lexists(pathname)
try:
os.lstat(pathname, dir_fd=dir_fd)
except (OSError, ValueError):
return False
else:
return True
def _isdir(pathname, dir_fd):
# Same as os.path.isdir(), but with dir_fd
if dir_fd is None:
return os.path.isdir(pathname)
try:
st = os.stat(pathname, dir_fd=dir_fd)
except (OSError, ValueError):
return False
else:
return stat.S_ISDIR(st.st_mode)

We could refactor this by adding dir_fd to os.path.lexists() & os.path.isdir():

-def lexists(path):
+def lexists(path, *, dir_fd: int | None = None):
     """Test whether a path exists.  Returns True for broken symbolic links"""
     try:
-        os.lstat(path)
+        os.lstat(path, dir_fd=dir_fd)
     except (OSError, ValueError):
         return False
     return True
-def isdir(s):
+def isdir(s, *, dir_fd: int | None = None):
     """Return true if the pathname refers to an existing directory."""
     try:
-        st = os.stat(s)
+        st = os.stat(s, dir_fd=dir_fd)
     except (OSError, ValueError):
         return False
     return stat.S_ISDIR(st.st_mode)

Note: nt._path_isdir() (& nt._path_lexists() when #117842 lands) need to raise an error for this.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

@nineteendo nineteendo added the type-feature A feature request or enhancement label Apr 17, 2024
@erlend-aasland erlend-aasland added the stdlib Python modules in the Lib dir label Apr 17, 2024
@nineteendo
Copy link
Contributor Author

cc @serhiy-storchaka before I start implementing this: is this something you would support?

@serhiy-storchaka
Copy link
Member

It is trivially implemented via os.stat. glob implements private helpers for historical reasons -- they should behave exactly like os.path versions.

If there will be other uses of such functions in several different places, it will be worth to add this feature.

@nineteendo
Copy link
Contributor Author

nineteendo commented Apr 17, 2024

It is trivially implemented via os.stat.

Note that that's overkill on Windows. You're right because dir_fd isn't supported on Windows, sorry.

glob implements private helpers for historical reasons

They could still be simplified in that case:

def _lexists(pathname, dir_fd):
    # Same as os.path.lexists()
    return os.path.lexists(pathname, dir_fd=dir_fd)

def _isdir(pathname, dir_fd):
    # Same as os.path.isdir()
    return os.path.isdir(pathname, dir_fd=dir_fd)

@nineteendo
Copy link
Contributor Author

nineteendo commented Apr 17, 2024

@barneygale, do you think this can speed things up? No, it can't, but it could also be used for #117737.

@eryksun
Copy link
Contributor

eryksun commented Apr 17, 2024

Note: nt._path_isdir() (& nt._path_lexists() when #117842 lands) would also need to be updated.

The builtin _path_* functions on Windows would have to be modified to raise NotImplementedError if the new dir_fd argument isn't None.

FYI, the NT kernel supports opening relative to a handle, which the Windows API makes use of when opening relative to the working directory. However, it's not directly exposed in the Windows API. A fundamental problem is that ".." components are resolved logically on Windows by the user-mode runtime library. Thus using a safe open on NT requires that the normalized relative path doesn't begin with a ".." component.

@barneygale
Copy link
Contributor

I don't feel strongly about it. I note it would be the first os.path function to accept dir_fd.

@nineteendo
Copy link
Contributor Author

The builtin _path_* functions on Windows would have to be modified to raise NotImplementedError if the new dir_fd argument isn't None.

I see, I assumed Windows supported it too. Then it can only be used for refactoring glob.

I note it would be the first os.path function to accept dir_fd.

At the moment it only seems to be needed for glob, so I'll leave it up to serhiy to decide.

@nineteendo
Copy link
Contributor Author

I'm closing this as there's not much support. Feel free to re-open when you change your mind.

@nineteendo nineteendo closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants