Issue 37412: os.getcwdb() doesn't implement PEP 528 (UTF-8) on Windows

On Windows, os.getcwdb() is implemented using getcwd() on Windows. This function uses the ANSI code page, whereas PEP 529 "Change Windows filesystem encoding to UTF-8" is supposed to use UTF-8 for all bytes paths and filenames. Moreover, this function emits a DeprecationWarning, whereas PEP 529 was supposed to avoid the need to deprecated bytes paths and filenames on Windows.

I guess that it was forgotten in the implementation of the PEP 529. Or was it a deliberate choice?

Attached PR modify os.getcwdb() to use UTF-8.

My PR 14396 modify os.getcwdb() to use UTF-8 and it removes the deprecation warning.

See also bpo-32920.

Definitely just an oversight. Thanks for fixing this!

I think it can go to 3.8 as well. Thoughts?

> I think it can go to 3.8 as well. Thoughts?

Oh, 3.8 is not released yet. Well, it's more or less a new feature, or a bugfix, depending on the point of view.

I would not be comfortable to change the encoding in 3.8.1, but it should be fine to do the change in 3.8.

I retargeted my PR 14396 to Python 3.8.

New changeset 689830ee6243126798a6c519c05aa11ba73db7cd by Victor Stinner in branch 'master':
bpo-37412: os.getcwdb() now uses UTF-8 on Windows (GH-14396)
https://github.com/python/cpython/commit/689830ee6243126798a6c519c05aa11ba73db7cd

New changeset 63429c839b19c6fe604deddcb12497443ca01a9a by Miss Islington (bot) in branch '3.8':
bpo-37412: os.getcwdb() now uses UTF-8 on Windows (GH-14396)
https://github.com/python/cpython/commit/63429c839b19c6fe604deddcb12497443ca01a9a

Ok, it's merged into 3.8. Thanks for the review Steve. One less deprecation warning.

This update breaks long-path support in Windows. It includes the following unnecessary check, which is using the wrong comparison operator:

        if (len >= PY_SSIZE_T_MAX / sizeof(wchar_t))

PyMem_RawMalloc already checks this and returns NULL if size > (size_t)PY_SSIZE_T_MAX. This bug is causing a MemoryError with long paths:

    >>> p = 'C:/Temp/longpath' + ('/' + 'a' * 255) * 9
    >>> os.chdir(p)
    >>> len(os.getcwd())
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    MemoryError

> This update breaks long-path support in Windows.

Oops.

This is the risk of not testing a feature :-) My PR added very basic tests for getcwd() and getcwdb(). I wrote PR 14424 to not only fix the integer overflow check, but also to add a new unit tests for "long path".

New changeset ec3e20a2d1edddb0558f9d32e2b367904ccdde88 by Victor Stinner in branch 'master':
bpo-37412: Fix os.getcwd() for long path on Windows (GH-14424)
https://github.com/python/cpython/commit/ec3e20a2d1edddb0558f9d32e2b367904ccdde88

New changeset 64580da33122a10aef75c76aa3ff87c0ee11e3d7 by Victor Stinner in branch 'master':
bpo-37412: pythoninfo: add Windows long paths (GH-14434)
https://github.com/python/cpython/commit/64580da33122a10aef75c76aa3ff87c0ee11e3d7

New changeset 68c1c398f3534af16309a86ab815b61d2487e6db by Victor Stinner (Miss Islington (bot)) in branch '3.8':
bpo-37412: Fix os.getcwd() for long path on Windows (GH-14424) (GH-14451)
https://github.com/python/cpython/commit/68c1c398f3534af16309a86ab815b61d2487e6db

Ok, the regression should now be fixed and test_os now also tests os.getcwd() with a "long path" :-) Thank a lot Eryk Sun for your great advices on reviews!

Oh. Now the test fails on macOS:

https://buildbot.python.org/all/#/builders/145/builds/2021
======================================================================
FAIL: test_getcwd_long_path (test.test_os.MiscTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.billenstein-sierra/build/Lib/test/test_os.py", line 115, in test_getcwd_long_path
    self.assertEqual(cwd, expected)
AssertionError: '/private/var/folders/sy/9hwmqyx14s11577cvgzwf2v40000gt/T/tmpchlt91ay' != '/var/folders/sy/9hwmqyx14s11577cvgzwf2v40000gt/T/tmpchlt91ay'
- /private/var/folders/sy/9hwmqyx14s11577cvgzwf2v40000gt/T/tmpchlt91ay
? --------
+ /var/folders/sy/9hwmqyx14s11577cvgzwf2v40000gt/T/tmpchlt91ay

Fail on FreeBSD as well:

https://buildbot.python.org/all/#/builders/168/builds/1222

======================================================================
FAIL: test_getcwd_long_path (test.test_os.MiscTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/home/buildbot/python/3.x.koobs-freebsd-current/build/Lib/test/test_os.py", line 115, in test_getcwd_long_path
    self.assertEqual(cwd, expected)
AssertionError: '/var/tmp/tmpq45z_od3' != '/tmp/tmpq45z_od3'
- /var/tmp/tmpq45z_od3
? ----
+ /tmp/tmpq45z_od3

New changeset 29f609ed07aa7856741ff8b8b8b050369d0faf81 by Victor Stinner in branch 'master':
bpo-37412: Fix test_os.test_getcwd_long_path() on macOS (GH-14452)
https://github.com/python/cpython/commit/29f609ed07aa7856741ff8b8b8b050369d0faf81

New changeset e3761ca91cda86462206d60244078f05c636dd13 by Miss Islington (bot) in branch '3.8':
bpo-37412: Fix test_os.test_getcwd_long_path() on macOS (GH-14452)
https://github.com/python/cpython/commit/e3761ca91cda86462206d60244078f05c636dd13

This is fixed now, right? The buildbots seem happy at least. I'll close.

> This is fixed now, right? The buildbots seem happy at least. I'll close.

Yeah, I forgot to close. I got some issues with backports, but the two commits are now merged into 3.8 as well.

assignee: vstinner
resolution: fixed
stage: patch review -> resolved

stage: patch review -> resolved

resolution: fixed
stage: patch review -> resolved