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

Importing ctypes.wintypes on Linux gives a ValueError instead of an ImportError #60600

Closed
techtonik mannequin opened this issue Nov 3, 2012 · 22 comments
Closed

Importing ctypes.wintypes on Linux gives a ValueError instead of an ImportError #60600

techtonik mannequin opened this issue Nov 3, 2012 · 22 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@techtonik
Copy link
Mannequin

techtonik mannequin commented Nov 3, 2012

BPO 16396
Nosy @amauryfa, @jaraco, @abalkin, @tiran, @benjaminp, @ezio-melotti, @meadori, @serhiy-storchaka, @zooba, @miss-islington, @hauntsaninja
PRs
  • bpo-16396: Allow wintypes to be imported on non-Windows systems. #21394
  • [3.9] bpo-16396: Allow wintypes to be imported on non-Windows systems. (GH-21394) #22790
  • [3.8] bpo-16396: Allow wintypes to be imported on non-Windows systems. (GH-21394) #22791
  • bpo-16396: fix BPO number in changelog #23951
  • [3.9] bpo-16396: fix BPO number in changelog (GH-23951) #23956
  • Files
  • 16396_vbool.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 = 'https://github.com/benjaminp'
    closed_at = <Date 2020-10-19.22:07:07.341>
    created_at = <Date 2012-11-03.15:22:51.972>
    labels = ['ctypes', 'type-bug', '3.8', '3.9', '3.10']
    title = 'Importing ctypes.wintypes on Linux gives a ValueError instead of an ImportError'
    updated_at = <Date 2020-12-29.11:52:15.433>
    user = 'https://bugs.python.org/techtonik'

    bugs.python.org fields:

    activity = <Date 2020-12-29.11:52:15.433>
    actor = 'serhiy.storchaka'
    assignee = 'benjamin.peterson'
    closed = True
    closed_date = <Date 2020-10-19.22:07:07.341>
    closer = 'steve.dower'
    components = ['ctypes']
    creation = <Date 2012-11-03.15:22:51.972>
    creator = 'techtonik'
    dependencies = []
    files = ['30684']
    hgrepos = []
    issue_num = 16396
    keywords = ['patch']
    message_count = 22.0
    messages = ['174634', '174635', '188245', '188707', '191756', '191763', '192324', '194927', '194948', '194985', '195652', '195675', '195825', '195924', '195926', '195940', '284191', '379033', '379042', '379047', '383808', '383983']
    nosy_count = 14.0
    nosy_names = ['amaury.forgeotdarc', 'jaraco', 'belopolsky', 'techtonik', 'christian.heimes', 'benjamin.peterson', 'ezio.melotti', 'meador.inge', 'serhiy.storchaka', 'steve.dower', 'dmi.baranov', 'Mike Place', 'miss-islington', 'hauntsaninja']
    pr_nums = ['21394', '22790', '22791', '23951', '23956']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue16396'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Nov 3, 2012

    >>> import ctypes.wintypes
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.2/ctypes/wintypes.py", line 20, in <module>
        class VARIANT_BOOL(ctypes._SimpleCData):
    ValueError: _type_ 'v' not supported
    >>> 

    Shouldn't it just import silently without failing? Or if it's destined to fail, explain how to make a cross-platform import?

    @techtonik techtonik mannequin added the topic-ctypes label Nov 3, 2012
    @techtonik
    Copy link
    Mannequin Author

    techtonik mannequin commented Nov 3, 2012

    Perhaps the patch already there - see http://www.themacaque.com/?p=826

    @ezio-melotti ezio-melotti added the type-bug An unexpected behavior, bug, or error label May 1, 2013
    @dmibaranov
    Copy link
    Mannequin

    dmibaranov mannequin commented May 1, 2013

    Found only this "patch" [1] :) I think is possible to change VARIANT_BOOL._type_ to any of short types [2] for non-"nt" platforms?

    [1] https://code.launchpad.net/~mandel/python-distutils-extra/import_issues/+merge/53519
    [2] http://msdn.microsoft.com/en-us/library/cc237864.aspx

    @ezio-melotti
    Copy link
    Member

    That patch is more a workaround than an actual fix. Lib/ctypes/wintypes.py should either fail with an ImportError or be importable. For the former it's possible to catch the ValueError and turn it into an ImportError, or perhaps raise it if some precondition is missing; for the latter, either the creation of that signle class is skipped if _type_ 'v' is not supported, or a way to define it that works on other platforms too should be found instead.

    @tiran
    Copy link
    Member

    tiran commented Jun 24, 2013

    The fix is trivial:

    • define VARIANT_FALSE and VARIANT_BOOL according to specs
    • enable 'v' on non-Windows systems
    • enable tests for vbool

    Done

    @jaraco
    Copy link
    Member

    jaraco commented Jun 24, 2013

    Looks good to me. Ben, any objections to applying this to 2.7?

    @tiran
    Copy link
    Member

    tiran commented Jul 4, 2013

    RM, please decide. :)

    @benjaminp
    Copy link
    Contributor

    Smells like a new feature to me.

    @ezio-melotti
    Copy link
    Member

    Even if the patch is applied only on 3.4, I would still like to see the ValueError turned into ImportError for 2.7/3.3.

    @jaraco
    Copy link
    Member

    jaraco commented Aug 12, 2013

    My sense on the issue is that wintypes was added to the library and was never intended to raise a ValueError on import. By that logic, the behavior is a bug, not a new feature. I agree with Ezio that raising a ValueError on import is a bug. And since the patch not only addresses the ValueError on import, but simply addresses the underlying cause, it seems to me the most obvious solution.

    @amauryfa
    Copy link
    Member

    I think it's the opposite: when Unix support was added to ctypes, 'import ctypes.wintypes' was not considered. By that logic, the patch is a new feature.
    IMO "historical" arguments are moot :-)

    I agree with the conclusion tough: the patch will not break code that carefully catches ValueError, and so it suitable for 2.7.

    @meadori
    Copy link
    Member

    meadori commented Aug 20, 2013

    On Mon, Aug 12, 2013 at 7:11 AM, Ezio Melotti <report@bugs.python.org>wrote:

    Even if the patch is applied only on 3.4, I would still like to see the
    ValueError turned into ImportError for 2.7/3.3.

    Why not raise an ImportError for all versions? I don't see how
    'ctypes.wintypes' is ever actually useful on anything but Windows.
    Allowing it to successfully import on non-Windows systems seems misleading.

    @benjaminp
    Copy link
    Contributor

    We can argue about whether it's a bugfix or not. But I don't see how having this patch in 2.7 helps anyone, since ctypes.wintypes is useless on non-Windows platforms.

    @jaraco
    Copy link
    Member

    jaraco commented Aug 22, 2013

    The first thing it helps is that it eliminates a ValueError on import. Without it, code must catch both ValueError and ImportError to run portably:

    import ctypes

    try:
    import ctypes.wintypes
    except ImportError, ValueError:
    # catch ImportError and ValueError due to bpo-16396
    pass

    ...

    def code_that_runs_only_on_win():
      ctypes.wintypes.foo

    One _could_ cause ctypes.wintypes to always raise an ImportError on non-Windows systems, allowing the import routine to be simpler:

    try:
    import ctypes.wintypes
    except ImportError:
    pass

    But it would be even nicer if ctypes.wintypes always imported on any platform, such that the statement could be simply:

    import ctypes.wintypes

    But it's conceivable that certain functionality in wintypes might be useful on other systems. Consider, for example, a routine that works with pickles produced on a Windows system, or simply a comparison of some object in ctypes.wintypes against a value produced on a Windows system.

    The argument for that need (esp. on Python 2.7) is not strong, but since the patch to address the ValueError is simple, straightforward, and perhaps even simpler than something that would address the ValueError specifically, it seems worthwhile to accept the patch.

    @meadori
    Copy link
    Member

    meadori commented Aug 22, 2013

    Using 'ctypes.wintypes' on non-Windows systems is most likely a bad idea. Most of the types are defined in terms of the types for the target that the interpreter is built for. Comparing serializations thereof in a cross platform manner doesn't make sense for a lot of cases.

    I really think it is only useful for Windows platforms and allowing it to silently import is misleading.

    @benjaminp
    Copy link
    Contributor

    This is not a regression, though, so most people wouldn't be able to simplify their code because they have to support older Python versions.

    @MikePlace
    Copy link
    Mannequin

    MikePlace mannequin commented Dec 28, 2016

    +1 to getting this patch in. The fact that this raises a ValueError and not an ImportError is really annoying and we definitely see it as a bug.

    @pppery pppery mannequin changed the title Importing ctypes.wintypes on Linux gives a traceback Importing ctypes.wintypes on Linux gives a ValueError instead of an ImportError Jan 1, 2017
    @jaraco jaraco added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Jul 7, 2020
    @zooba
    Copy link
    Member

    zooba commented Oct 19, 2020

    New changeset 5456e78 by Jason R. Coombs in branch 'master':
    bpo-16396: Allow wintypes to be imported on non-Windows systems. (GH-21394)
    5456e78

    @zooba zooba closed this as completed Oct 19, 2020
    @miss-islington
    Copy link
    Contributor

    New changeset 6e998fa by Miss Skeleton (bot) in branch '3.8':
    bpo-16396: Allow wintypes to be imported on non-Windows systems. (GH-21394)
    6e998fa

    @miss-islington
    Copy link
    Contributor

    New changeset 05d52a0 by Miss Skeleton (bot) in branch '3.9':
    bpo-16396: Allow wintypes to be imported on non-Windows systems. (GH-21394)
    05d52a0

    @miss-islington
    Copy link
    Contributor

    New changeset 7865f51 by Shantanu in branch 'master':
    bpo-16396: fix BPO number in changelog (GH-23951)
    7865f51

    @serhiy-storchaka
    Copy link
    Member

    New changeset 71d7390 by Miss Islington (bot) in branch '3.9':
    bpo-16396: fix BPO number in changelog (GH-23951) (GH-23956)
    71d7390

    @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
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    9 participants