-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
__file__ is now fully qualified in 3.8 and 3.9 #88236
Comments
There was a change in behavior in Python 3.8.10 when using relative paths in sys.path. It appears that the paths are now converted to absolute paths that are cached and can cause import errors in some cases. Repro: $ cat repro.sh
#!/bin/bash
python --version
mkdir -p /tmp/repro/{A,B}/testproject
echo "msg = 'from A'" > /tmp/repro/A/testproject/app.py
echo "msg = 'from B'" > /tmp/repro/B/testproject/app.py
python -c "
import sys, os, shutil
os.chdir('/tmp/repro/A')
sys.path.append('testproject')
import app
print(app)
print(app.msg) os.chdir('/tmp/repro/B')
shutil.rmtree('/tmp/repro/A')
del sys.modules['app']
import app
print(app)
print(app.msg)
"
rm -rf /tmp/repro On Python 3.8.9 I get: $ ./repro.sh
Python 3.8.9
<module 'app' from 'testproject/app.py'>
from A
<module 'app' from 'testproject/app.py'>
from B On Python 3.8.10 I get: $ ./repro.sh
Python 3.8.10
<module 'app' from '/private/tmp/repro/A/testproject/app.py'>
from A
Traceback (most recent call last):
File "<string>", line 12, in <module>
ModuleNotFoundError: No module named 'app' I haven't confirmed this (I can't work out the frozen bootstrap stuff), but this might be caused by https://bugs.python.org/issue43105, whose patch does seem to be converting paths to absolute paths. |
without inspecting the code: I wonder if this is related to the same change as https://bugs.python.org/issue44061 ? |
It's almost certainly due to that change, though I'd be interested to see a more realistic repro. For the repro shown, you really ought to be clearing the importer cache as well when you remove a search path (as implied by chdir with a relative path on sys.path) and delete from sys.modules. The new abspath is only applied to the full path to the module, so the cache is still for the module, and not for the sys.path entry. What's the actual scenario that this broke? |
I only noticed this because a project that I work on (https://github.com/aws/chalice/) started failing CI for seemingly unrelated changes. A specific test run is here: https://github.com/jamesls/chalice/runs/2529906754. This didn't actually break the framework itself, just the tests for the framework. Chalice imports your application to figure out what resources to deploy to AWS, so the functional tests need to setup/teardown misc. applications and re-import them fresh for each test. Turns out the GitHub action I was using switched their Python 3.8 from 3.8.9 to 3.8.10 so I started looking into why this failed. My takeaway from this is to stop using relative imports in sys.path (I don't recall us having a specific reason to do this other than it worked). I figured I'd file an issue as I'm not actually sure if this consequence was intentional (I only saw bpo-43105 mentioned in the changelog), and was surprised this behavior changed in a patch release. |
I've just found this while investigating a regression with my project following update to 3.9.5. It took me some time to discover that the new test failures were due to __file__ now being fully qualified when it wasn't before. As far as I can tell so far it is just breaking the tests, not the software (https://github.com/omniosorg/pkg5) but this doesn't feel like a change that should be made in a minor release. |
Yeah, you're probably right. That effect was not noticed when implementing it. We should update the fix in 3.9 and 3.8 to limit it to .pyd's on Windows to protect against the security risks, but leave other import types loaded with relative paths. I think it's fine to leave the resolution in 3.10, as it's closer to the intended behaviour. I also don't think that fixing this justifies an extra maintenance release of 3.8, which is now in security-fix only mode, but I'll leave that decision to the RM. |
hello, just chiming in to let you know that this broke CI on twisted: twisted/twisted#1628 (As above for Chalice, I think this didn't actually break the framework itself, just the tests for the framework. ) do you know what the correct usage of importlib.util.spec_from_file_location is to match importlib.import_module ? I have a tree like this: . 2 directories, 4 files but I'm not sure how to get a spec for ./mypkg/demo.py: Python 3.9.6 (default, Jul 3 2021, 16:40:50)
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path
['', '/usr/lib/python39.zip', '/usr/lib/python3.9', '/usr/lib/python3.9/lib-dynload', '/usr/local/lib/python3.9/dist-packages', '/usr/lib/python3/dist-packages']
>>> import os
>>> os.getcwd()
'/home/graingert/projects/syspath-tests'
>>> import importlib.util
>>> importlib.util.spec_from_file_location(name="mypkg.demo", location="mypkg/demo.py")
ModuleSpec(name='mypkg.demo', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fc94644bfd0>, origin='mypkg/demo.py')
>>> import mypkg.demo
>>> mypkg.demo.__spec__
ModuleSpec(name='mypkg.demo', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fc94645a160>, origin='/home/graingert/projects/syspath-tests/mypkg/demo.py')
>>> |
Maybe I'm misreading your example, but isn't the correct usage here to pass an absolute location= argument? Or are you suggesting that spec_from_file_location should also make it absolute (in 3.10 and later, since this change has already been reverted in 3.8-9). |
Yep that's the fix I went for in the end https://github.com/twisted/twisted/pull/1628/files#diff-81177770fe13865c1f305bb999e82cd7514b2e7269f41087bd7ef39c3d6de508R105 |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: