os.path.abspath with unicode argument should call os.getcwdu

[saturnmimas]

BPO	3426
Nosy	@ronaldoussoren, @amauryfa, @ezio-melotti, @briancurtin, @florentx
Superseder	bpo-7669: test_unicode_file fails with non-ascii path
Files	issue3426.diff: Patch that uses os.getcwdu() instead of os.getcwd() when the arg of abspath is unicode + unittests + helper context manager issue3426-2.diff: Added fix for ntpath, refactored the tests issue3426-3.diff: Fix for posixpath, ntpath, macpath, and os2emxpath issue3426_any_cwd.diff: Patch, apply to trunk

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ezio-melotti'
closed_at = <Date 2010-02-21.11:26:27.556>
created_at = <Date 2008-07-22.14:32:13.090>
labels = ['type-bug', 'expert-unicode']
title = 'os.path.abspath with unicode argument should call os.getcwdu'
updated_at = <Date 2010-02-21.11:26:27.555>
user = 'https://bugs.python.org/saturnmimas'

bugs.python.org fields:

activity = <Date 2010-02-21.11:26:27.555>
actor = 'ezio.melotti'
assignee = 'ezio.melotti'
closed = True
closed_date = <Date 2010-02-21.11:26:27.556>
closer = 'ezio.melotti'
components = ['Unicode']
creation = <Date 2008-07-22.14:32:13.090>
creator = 'saturn_mimas'
dependencies = []
files = ['15847', '15867', '15902', '16267']
hgrepos = []
issue_num = 3426
keywords = ['patch', 'needs review', 'buildbot']
message_count = 16.0
messages = ['70148', '70151', '90101', '97645', '97676', '97680', '97752', '97799', '97801', '97806', '97861', '98044', '98650', '98665', '99623', '99651']
nosy_count = 7.0
nosy_names = ['ronaldoussoren', 'aimacintyre', 'amaury.forgeotdarc', 'ezio.melotti', 'saturn_mimas', 'brian.curtin', 'flox']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = '7669'
type = 'behavior'
url = 'https://bugs.python.org/issue3426'
versions = ['Python 2.6', 'Python 2.7']

[saturnmimas]

If current working directory contains non-ascii characters, calling
os.path.abspath(u".") will result in an error. I expect it to call the
underlying os.getcwdu() in this case.

>>> import os
>>> os.path.abspath(u".")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File
"/home/packages/python-2.5.1/x86-linux/lib/python2.5/posixpath.py", line
403, in abspath
    path = join(os.getcwd(), path)
  File
"/home/packages/python-2.5.1/x86-linux/lib/python2.5/posixpath.py", line
65, in join
    path += '/' + b
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe4 in position 29:
ordinal not in range(128)

It works if I do it manually, using os.getcwdu():

>>> os.path.join(os.getcwdu(), u".")
u'/disk1/chn_local/work/test/sk\xe4rg\xe5rds\xf6-latin1/.'

[amauryfa]

Well, os.path.supports_unicode_filenames is False on posix platforms.
So os.path.abspath is not supposed to work with unicode values.

I was about to close the issue, but python 3.0 also defines
supports_unicode_filenames to False! In addition, os.getcwd() fails when
the current dir has non-ascii characters. Should we drop its
implementation, and use os.getcwdu instead?

[ezio-melotti]

This seems to work fine with Py 3.0 and 3.1 on Linux, it still fails
with Py 2.6 and 2.7.

[ezio-melotti]

This also caused the failure in bpo-7669.
I think that the functions in os.path are supposed to return unicode when they get unicode, so I agree that os.getcwdu should be used instead.
I'm not sure about os.path.supports_unicode_filenames though.

[ezio-melotti]

Here is a patch that uses os.getcwdu() instead of os.getcwd() when the arg of abspath is unicode (with tests).

It also include an helper context manager in test_support used to create temp dirs and set them as cwd (this will be committed separately) and two helper methods (assertUnicode and assertStr) that will probably be useful when I (or someone else) will add more tests with unicode strings for the other functions.

[briancurtin]

You could use assertIsInstance(s, unicode, '%r is not unicode' % s) in the tests instead of your assertTrue.

I think the rest of it looks good. Works for me.

[ezio-melotti]

I added the fix on ntpath and more tests. I also refactored the tests I added for posixpath.
I don't know what is the situation of os2emxpath and macpath and if they should be fixed too. The code for abspath is currently the same on all 4 the modules but I don't see any easy way to avoid the duplication and move it to genericpath because they call the module-dependent join and normpath.

[briancurtin]

assertStr and assertUnicode don't exist in test_ntpath so the tests fail on Windows. I copied/pasted the functions over from test_posixpath just to see and test_ntpath passes. Other than that, it looks good to me.

[ezio-melotti]

I'll fix the patch. I added Ronald and Andrew to see if they have any opinion about macpath and os2emxpath. The main idea is that abspath should always return unicode if its arg is unicode or str otherwise.

[ronaldoussoren]

abspath is basically dead code in macpath, the module is used to manipulate classic MacOS9-style paths and is no longer used as os.path on any supported platform (it used to be os.path on MacOS9).

BTW. the module itself is not dead, OSX still uses OS9-style paths in a number of older APIs.

[ezio-melotti]

For consistency I updated all 4 the modules. If the tests pass on both Windows and Mac the patch should be ready to go in.

[briancurtin]

Passes on Windows, Mac, and Linux.

[florentx]

Small note:

• the unit tests could use assertIsInstance(..., str), assertIsInstance(..., unicode) instead of the custom methods.
• some "assertTrue" may be replaced by "assertEqual"

[briancurtin]

The patch intentionally doesn't use assertIsInstance because that method doesn't exist in 2.6.

[florentx]

The attached patch proposes a decorator which can be used to strengthen any test which is affected by the CWD. (and fixes for test_(mac|nt|posix)path too)

[ezio-melotti]

Fixed in r78247 (trunk) and r78248 (release26-maint) (plus a fix in r78272 and r78279 to avoid test failures when the filesystem encoding is ascii).
I didn't use the any_cwd decorator -- I might consider it in future if it turns out that there are more tests like these.