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

Immutable module type can't be used as package in custom loader #83517

Closed
DinoV opened this issue Jan 14, 2020 · 7 comments
Closed

Immutable module type can't be used as package in custom loader #83517

DinoV opened this issue Jan 14, 2020 · 7 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@DinoV
Copy link
Contributor

DinoV commented Jan 14, 2020

BPO 39336
Nosy @brettcannon, @vstinner, @DinoV, @corona10
PRs
  • bpo-39336: Allow packages to not let their child modules be set on them #18006
  • 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/DinoV'
    closed_at = <Date 2020-01-23.00:57:27.184>
    created_at = <Date 2020-01-14.21:33:31.500>
    labels = ['interpreter-core', 'type-bug']
    title = "Immutable module type can't be used as package in custom loader"
    updated_at = <Date 2020-01-28.02:21:01.711>
    user = 'https://github.com/DinoV'

    bugs.python.org fields:

    activity = <Date 2020-01-28.02:21:01.711>
    actor = 'vstinner'
    assignee = 'dino.viehland'
    closed = True
    closed_date = <Date 2020-01-23.00:57:27.184>
    closer = 'dino.viehland'
    components = ['Interpreter Core']
    creation = <Date 2020-01-14.21:33:31.500>
    creator = 'dino.viehland'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 39336
    keywords = ['patch']
    message_count = 7.0
    messages = ['360000', '360006', '360014', '360164', '360525', '360745', '360832']
    nosy_count = 4.0
    nosy_names = ['brett.cannon', 'vstinner', 'dino.viehland', 'corona10']
    pr_nums = ['18006']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue39336'
    versions = []

    @DinoV
    Copy link
    Contributor Author

    DinoV commented Jan 14, 2020

    I'm trying to create a custom module type for a custom loader where the returned modules are immutable. But I'm running into an issue where the immutable module type can't be used as a module for a package. That's because the import machinery calls setattr to set the module as an attribute on it's parent in _boostrap.py

            # Set the module as an attribute on its parent.
            parent_module = sys.modules[parent]
            setattr(parent_module, name.rpartition('.')[2], module)

    I'd be okay if for these immutable module types they simply didn't have their children packages published on them.

    A simple simulation of this is a package which replaces its self with an object which doesn't support adding arbitrary attributes:

    x/init.py:
    import sys

    class MyMod(object):
        __slots__ = ['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__']
        def __init__(self):
            for attr in self.__slots__:
                setattr(self, attr, globals()[attr])

    sys.modules['x'] = MyMod()

    x/y.py:
    # Empty file

    >>> from x import y
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<frozen importlib._bootstrap>", line 983, in _find_and_load
      File "<frozen importlib._bootstrap>", line 971, in _find_and_load_unlocked
    AttributeError: 'MyMod' object has no attribute 'y'

    There's a few different options I could see on how this could be supported:
    1) Simply handle the attribute error and allow things to continue
    2) Add ability for the modules loader to perform the set, and fallback to setattr if one isn't available. Such as:
    getattr(parent_module, 'add_child_module', setattr)(parent_module, name.rpartition('.')[2], module)
    3) Add the ability for the module type to handle the setattr:
    getattr(type(parent_module), 'add_child_module', fallback)(parent_module,
    , name.rpartition('.')[2], module)

    @DinoV DinoV self-assigned this Jan 14, 2020
    @DinoV DinoV added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jan 14, 2020
    @DinoV DinoV self-assigned this Jan 14, 2020
    @DinoV DinoV added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jan 14, 2020
    @brettcannon
    Copy link
    Member

    So I think this is way too marginal a use-case to expand the API to let loaders inject themselves into the semantics of it.

    I assume going with option 1 but raising an ImportWarning would be too noisy for your use-case? If not I'm totally fine with that solution.

    @DinoV
    Copy link
    Contributor Author

    DinoV commented Jan 14, 2020

    I think the warning shouldn't be too bad. It looks like ImportWarnings are filtered by default already, and the extra overhead of raising a warning in this case probably is nothing compared to the actual work in loading the module.

    @corona10
    Copy link
    Member

    I apologize for the noise caused by the wrong PR connection.

    @DinoV
    Copy link
    Contributor Author

    DinoV commented Jan 23, 2020

    New changeset 9b6fec4 by Dino Viehland in branch 'master':
    bpo-39336: Allow packages to not let their child modules be set on them (bpo-18006)
    9b6fec4

    @DinoV DinoV closed this as completed Jan 23, 2020
    @DinoV DinoV closed this as completed Jan 23, 2020
    @vstinner
    Copy link
    Member

    test_unwritable_module() fails on AMD64 Fedora Stable Clang Installed 3.x: bpo-39459.

    @vstinner
    Copy link
    Member

    commit 2528a6c
    Author: Dino Viehland <dinoviehland@gmail.com>
    Date: Mon Jan 27 14:04:56 2020 -0800

    Add test.test_import.data.unwritable package to makefile (bpo-18211)
    

    @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) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants