Issue 26538: regrtest: setup_tests() must not replace module.__path__ (_NamespacePath) with a simple list // importlib & abspath

Messages (6) msg261562 - (view) Author: STINNER Victor (vstinner) * Date: 2016-03-11 11:44

Extract of test.libregrtest.setup_tests():

    for module in sys.modules.values():
        if hasattr(module, '__path__'):
            module.__path__ = [os.path.abspath(path)
                               for path in module.__path__]
        if hasattr(module, '__file__'):
            module.__file__ = os.path.abspath(module.__file__)

Because of this code, it's not possible to store test files outside Lib/test/. For the issue #26295 (test_regrtest), I would like to create a temporary directory and then a subdirectory test/ to create temporary test files.

Attached patch adds _NamespacePath.__setitem__() method and modify setup_tests() to keep the _NamespacePath type of module.__path__.

Maybe we should move this abspath() code somewhere in importlib. The site module already contains similar code, abs_paths() function:

    for m in set(sys.modules.values()):
        if (getattr(getattr(m, '__loader__', None), '__module__', None) not in
                ('_frozen_importlib', '_frozen_importlib_external')):
            continue   # don't mess with a PEP 302-supplied __file__
        try:
            m.__file__ = os.path.abspath(m.__file__)
        except (AttributeError, OSError):
            pass
        try:
            m.__cached__ = os.path.abspath(m.__cached__)
        except (AttributeError, OSError):
            pass

Since this code looks to depend on the implementation of importlib (the __loader__ test), IMHO it makes sense to move the code directly somewhere in importlib.

msg261581 - (view) Author: Brett Cannon (brett.cannon) * Date: 2016-03-11 16:56

I think a more important question is why is test.libregrtest doing what it's doing? Is it simply to guarantee that the `site` code is run? If so it should just import `site`.

As for adding the __setitem__() method, I'm fine with it but maybe Eric has an opinion?

msg261587 - (view) Author: STINNER Victor (vstinner) * Date: 2016-03-11 17:55

Longer extract of setup_tests(), with the long comment:

    # Some times __path__ and __file__ are not absolute (e.g. while running from
    # Lib/) and, if we change the CWD to run the tests in a temporary dir, some
    # imports might fail.  This affects only the modules imported before os.chdir().
    # These modules are searched first in sys.path[0] (so '' -- the CWD) and if
    # they are found in the CWD their __file__ and __path__ will be relative (this
    # happens before the chdir).  All the modules imported after the chdir, are
    # not found in the CWD, and since the other paths in sys.path[1:] are absolute
    # (site.py absolutize them), the __file__ and __path__ will be absolute too.
    # Therefore it is necessary to absolutize manually the __file__ and __path__ of
    # the packages to prevent later imports to fail when the CWD is different.
    for module in sys.modules.values():
        if hasattr(module, '__path__'):
            module.__path__ = [os.path.abspath(path)
                               for path in module.__path__]
        if hasattr(module, '__file__'):
            module.__file__ = os.path.abspath(module.__file__)

msg261592 - (view) Author: Brett Cannon (brett.cannon) * Date: 2016-03-11 19:21

Figures. If you do decide to centralize the code then put it in importlib.util and please don't make it public. It's something that should only be done once and not something most people should have to worry about.

msg261831 - (view) Author: Roundup Robot (python-dev) Date: 2016-03-15 22:47

New changeset 96b73b649b15 by Victor Stinner in branch 'default':
regrtest: Fix module.__path__
https://hg.python.org/cpython/rev/96b73b649b15

msg261904 - (view) Author: STINNER Victor (vstinner) * Date: 2016-03-17 10:40

I pushed my minimum fix. I now understand that it is not needed to change importlib, it's ok to keep "abspath" code in site & libregrtest.

History Date User Action Args 2022-04-11 14:58:28adminsetgithub: 70725 2016-03-17 10:40:38vstinnersetstatus: open -> closed
resolution: fixed 2016-03-17 10:40:28vstinnersetmessages: + msg261904 2016-03-15 22:47:30python-devsetnosy: + python-dev
messages: + msg261831
2016-03-11 19:21:07brett.cannonsetmessages: + msg261592 2016-03-11 17:55:49vstinnersetmessages: + msg261587 2016-03-11 16:56:12brett.cannonsetnosy: + eric.smith
messages: + msg261581
2016-03-11 11:44:27vstinnercreate