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

Deprecate usage of the Windows ANSI API in the nt module #57583

Closed
vstinner opened this issue Nov 9, 2011 · 17 comments
Closed

Deprecate usage of the Windows ANSI API in the nt module #57583

vstinner opened this issue Nov 9, 2011 · 17 comments
Labels
stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

vstinner commented Nov 9, 2011

BPO 13374
Nosy @loewis, @mhammond, @vstinner, @florentx
Files
  • deprecate_win_bytes_api.patch
  • deprecate_win_bytes_api-2.patch
  • 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 = None
    closed_at = <Date 2011-11-16.22:44:16.802>
    created_at = <Date 2011-11-09.00:10:06.375>
    labels = ['library']
    title = 'Deprecate usage of the Windows ANSI API in the nt module'
    updated_at = <Date 2011-11-16.22:44:16.801>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2011-11-16.22:44:16.801>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2011-11-16.22:44:16.802>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2011-11-09.00:10:06.375>
    creator = 'vstinner'
    dependencies = []
    files = ['23640', '23647']
    hgrepos = []
    issue_num = 13374
    keywords = ['patch']
    message_count = 17.0
    messages = ['147323', '147325', '147326', '147327', '147356', '147357', '147371', '147372', '147373', '147529', '147537', '147582', '147709', '147710', '147713', '147731', '147775']
    nosy_count = 7.0
    nosy_names = ['loewis', 'mhammond', 'vstinner', 'flox', 'santoso.wijaya', 'python-dev', 'sbt']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue13374'
    versions = ['Python 3.3']

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    Attached patch deprecates the Windows ANSI API (bytes API) in the nt module. Use Unicode filenames instead of bytes filenames to not depend on the ANSI code page anymore and to support any Unicode filename.

    The patch changes also os.link(), os.rename() and os.symlink() to not accept two filenames of different types: require two Unicode filenames or two bytes filenames. It is an expected change, it did it to simplify the source code. I change it if necessary.

    @vstinner vstinner added the stdlib Python modules in the Lib dir label Nov 9, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 9, 2011

    New changeset 6bf07db23445 by Victor Stinner in branch 'default':
    Issue bpo-13374: Use Unicode filenames instead of bytes filenames
    http://hg.python.org/cpython/rev/6bf07db23445

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    The patch deprecates bytes filenames for the following functions:

    nt._getfullpathname
    nt._isdir
    os.access
    os.chdir
    os.chmod
    os.link
    os.listdir
    os.lstat
    os.mkdir
    os.open
    os.rename
    os.rmdir
    os.stat
    os.symlink
    os.unlink
    os.utime

    Oh, I forgot a test for os.open(bytes).

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    Functions like os.execv() or os.readlink() are not deprecated because the underlying C function really uses a bytes API (execv and readlink).

    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Nov 9, 2011

    Functions like os.execv() or os.readlink() are not deprecated because
    the underlying C function really uses a bytes API (execv and readlink).

    Probably os.execv() should be implemented on Windows with _wexecv() instead of _execv(). Likewise for other functions which have "wide" versions. Or maybe it wouldn't be worth the effort, since it would mean writing separate Windows implementations.

    I don't know what you mean about os.readlink() though: the Windows implementation uses CreateFileW() and DeviceIoControl().

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    Probably os.execv() should be implemented on Windows with _wexecv() instead
    of _execv().

    That's a different story. Would you like to implement it? If yes, please open a
    new issue.

    I don't know what you mean about os.readlink() though: the Windows
    implementation uses CreateFileW() and DeviceIoControl().

    Oops, you are right. The Windows implement only accepts Unicode, so no
    deprecation warning is needed here.

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    Updated patch:

    • os.rename(), os.symlink(), os.link() accept (bytes, str) and (str, bytes) again
    • ensure that the warning is emited after parsing arguments, not before (to not emit a warning if an int is passed instead of bytes or str)
    • add a test on os.open()

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 9, 2011

    Probably os.execv() should be implemented on Windows with _wexecv()
    instead of _execv(). Likewise for other functions which have "wide"
    versions. Or maybe it wouldn't be worth the effort, since it would
    mean writing separate Windows implementations.

    Writing separate Windows versions has a long tradition in posixmodule.c,
    so in principle it's fine. It still may not be worth the effort since
    the function is deprecated in favor of the subprocess module. However,
    if code was contributed in that direction, we would likely accept it.

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 9, 2011

    deprecate_win_bytes_api-2.patch:

    • test_os.py: catch_warning() should be moved into test_link_bytes()
    • the change on Py_GetFinalPathNameByHandleA may be done in another commit

    @sbt
    Copy link
    Mannequin

    sbt mannequin commented Nov 12, 2011

    I notice that the patch changes rename() and link() to use
    win32_decode_filename() to coerce the filename to unicode before using
    the "wide" win32 api. (Previously, rename() first tried the wide api,
    falling back to narrow if that failed; link() used wide if the args were
    both unicode, narrow otherwise. Some other functions like symlink()
    already only use the wide api.)

    Is this approach of coercing to unicode and only using the wide api
    "blessed"? I certainly think it should be. If so then one can get
    rid lots windows specific code.

    And are we able to assume that on Windows we have access to wide libc
    functions? _wcsicmp(), _snwprintf(), _wputenv() are all used already,
    so I guess we already make that assumption. It looks like a lot of the
    windows specific code attempts to reimplement basic libc functions using
    the win32 api just to support unicode - presumably there was a time when
    we could not assume that wide libc functions would be available. Other functions like execv() and spawnv() were never given unicode support.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 13, 2011

    Is this approach of coercing to unicode and only using the wide api
    "blessed"?

    It's not. If people use byte strings, they specifically ask for what
    they get; Python shouldn't second-guess the data types.

    I certainly think it should be. If so then one can get
    rid lots windows specific code.

    How so? This entire handling of file names is windows specific;
    dealing with different file name data types doesn't make it more
    windows specific than it already is.

    And are we able to assume that on Windows we have access to wide libc
    functions?

    Yes, but Python should avoid using them.

    _wcsicmp(), _snwprintf(), _wputenv() are all used already,
    so I guess we already make that assumption. It looks like a lot of the
    windows specific code attempts to reimplement basic libc functions using
    the win32 api just to support unicode - presumably there was a time when
    we could not assume that wide libc functions would be available.

    No:
    a) we try to get rid of MS libc as much as possible. Ideally, some
    future version of Python will not rely on libc at all for Windows.
    If Microsoft had chosen to make the C library a system API, this
    we would happily use it. Alas, they chose to make it an API of their
    compiler instead, so we really shouldn't use it.
    b) the wide libc functions assume a 16-bit wchar_t type. This is not a
    good match for Python's unicode data type, which readily supports
    32-bit characters.

    @vstinner
    Copy link
    Member Author

    I notice that the patch changes rename() and link() to use
    win32_decode_filename() to coerce the filename to unicode before using
    the "wide" win32 api.

    Well, I did that to simplify the source code.

    (Previously, rename() first tried the wide api,
    falling back to narrow if that failed; link() used wide if the args were
    both unicode, narrow otherwise. Some other functions like symlink()
    already only use the wide api.)

    I can change my patch to mimick the previous behaviour: try Unicode-Unicode,
    or fall back to encoding both arguments to the filesystem encoding.

    Is this approach of coercing to unicode and only using the wide api
    "blessed"? I certainly think it should be. If so then one can get
    rid lots windows specific code.

    It was already discussed before to drop the bytes API to decode Unicode
    filenames in Python and only use the Unicode Windows API. There is no consensus
    on this topic: the statut is that the bytes API is kept but deprecated. bytes
    filenames will continue to use the bytes Windows API.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 15, 2011

    New changeset d42811b93357 by Victor Stinner in branch 'default':
    Issue bpo-13374: The Windows bytes API has been deprecated in the os module. Use
    http://hg.python.org/cpython/rev/d42811b93357

    @florentx
    Copy link
    Mannequin

    florentx mannequin commented Nov 15, 2011

    IIUC, it means that the library/application should not use the bytes API if it intends to be supported on major platforms.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Nov 15, 2011

    IIUC, it means that the library/application should not use the bytes API if it intends to be supported on major platforms.

    I think you misunderstand; it does not literally mean that. Instead, it
    means that the library/application either must not use the bytes API at
    all, or else make use of it conditional on non-Windows systems (i.e.
    special-case Windows).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 15, 2011

    New changeset afc716e463a1 by Victor Stinner in branch 'default':
    Issue bpo-13374: Skip deprecation tests for os.symlink() on Windows XP
    http://hg.python.org/cpython/rev/afc716e463a1

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 16, 2011

    New changeset 5f239b0ba819 by Victor Stinner in branch 'default':
    Issue bpo-13374: Deprecate os.getcwdb() on Windows
    http://hg.python.org/cpython/rev/5f239b0ba819

    @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
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant