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

Upstream removal of fake site module breaks TensorFlow support #529

Closed
SnoopJ opened this issue Dec 22, 2022 · 2 comments · Fixed by #546
Closed

Upstream removal of fake site module breaks TensorFlow support #529

SnoopJ opened this issue Dec 22, 2022 · 2 comments · Fixed by #546
Labels
bug Something isn't working

Comments

@SnoopJ
Copy link

SnoopJ commented Dec 22, 2022

Describe the bug
PyInstaller's fake site module was recently removed, allowing the value of site.USER_SITE to once again be None. This bug affects PyInstaller from 5.5 through 5.7

This causes a problem with some introspection in TensorFlow that runs at import time which assumes this value will always be a string.

Users affected by this bug can either pin PyInstaller<5.5 or work around the issue by executing import site; site.USER_SITE = '' before any TensorFlow imports occur (either in your application code or in a custom runtime hook), and the fix is probably to do something similar in a runtime hook for the library.

To Reproduce

A minimal example file:

import tensorflow.lite

print("Test program: tflite import succeeded")

PyInstaller command:

python -m PyInstaller --collect-all tensorflow testapp.py

Error (when running the resulting distribution):

PS C:\> .\dist\testapp\testapp.exe
Traceback (most recent call last):
  File "testapp.py", line 1, in <module>
    import tensorflow.lite
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "PyInstaller\loader\pyimod02_importers.py", line 499, in exec_module 
  File "tensorflow\__init__.py", line 102, in <module>
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "PyInstaller\loader\pyimod02_importers.py", line 499, in exec_module 
  File "tensorflow_core\__init__.py", line 771, in <module>
  File "tensorflow_core\__init__.py", line 769, in _running_from_pip_package
  File "tensorflow_core\__init__.py", line 769, in <genexpr>
TypeError: startswith first arg must be str or a tuple of str, not NoneType 
[15524] Failed to execute script 'testapp' due to unhandled exception!

Note that the line numbers reported here are on the far side of template interpolation during TensorFlow's build process, tensorflow_core\__init__.py:769 in this error report corresponds to Line 130 of the templated file

Expected behavior
The test program should not error on the import and should print the success message, as it does in PyInstaller versions preceding 5.5

Screenshots
N/A

Desktop (please complete the following information):

  • OS: Windows 10
  • Python Version: 3.7.9
  • Version of pyinstaller-hooks-contrib: 2022.14
  • Version of PyInstaller: 5.7

Additional context
N/A

@SnoopJ SnoopJ added the state:triage We're still figuring out how severe this issue is label Dec 22, 2022
@rokm
Copy link
Member

rokm commented Dec 22, 2022

This causes a problem with some introspection in TensorFlow that runs at import time which assumes this value will always be a string.

That's obviously tensorflow's bug, because site docs clearly state that the value might be None. Even if it happened to work by chance due to PyInstaller's non-standard site override, it did not work when python was started with site disabled (the -S flag).

And it seems to have been fixed on the tensorflow side since v2.3.0 (tensorflow/tensorflow@492e930), so only earlier versions are affected.

On the other hand, the suggested work-around with runtime tensorflow hook setting site.USER_SITE to an empty string is probably the sanest option for everyone involved, at least until we drop support for python 3.7 and 3.8 and therefore the affected tensorflow versions...

@rokm rokm added bug Something isn't working and removed state:triage We're still figuring out how severe this issue is labels Dec 22, 2022
@SnoopJ
Copy link
Author

SnoopJ commented Dec 22, 2022

And it seems to have been fixed on the tensorflow side since v2.3.0 (tensorflow/tensorflow@492e930), so only earlier versions are affected.

Oops, I missed this in my review of the issue! I indeed discovered this against an older version, but glad to know they've made their code a little less naive about site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants