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

unittest import loop with pre-release Python #4376

Closed
mwichmann opened this issue Jul 12, 2023 · 3 comments · Fixed by #4381
Closed

unittest import loop with pre-release Python #4376

mwichmann opened this issue Jul 12, 2023 · 3 comments · Fixed by #4381
Labels
Python 3.12 testsuite Things that only affect the SCons testing. Do not use just because a PR has tests.

Comments

@mwichmann
Copy link
Collaborator

Filing this so it's recorded - at the moment a bit stumped.

In the last two Python 3.12 betas, the Platform unit tests fail on Windows with an import loop.

An unsubstantiated theory is that it's no longer possible to have a local module file named "posix". Standard library module names should be considered reserved. The case of posix is strange, it's a submodule of os with instructions never to import it directly:

This module provides access to operating system functionality that is standardized by the C Standard and the POSIX standard (a thinly disguised Unix interface).

Do not import this module directly Instead, import the module os, which provides a portable version of this interface. On Unix, the os module provides a superset of the posix interface. On non-Unix operating systems the posix module is not available, but a subset is always available through the os interface.

Is it possible that changes to the import system and internal Python checks based on whether posix is available are now being fooled by us having imported our own posix module?

Anyway, here are the two fails showing the broken import chain:

15/1279 (1.17%) C:\Users\mats\AppData\Local\Programs\Python\Python312\python.exe SCons\Platform\PlatformTests.py
Traceback (most recent call last):
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\PlatformTests.py", line 25, in <module>
    import unittest
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\unittest\__init__.py", line 65, in <module>
    from .loader import TestLoader, defaultTestLoader
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\unittest\loader.py", line 10, in <module>
    from fnmatch import fnmatch, fnmatchcase
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\fnmatch.py", line 13, in <module>
    import posixpath
  File "<frozen importlib._bootstrap>", line 1266, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1237, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 841, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1082, in exec_module
  File "<frozen posixpath>", line 374, in <module>
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\posix.py", line 34, in <module>
    from SCons.Platform import TempFileMunge
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\__init__.py", line 52, in <module>
    import SCons.Tool
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\__init__.py", line 51, in <module>
    from SCons.Tool.linkCommon import LibSymlinksActionFunction, LibSymlinksStrFun
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\linkCommon\__init__.py", line 30, in <module>
    from SCons.Tool.DCommon import isD
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\DCommon.py", line 32, in <module>
    from pathlib import Path
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\pathlib.py", line 73, in <module>
    _FNMATCH_PREFIX, _FNMATCH_SUFFIX = fnmatch.translate('_').split('_')
                                       ^^^^^^^^^^^^^^^^^
AttributeError: partially initialized module 'fnmatch' has no attribute 'translate' (most likely due to a circular import)

Test execution time: 0.8 seconds
16/1279 (1.25%) C:\Users\mats\AppData\Local\Programs\Python\Python312\python.exe SCons\Platform\virtualenvTests.py
Traceback (most recent call last):
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\virtualenvTests.py", line 25, in <module>
    import unittest
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\unittest\__init__.py", line 65, in <module>
    from .loader import TestLoader, defaultTestLoader
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\unittest\loader.py", line 10, in <module>
    from fnmatch import fnmatch, fnmatchcase
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\fnmatch.py", line 13, in <module>
    import posixpath
  File "<frozen importlib._bootstrap>", line 1266, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1237, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 841, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1082, in exec_module
  File "<frozen posixpath>", line 374, in <module>
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\posix.py", line 34, in <module>
    from SCons.Platform import TempFileMunge
  File "C:\Users\mats\Documents\github\scons\SCons\Platform\__init__.py", line 52, in <module>
    import SCons.Tool
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\__init__.py", line 51, in <module>
    from SCons.Tool.linkCommon import LibSymlinksActionFunction, LibSymlinksStrFun
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\linkCommon\__init__.py", line 30, in <module>
    from SCons.Tool.DCommon import isD
  File "C:\Users\mats\Documents\github\scons\SCons\Tool\DCommon.py", line 32, in <module>
    from pathlib import Path
  File "C:\Users\mats\AppData\Local\Programs\Python\Python312\Lib\pathlib.py", line 73, in <module>
    _FNMATCH_PREFIX, _FNMATCH_SUFFIX = fnmatch.translate('_').split('_')
                                       ^^^^^^^^^^^^^^^^^
AttributeError: partially initialized module 'fnmatch' has no attribute 'translate' (most likely due to a circular import)
@mwichmann
Copy link
Collaborator Author

FWIW, I listed "D Language" because it is its import of pathlib (which uses fnmatch, thus completing the loop) that triggers the exception, but I don't actually believe the D tool is at fault.

@mwichmann
Copy link
Collaborator Author

Traced this a little bit. On Windows, the import of posix is expected to fail, in rather a crucial place. This is from Lib/posixpath.py:

try:
    from posix import _path_normpath

except ImportError:
    def normpath(path):
...
else:
    def normpath(path):
        # stuff making use of _path_normpath

But at the point this is happening in the initialization chain, sys.path has a first element of full-path-to-scons\\SCons\\Platform, so the import of posix actually succeeds. I do not know why this did not fail before and fails now - presumably something in cpython is different that now triggers this. I do not know how Platform gets into the search path, if it wasn't there we'd presumably be okay.

@mwichmann
Copy link
Collaborator Author

Okay, more understanding now, feel a little slow having it take this long. Two things contribute to make this fail:

  1. Python by default adds the script directory (if any) to the front of sys.path. When running SCons/Platform/PlatformTests.py, then full-path-to/SCons/Platform is in the path, which means an import of posix will find that one first.
  2. A Python PR bpo-45582: Port getpath[p].c to Python python/cpython#29041 changed the behavior of posixpath.py to do an import of posix in a try block, where previously it just defined a generic normpath function without trying the import.

So as seen in the chain above, unittest -> fnmatch -> posixpath now tries to import posix, and because of item 1 above, when running on Windows, we get ours, rather than the expected ImportError, and now things are broken. It only affects this one test, because that's the only time SCons/Platform is in the search path.

Solutions I can see (both work): in PlatformTests.py, remove the script path from the front of sys.path before the stdlib imports. I don't think it even needs to be put back. Or... move PlatformTests.py up a level in the tree so calling it doesn't put SCons/Platform in sys.path, but rather SCons.

mwichmann added a commit to mwichmann/scons that referenced this issue Jul 20, 2023
For Python 3.12+, cpython module "posixpath" test-imports 'posix',
expecting it to fail on win32. However, for this one test program,
since it's in SCons/Platform, that directory as the "script path"
is in sys.path, and it finds our "posix" module, thus breaking things.
This removes that element from the path - it's not put back because
we don't actually need it here.

Fixes SCons#4376

Unlrelated - UtilTests.py is moved down into SCons/Util, to follow
the usual naming conventions where the unit test is in the package dir.

Signed-off-by: Mats Wichmann <mats@linux.com>
@mwichmann mwichmann added testsuite Things that only affect the SCons testing. Do not use just because a PR has tests. and removed D language labels Jul 20, 2023
mwichmann added a commit to mwichmann/scons that referenced this issue Jul 20, 2023
For Python 3.12+, cpython module "posixpath" test-imports 'posix',
expecting it to fail on win32. However, for this one test program,
since it's in SCons/Platform, that directory as the "script path"
is in sys.path, and it finds our "posix" module, thus breaking things.
This removes that element from the path - it's not put back because
we don't actually need it here.

Fixes SCons#4376

Signed-off-by: Mats Wichmann <mats@linux.com>
mwichmann added a commit to mwichmann/scons that referenced this issue Jul 27, 2023
Oops, missed that SCons/Platform/virtualenvTests.py needed the same
hack as SCons/Platform/PlatformTests.py (in PR SCons#4381)

Signed-off-by: Mats Wichmann <mats@linux.com>
bdbaddog added a commit that referenced this issue Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python 3.12 testsuite Things that only affect the SCons testing. Do not use just because a PR has tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant