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

Set zip_safe = False to disable egg installation #842

Merged
merged 1 commit into from
Aug 4, 2020

Conversation

mthrok
Copy link
Collaborator

@mthrok mthrok commented Jul 30, 2020

Currently setuptools assumes that torhcaudio is zip safe and performs
egg (zip) installation when python setup.py install (or pip install .).

When torchaudio load extension module written for Torchscript, the corresponding
loading function requires the so file to be present as an actual file, and this
does not work well with egg installation.

This PR fixes this by setting zip_safe=False in setup.py and disable egg
installation, so that python setup.py install installs uncompressed files
with regular directory structure.

The following is the error you get when you try to use the package installed with
python setup.py install, before this change.

    import torchaudio
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 656, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torchaudio-0.7.0a0+d6831c6-py3.6-linux-x86_64.egg/torchaudio/__init__.py", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 656, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torchaudio-0.7.0a0+d6831c6-py3.6-linux-x86_64.egg/torchaudio/extension/__init__.py", line 5, in <module>
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torchaudio-0.7.0a0+d6831c6-py3.6-linux-x86_64.egg/torchaudio/extension/extension.py", line 12, in _init_extension
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torchaudio-0.7.0a0+d6831c6-py3.6-linux-x86_64.egg/torchaudio/extension/extension.py", line 19, in _init_script_module
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torch/_classes.py", line 46, in load_library
    torch.ops.load_library(path)
  File "/torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torch/_ops.py", line 105, in load_library
    ctypes.CDLL(path)
  File "/torchaudio/envs/master-py3.6/lib/python3.6/ctypes/__init__.py", line 348, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: /torchaudio/envs/master-py3.6/lib/python3.6/site-packages/torchaudio-0.7.0a0+d6831c6-py3.6-linux-x86_64.egg/torchaudio/_torchaudio.py

@mthrok mthrok changed the title Fix setup.py Enable ABI suffix for extension module Jul 31, 2020
@mthrok mthrok force-pushed the fix-setup-py branch 4 times, most recently from 806f45a to c5d0cc0 Compare July 31, 2020 21:15
@mthrok mthrok changed the title Enable ABI suffix for extension module Set zip_safe = False to disable egg installation Jul 31, 2020
@mthrok mthrok force-pushed the fix-setup-py branch 2 times, most recently from 62f4825 to fd7a8fe Compare August 3, 2020 19:25
Currently `setuptools` assumes that torhcaudio is zip safe and performs
egg (zip) installation when `python setup.py install` (or `pip install .`).

Torchaudio checks module (C++ extension) availavility dynamically at runtime,
using importlib, and egg installation does not work well with this. (importlib
cannot find C++ extension with egg installation even though it exists)

An workaround for this is to set zip_safe=False in setup.py and disable egg
installation, so that `python setup.py install` installs uncompressed files
with regular directory structure.
@mthrok mthrok marked this pull request as ready for review August 4, 2020 19:04
@mthrok mthrok requested a review from vincentqb August 4, 2020 19:04
Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and the errors don't seem related. Can you double-check?

@mthrok
Copy link
Collaborator Author

mthrok commented Aug 4, 2020

LGTM, and the errors don't seem related. Can you double-check?

Checked that errors are not related. I feel there is something (configuration) wrong about our macOS test setup, but that's a separate issue.

@mthrok mthrok merged commit 3b05589 into pytorch:master Aug 4, 2020
@mthrok mthrok deleted the fix-setup-py branch August 4, 2020 20:08
@mthrok
Copy link
Collaborator Author

mthrok commented Aug 4, 2020

thanks!

mthrok added a commit that referenced this pull request Aug 5, 2020
We have been running unit test with editable installation. (i.e. `python setup.py develop`), with which we missed issues like #842. 

This CC makes installation in CI non-editable, and change test directory structure so that the source code will not shadow the installed version of `torchaudio`. With simple `pytest test`, `pytest` modifies `sys.path` and prepend checked out repository, which shadows the installed version.

To remedy this, the whole test suites has been moved from `./test` to `./test/torchaudio_unittest`. This adds nice module structure to our test code and we can do absolute import in each test module, which makes it possible again to run test with `python -m unittest torchaudio_unittest/XXX.py`

This change does not affect the regular development process (`python setup.py develop` && `pytest test`)
mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants