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

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

Closed
saturnmimas mannequin opened this issue Jul 22, 2008 · 16 comments
Closed

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

saturnmimas mannequin opened this issue Jul 22, 2008 · 16 comments
Assignees
Labels
topic-unicode type-bug An unexpected behavior, bug, or error

Comments

@saturnmimas
Copy link
Mannequin

saturnmimas mannequin commented Jul 22, 2008

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
    Copy link
    Mannequin Author

    saturnmimas mannequin commented Jul 22, 2008

    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/.'

    @saturnmimas saturnmimas mannequin added topic-unicode type-bug An unexpected behavior, bug, or error labels Jul 22, 2008
    @amauryfa
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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 ezio-melotti self-assigned this Jan 12, 2010
    @ezio-melotti
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Member

    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
    Copy link
    Contributor

    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
    Copy link
    Member

    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
    Copy link
    Member

    Passes on Windows, Mac, and Linux.

    @florentx
    Copy link
    Mannequin

    florentx mannequin commented Feb 1, 2010

    Small note:

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

    @briancurtin
    Copy link
    Member

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

    @florentx
    Copy link
    Mannequin

    florentx mannequin commented Feb 20, 2010

    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
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-unicode type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants