Repository navigation
Add follow_symlinks to os.path.exists() #117705
Description
Activity
- addedtype-featureA feature request or enhancementA feature request or enhancement
on Apr 10, 2024 - addedstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory
on Apr 10, 2024 [...] Also, it looks like
pathlib.Pathdoesn't have anlexists()function.See previous discussion:
Reacted by Nice ZombiesOK, thanks for the information. I won't be pushing for that then.
Reacted by Erlend E. AaslandOK, thanks for the information. I won't be pushing for that then.
It is useful to search the tracker, Discourse, and the old python-ideas/python-dev mailing list archives before submitting feature requests.
BTW, I would encourage you to take a good look at the devguide; it is a good read. Suggesting to start with the lifecycle of a pull request, and especially the subsection about making good PRs.
It is useful to search the tracker, Discourse, and the old python-ideas/python-dev mailing list archives before submitting feature requests.
It's indeed useful, it looks like this functionality was rejected for
os.path.isdir(): #57180, but this was later added to pathlib without mention of it: #105793. I guess we could then also reopen the discussing about addingfollow_symlinkstoos.path.isdir()&os.path.isfile().I guess we could then also reopen the discussing about adding
follow_symlinkstoos.path.isdir()&os.path.isfile().The place for such a discussion would be Discourse. Iff there is consensus, follow up with an issue on the tracker.
BTW, I would encourage you to take a good look at the devguide; it is a good read. Suggesting to start with the lifecycle of a pull request, and especially the subsection about making good PRs.
I have printed the latter, but I have 2 questions:
- How can I run tests on Windows? I don't have the
makecommand. - How am I supposed to configure my linter in VS Code? Autopep8 & isort are way too strict and I had to turn them off. It seems like only a trailing newline is required.
- How can I run tests on Windows? I don't have the
- How can I run tests on Windows? I don't have the
makecommand.
You'll find instructions both for building and running tests in the devguide.
- How am I supposed to configure my linter in VS Code? Autopep8 & isort are way too strict and I had to turn them off. It seems like only a trailing newline is required.
I have no idea; I don't use VS Code :) I think you are better off asking that question in a VS Code forum.
- How can I run tests on Windows? I don't have the
On Windows, the
os.pathfunctionsexists(),isfile(),isdir(), andislink()are implemented in "Modules/posixmodule.c" as the correspondingos__path_*_impl()builtin functions. We never got around to implementingisjunction()as a builtin. I don't recall whylexists()wasn't implemented as a builtin.In the context of stat queries, a name-surrogate reparse point is considered a 'symlink'. The
GetFileInformationByName()fast path will have to be modified to use the result when not following symlinks if it's a name-surrogate reparse point. The slow path will have to callSTAT()when not following symlinks, orwin32_xstat_slow_impl()could be refactored into reusable helper functions that encapsulate the design decisions and logic.Reacted by Nice ZombiesIs this something you would be willing to make a pull request for?
Alternatively, we could make
ntpath.exists()a wrapper fornt._path_exists()&genericpath.lexists(). Then we wouldn't need to change the C code.That works, if you're averse to programming in C, assuming the wrapper doesn't significantly hurt the performance of
exists(). Otherwise, I'd rather have a fast path forlexists(). It's currently a slower path based on a fullos.lstat(). If_path_exists()supportsfollow_symlinks, we can trivially implement builtin_path_lexists()viaos__path_exists_impl().There's around 1% hit on performance. We have better things to do than optimise that using C code.
Let's leave implementing_path_lexists()for another time.FWIW, here's code to extend the implementation of
nt._path_exists()in "Modules/posixmodule.c" to supportfollow_symlinksand to implementnt._path_lexists(). You have to manually addOS__PATH_LEXISTS_METHODDEFto theposix_methodsarray. Then runTools/clinic/clinic.py Modules/posixmodule.c, and rebuild Python./*[clinic input] os._path_exists path: 'O' Path to be tested; can be a string, bytes object, a path-like object, or an integer that references an open file descriptor. follow_symlinks: bool = True If False, and the last element of the path is a symbolic link, do not test the target of the symbolic link. Test whether a path exists. Returns False for broken symbolic links [clinic start generated code]*/ static PyObject * os__path_exists_impl(PyObject *module, PyObject *path, int follow_symlinks) /*[clinic end generated code: output=f31130f636d45dcc input=26779f82ae9d4f5a]*/ { path_t _path = PATH_T_INITIALIZE("exists", "path", 0, 1); HANDLE hfile; int result = 0; if (!path_converter(path, &_path)) { path_cleanup(&_path); if (PyErr_ExceptionMatches(PyExc_ValueError)) { PyErr_Clear(); Py_RETURN_FALSE; } return NULL; } Py_BEGIN_ALLOW_THREADS if (_path.fd != -1) { hfile = _Py_get_osfhandle_noraise(_path.fd); if (hfile != INVALID_HANDLE_VALUE) { result = 1; } } else if (_path.wide) { BOOL slow_path = TRUE; FILE_STAT_BASIC_INFORMATION statInfo; if (_Py_GetFileInformationByName(_path.wide, FileStatBasicByNameInfo, &statInfo, sizeof(statInfo))) { if (!(statInfo.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) || !follow_symlinks && IsReparseTagNameSurrogate(statInfo.ReparseTag)) { slow_path = FALSE; result = 1; } } else if (_Py_GetFileInformationByName_ErrorIsTrustworthy( GetLastError())) { slow_path = FALSE; } if (slow_path) { STRUCT_STAT st; if (!follow_symlinks) { if (!LSTAT(_path.wide, &st)) { result = 1; } } else { hfile = CreateFileW(_path.wide, FILE_READ_ATTRIBUTES, 0, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); if (hfile != INVALID_HANDLE_VALUE) { CloseHandle(hfile); result = 1; } else { switch (GetLastError()) { case ERROR_ACCESS_DENIED: case ERROR_SHARING_VIOLATION: case ERROR_CANT_ACCESS_FILE: case ERROR_INVALID_PARAMETER: if (!STAT(_path.wide, &st)) { result = 1; } } } } } } Py_END_ALLOW_THREADS path_cleanup(&_path); if (result) { Py_RETURN_TRUE; } Py_RETURN_FALSE; } /*[clinic input] os._path_lexists path: 'O' Path to be tested; can be a string, bytes object, a path-like object, or an integer that references an open file descriptor. Test whether a path exists. Returns True for broken symbolic links [clinic start generated code]*/ static PyObject * os__path_lexists_impl(PyObject *module, PyObject *path) /*[clinic end generated code: output=b9a42a50b1df6651 input=96bc06e253ad0cd7]*/ { return os__path_exists_impl(module, path, 0); }
Reacted by Nice ZombiesThanks, that seems to work. Is there a reason you don't make pull requests?
There is an old design rule -- rather than adding a boolean parameter to a function, it is better to add a new function. It does not always work, especially if there are several boolean parameters, but Python usually follows it.
os.path.exists()andos.path.lexists()are simple functions, and there is no need to complicateos.path.exists()to makeos.path.lexists()from it.Reacted by Erlend E. AaslandThen what about
,pathlib.Path.exists()os.stat()&os.chmod()? I don't see how this is any different from these functions.
If anything, it's inconsistent.I don't see how this is any different from these functions.
pathlibis a high-level API.os.pathis a low-level API.And
os.stat()&os.chmod()then? Those are a low-level API and yet they also have two ways of doing it.The
follow_symlionksparameter was added toos.stat()and otherosfunctions when many other optional parameters were added. This is the case when this rule is not applicable.I do not see the need for this change. Nobody asked for it. It seems that the only reason for this change is that you can implement it.
Actually, the correct answer is that this was used for refactoring (otherwise just having
lstat()would've been fine): 9cf065c.
But that's not applicable here.I'm happy to help if you want to use the code that I posted to make a pull request to implement
_path_lexists()to accelerateos.path.lexists()and clean up the implementation of_path_exists(), which is currently a bit convoluted. The implementation in "Modules/posixmodule.c" would just move into a common_path_exists()function that's called byos__path_exists_impl()andos__path_lexists_impl(), without exposing a newfollow_symlinksparameter foros.path.exists().
Feature or enhancement
Proposal:
Because we have
os.path.exists()&os.path.lexists(), it might make sense to add afollow_symlinksargument to the former:This matches
pathlib.Path.exists()(as well asos.stat()&os.chmod()).Also, it looks like(but that's less explicit and so unnecessary, see #21157 (comment))pathlib.Pathdoesn't have anlexists()function.Note: this argument also needs to be added toDone.nt._path_exists().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
Linked PRs
follow_symlinkstoos.path.exists()#117729