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

frozen packages set an improper __path__ value #48461

Closed
brettcannon opened this issue Oct 27, 2008 · 6 comments
Closed

frozen packages set an improper __path__ value #48461

brettcannon opened this issue Oct 27, 2008 · 6 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

BPO 4211
Nosy @gvanrossum, @brettcannon, @ncoghlan, @benjaminp
Files
  • issue4211.diff
  • 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 2008-11-05.22:48:44.370>
    created_at = <Date 2008-10-27.02:42:04.433>
    labels = ['interpreter-core', 'type-bug', 'release-blocker']
    title = 'frozen packages set an improper __path__ value'
    updated_at = <Date 2008-11-05.22:48:44.363>
    user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2008-11-05.22:48:44.363>
    actor = 'benjamin.peterson'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-11-05.22:48:44.370>
    closer = 'benjamin.peterson'
    components = ['Interpreter Core']
    creation = <Date 2008-10-27.02:42:04.433>
    creator = 'brett.cannon'
    dependencies = []
    files = ['11920']
    hgrepos = []
    issue_num = 4211
    keywords = ['patch', 'needs review']
    message_count = 6.0
    messages = ['75250', '75269', '75432', '75531', '75532', '75533']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'brett.cannon', 'ncoghlan', 'benjamin.peterson']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue4211'
    versions = ['Python 2.6', 'Python 2.5', 'Python 3.0']

    @brettcannon
    Copy link
    Member Author

    If you import a frozen package (e.g. __phello__), __path__ is set to
    '__phello__'. But this should be a list as specified by both PEP-302
    (http://www.python.org/dev/peps/pep-0302/) and the original doc
    outlining the package mechanism
    (http://www.python.org/doc/essays/packages.html).

    Changing import to put the string in a list is all that is needed to
    correct this (although it might break some code relying on the fact that
    it has been a string in previous versions).

    @brettcannon brettcannon added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Oct 27, 2008
    @brettcannon
    Copy link
    Member Author

    Looking at Python/import.c:find_module, fixing this will require
    changing the setting of __path__ for frozen packages and how frozen
    modules are found. The former will require changing
    PyImport_ImportFrozenModule() to use a list instead of a string for the
    packages.

    To make the change work for find frozen modules, find_module() will need
    to change. Basically the special-casing for 'path' around frozen module
    just needs to go. This will lead to a slight performance penalty as all
    imports will require checking for an equivalent frozen module, but since
    it is a strcmp in a loop it should not be too bad.

    The performance cost can go away if some strapping young lad happens to
    re-implement import in such a way that the frozen module parts can
    easily be left out. =)

    Setting as a release blocker for now in case Barry is willing to let
    this go into 3.0rc4.

    @brettcannon
    Copy link
    Member Author

    I have an attached patch that fixes the reported problem.

    First, it adds a sanity check that no empty module name is checked for
    (crashes the interpreter otherwise).

    Second, the __path__ attribute is now a list. This does break
    backwards-compatibility and thus cannot be backported to 2.6.

    Third, I removed the special-casing of frozen packages and just made
    frozen module checks a universal thing. This did change the module
    resolution order such that frozen modules are checked before built-in
    modules and registered modules under Windows (whatever those are).
    Moving frozen modules after those two cases are possible if it's really
    desired.

    At this point I need a review to get this into 3.0 and a decision on
    whether to backport to 2.7 (I say don't bother, out of
    backward-compatibility, but it probably wouldn't really hurt anything
    either).

    @brettcannon brettcannon removed their assignment Oct 31, 2008
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Nov 5, 2008

    FWIW, I agree with the idea of fixing it for 3.0 and leaving it in for 2.x.

    @gvanrossum
    Copy link
    Member

    I approve of the API change. It's 3.0, dammit!

    @benjaminp
    Copy link
    Contributor

    Fixed in r67112.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) release-blocker type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants