classification
Title: os.getcwdb() doesn't implement PEP 528 (UTF-8) on Windows
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: vstinner Nosy List: ZackerySpytz, eryksun, miss-islington, steve.dower, vstinner
Priority: normal Keywords: patch

Created on 2019-06-26 14:28 by vstinner, last changed 2019-07-01 16:30 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 14396 merged vstinner, 2019-06-26 14:32
PR 14399 merged miss-islington, 2019-06-26 15:31
PR 14424 merged vstinner, 2019-06-27 15:14
PR 14434 merged vstinner, 2019-06-27 22:37
PR 14451 merged miss-islington, 2019-06-28 16:02
PR 14452 merged vstinner, 2019-06-28 16:30
PR 14454 merged miss-islington, 2019-06-28 17:47
Messages (21)
msg346620 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 14:28
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.
msg346621 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 14:36
My PR 14396 modify os.getcwdb() to use UTF-8 and it removes the deprecation warning.
msg346622 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-06-26 14:39
See also bpo-32920.
msg346626 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-06-26 14:53
Definitely just an oversight. Thanks for fixing this!

I think it can go to 3.8 as well. Thoughts?
msg346628 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 15:00
> 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.
msg346629 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 15:03
I retargeted my PR 14396 to Python 3.8.
msg346636 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 15:31
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
msg346648 - (view) Author: miss-islington (miss-islington) Date: 2019-06-26 16:14
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
msg346672 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-26 19:42
Ok, it's merged into 3.8. Thanks for the review Steve. One less deprecation warning.
msg346708 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-06-27 01:10
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
msg346743 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-27 15:18
> 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".
msg346825 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:02
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
msg346826 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:05
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
msg346832 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:23
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
msg346833 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:24
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!
msg346834 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:30
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
msg346835 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 16:32
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
msg346843 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-06-28 17:39
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
msg346847 - (view) Author: miss-islington (miss-islington) Date: 2019-06-28 18:07
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
msg347037 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-07-01 16:15
This is fixed now, right? The buildbots seem happy at least. I'll close.
msg347041 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-07-01 16:30
> 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.
History
Date User Action Args
2019-07-01 16:30:21vstinnersetmessages: + msg347041
2019-07-01 16:15:37steve.dowersetstatus: open -> closed
messages: + msg347037

assignee: vstinner
resolution: fixed
stage: patch review -> resolved
2019-06-28 18:07:15miss-islingtonsetmessages: + msg346847
2019-06-28 17:47:50miss-islingtonsetpull_requests: + pull_request14271
2019-06-28 17:39:51vstinnersetmessages: + msg346843
2019-06-28 16:32:24vstinnersetmessages: + msg346835
2019-06-28 16:30:58vstinnersetstage: resolved -> patch review
pull_requests: + pull_request14268
2019-06-28 16:30:50vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg346834
2019-06-28 16:24:04vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg346833

stage: patch review -> resolved
2019-06-28 16:23:10vstinnersetmessages: + msg346832
2019-06-28 16:05:08vstinnersetmessages: + msg346826
2019-06-28 16:02:19miss-islingtonsetpull_requests: + pull_request14267
2019-06-28 16:02:03vstinnersetmessages: + msg346825
2019-06-27 22:37:29vstinnersetstage: resolved -> patch review
pull_requests: + pull_request14251
2019-06-27 15:18:19vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg346743
2019-06-27 15:14:59vstinnersetpull_requests: + pull_request14240
2019-06-27 01:10:01eryksunsetmessages: + msg346708
2019-06-26 19:42:08vstinnersetstatus: open -> closed
versions: + Python 3.9
messages: + msg346672

resolution: fixed
stage: patch review -> resolved
2019-06-26 16:14:37miss-islingtonsetnosy: + miss-islington
messages: + msg346648
2019-06-26 15:31:50miss-islingtonsetpull_requests: + pull_request14213
2019-06-26 15:31:24vstinnersetmessages: + msg346636
2019-06-26 15:03:06vstinnersetmessages: + msg346629
versions: + Python 3.8, - Python 3.9
2019-06-26 15:00:23vstinnersetmessages: + msg346628
2019-06-26 14:53:50steve.dowersetmessages: + msg346626
2019-06-26 14:40:17vstinnersetnosy: + eryksun
2019-06-26 14:40:11vstinnerlinkissue32920 superseder
2019-06-26 14:39:17ZackerySpytzsetnosy: + ZackerySpytz
messages: + msg346622
2019-06-26 14:36:50vstinnersetmessages: + msg346621
2019-06-26 14:32:20vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request14210
2019-06-26 14:28:07vstinnercreate