Skip to content
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

bpo-1154351: Add get_current_dir_name() to os module #10117

Closed
wants to merge 9 commits into from
12 changes: 12 additions & 0 deletions Doc/library/os.rst
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ process and user.
.. function:: chdir(path)
fchdir(fd)
getcwd()
get_current_dir_name()
:noindex:

These functions are described in :ref:`os-file-dir`.
Expand Down Expand Up @@ -1698,6 +1699,17 @@ features:
Return a bytestring representing the current working directory.


.. function:: get_current_dir_name()

Return a string representing the current working directory taking into
consideration the users :envvar:`PWD` environment variable if it exists. This is
opposed to :func:`getcwd` which dereferences symlinks in the path. This
function is identical to :func:`getcwd` on systems that do **not**
support the :envvar:`PWD` environment variable.

.. versionadded:: 3.8


.. function:: lchflags(path, flags)

Set the flags of *path* to the numeric *flags*, like :func:`chflags`, but do
Expand Down
6 changes: 6 additions & 0 deletions Doc/whatsnew/3.8.rst
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ by right-clicking the button. (Contributed by Tal Einat in :issue:`1529353`.)
The changes above have been backported to 3.7 maintenance releases.


os
--

Added :func:`~os.get_current_dir_name` function.
(Contributed by Braden Groom in :issue:`1154351`.)

os.path
-------

Expand Down
22 changes: 21 additions & 1 deletion Lib/os.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
__all__ = ["altsep", "curdir", "pardir", "sep", "pathsep", "linesep",
"defpath", "name", "path", "devnull", "SEEK_SET", "SEEK_CUR",
"SEEK_END", "fsencode", "fsdecode", "get_exec_path", "fdopen",
"popen", "extsep"]
"popen", "extsep", "get_current_dir_name"]

def _exists(name):
return name in globals()
Expand Down Expand Up @@ -651,6 +651,26 @@ def get_exec_path(env=None):
return path_list.split(pathsep)


def get_current_dir_name():
"""
Return a string representing the current working directory taking into
consideration the users *PWD* environment variable if it exists. This
is opposed to getcwd() which dereferences symlinks in the path. This
function is identical to getcwd() on systems that do *not* support
the *PWD* environment variable.
"""
cwd = getcwd()
if name == 'nt':
Copy link
Member

Choose a reason for hiding this comment

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

Why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you were performing review as I was writing a comment :)
#10117 (comment)

This function is documented with:

This function is identical to :func:getcwd on systems that do not support the :envvar:PWD environment variable.

Please correct me if I'm wrong since I'm less familiar with Windows, but I don't believe it uses the PWD environment variable in any way. I added that check so that we would not incorrectly change behavior from getcwd() if the user does set the PWD on Windows for some reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

in git bash on windows:

Anthony@AnthonysDesktop MINGW64 ~
$ python -c 'import os; print(os.getenv("PWD"))'
C:/Users/Anthony

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asottile
Thanks. Looks like my memory was incorrect. I'll fix this up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually not sure this function is useful/a good idea -- it's entirely dependent on whether python is executed in the context of an interactive shell that sets or doesn't set PWD.

I've commented on the bpo issue indicating that as well.

return cwd
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this so that we don't look at the PWD environment variable on windows. I believe this matches the behavior documented.

try:
pwd = environ["PWD"]
except KeyError:
return cwd
if path.samefile(cwd, pwd):
return pwd
return cwd


# Change environ to automatically call putenv(), unsetenv if they exist.
from _collections_abc import MutableMapping

Expand Down
43 changes: 43 additions & 0 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import uuid
import warnings
from test import support
from unittest import mock

try:
import resource
Expand Down Expand Up @@ -1252,6 +1253,48 @@ def tearDownClass(cls):
os.rmdir(support.TESTFN)


class CurrentDirTests(unittest.TestCase):

def setUp(self):
base = os.path.abspath(support.TESTFN)
self.tmp_dir = base + '_dir'
self.tmp_lnk = base + '_lnk'

def test_getcwd(self):
# os.getcwd() always returns the dereferenced path
with support.temp_cwd(self.tmp_dir):
os.chdir(self.tmp_dir)
self.assertEqual(os.getcwd(), self.tmp_dir)
self.addCleanup(support.unlink, self.tmp_lnk)
os.symlink(self.tmp_dir, self.tmp_lnk, True)
os.chdir(self.tmp_lnk)
if os.name == 'nt':
# windows doesn't dereference the path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is a bug or the expected behavior. I haven't used windows in quite some time, but this is the behavior I'm seeing in CI.

expected_cwd = self.tmp_lnk
else:
expected_cwd = self.tmp_dir
self.assertEqual(os.getcwd(), expected_cwd)
with mock.patch.dict('os.environ', {'PWD': self.tmp_dir}):
self.assertEqual(os.getcwd(), expected_cwd)
with mock.patch.dict('os.environ', {'PWD': self.tmp_lnk}):
self.assertEqual(os.getcwd(), expected_cwd)

def test_get_current_dir_name(self):
# os.get_current_dir_name() returns the direct path--mirroring
# the PWD environment variable if it exists regardless of
# whether the path contains symlinks.
with support.temp_cwd(self.tmp_dir):
with mock.patch.dict('os.environ', {'PWD': self.tmp_dir}):
self.assertEqual(os.get_current_dir_name(), self.tmp_dir)
self.addCleanup(support.unlink, self.tmp_lnk)
os.symlink(self.tmp_dir, self.tmp_lnk, True)
with mock.patch.dict('os.environ', {'PWD': self.tmp_lnk}):
if os.name == 'posix':
self.assertEqual(os.get_current_dir_name(), self.tmp_lnk)
else:
self.assertEqual(os.get_current_dir_name(), self.tmp_dir)


class RemoveDirsTests(unittest.TestCase):
def setUp(self):
os.makedirs(support.TESTFN)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add get_current_dir_name() to the os module.
Copy link
Member

Choose a reason for hiding this comment

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

"Patch by ..."

Patch by Braden Groom