Skip to content

Some os and os.path function signatures do not admit legal integer (file descriptor) arguments #1653

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
gwk opened this issue Oct 6, 2017 · 7 comments

Comments

@gwk
Copy link
Contributor

gwk commented Oct 6, 2017

From the docs:

16.1.5. Files and Directories
On some Unix platforms, many of these functions support one or more of these features:

specifying a file descriptor: For some functions, the path argument can be not only a string giving a path name, but also a file descriptor. The function will then operate on the file referred to by the descriptor. (For POSIX systems, Python will call the f... version of the function.)

You can check whether or not path can be specified as a file descriptor on your platform using os.supports_fd. If it is unavailable, using it will raise a NotImplementedError.

Not sure if we want to widen the types for all platforms, or try to special case just for Posix. Here is a list of offenders (via suports_fd on python3.7): chdir, chmod, chown, listdir, pathconf, scandir, stat, statvfs, truncate, utime.

I also looked through os.path manually, and found one more: os.path.exists.

os.chdir(path)

Change the current working directory to path.
This function can support specifying a file descriptor. The descriptor must refer to an opened directory, not an open file.

typeshed: def chdir(path: _PathType) -> None: ...

os.chmod(path, mode, *, dir_fd=None, follow_symlinks=True)

New in version 3.3: Added support for specifying path as an open file descriptor, and the dir_fd and follow_symlinks arguments.

typeshed: def chmod(path: _PathType, mode: int) -> None: ...

os.chown(path, uid, gid, *, dir_fd=None, follow_symlinks=True)

New in version 3.3: Added support for specifying an open file descriptor for path, and the dir_fd and follow_symlinks arguments.

typeshed: def chown(path: _PathType, uid: int, gid: int) -> None: ... # Unix only

os.listdir(path=’.’)

New in version 3.3: Added support for specifying an open file descriptor for path.

typeshed:

@overload
def listdir(path: str = ...) -> List[str]: ...
@overload
def listdir(path: bytes) -> List[bytes]: ...

os.pathconf(path, name)

This function can support specifying a file descriptor.

typeshed: def pathconf(path: _PathType, name: Union[str, int]) -> int: ... # Unix only

os.stat(path, *, dir_fd=None, follow_symlinks=True)

New in version 3.3: Added the dir_fd and follow_symlinks arguments, specifying a file descriptor instead of a path.

typeshed: def stat(path: _PathType) -> stat_result: ...

os.statvfs(path)

New in version 3.3: Added support for specifying an open file descriptor for path.

typeshed: def statvfs(path: _PathType) -> statvfs_result: ... # Unix only

os.truncate(path, length)

This function can support specifying a file descriptor.

typeshed: def truncate(path: Union[_PathType, int], length: int) -> None: ... # Unix only up to version 3.4

os.utime(path, times=None, *, [ns, ]dir_fd=None, follow_symlinks=True)

New in version 3.3: Added support for specifying an open file descriptor for path, and the dir_fd, follow_symlinks, and ns parameters.

typeshed: def utime(path: _PathType, times: Optional[Union[Tuple[int, int], Tuple[float, float]]] = ...,

os.path.exists(path)

Return True if path refers to an existing path or an open file descriptor.

typeshed: def exists(path: _PathType) -> bool: ...

@JelleZijlstra
Copy link
Member

#1645 is fixing a number of these. Can you check for this again after that PR is merged?

@gwk
Copy link
Contributor Author

gwk commented Oct 10, 2017

I just read through the #1645 patch and it looks like the following still need to be addressed: os.listdir, os.path.exist.

mkurek pushed a commit to mkurek/typeshed that referenced this issue Oct 20, 2017
Fix python#1653
Use `_FdOrPathType` when possible in `os.path` module.
mkurek pushed a commit to mkurek/typeshed that referenced this issue Oct 20, 2017
Fix python#1653
Use `_FdOrPathType` when possible in `os.path` module.
@gvanrossum
Copy link
Member

From a typing correctness POV it's really a shame that these functions were made polymorphic. The use cases for FDs here are very esoteric and it means that some buggy code will now no longer be caught by mypy.

I definitely don't endorse doing this to anything in os.path -- that's a coincidence and may break at any time.

Also, this should not typecheck when platform isn't one of the UNIXoid systems. (OK, let's say it should not typecheck on Windows. :-)

@gwk
Copy link
Contributor Author

gwk commented Oct 20, 2017

@gvanrossum I'm sorry but I don't follow. os.path.exists clearly documents the file descriptor behavior as a major version change - how is it a coincidence that could break?

@gvanrossum
Copy link
Member

Sorry I did not realize that it was documented. And it seems to be cross-platform too.

@JelleZijlstra
Copy link
Member

To be clear, os.path.exists (https://docs.python.org/3/library/os.path.html#os.path.exists) documents supports for fds, but other functions, like os.path.isfile, do not.

@gwk
Copy link
Contributor Author

gwk commented Jun 28, 2018

os.listdir at least now appears to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants