BlueBrain / MorphIO

A python and C++ library for reading and writing neuronal morphologies
https://morphio.readthedocs.io
Apache License 2.0
26 stars 22 forks source link

Revert installing Python module systemwide. #473

Closed 1uc closed 12 months ago

1uc commented 12 months ago

The lines

install(
  TARGETS _morphio
  LIBRARY DESTINATION ${Python3_SITEARCH}/morphio/
)

cause CMake to find the directory where Python modules are installed. This would be the systemwide directory if one is simply building with the systemwide Python. If one has a venv active it'll install into that virtual environment.

Note that CMAKE_INSTALL_PREFIX doesn't affect where the module will be installed. Therefore, this will cause build/installation failure.

Installing into a venv is questionable, because that's the responsibility of the Python package manager. Which will do so correctly without the additional CMake install statement.

Finally, even if the install succeeds, it only installs the lib*.so without any __init__.py files. Therefore, it leads to a broken installation, e.g.

python -c "import morphio; morphio.Soma"

will successfully import morphio but there won't be any morphio.Soma, because that's handled via __init__.py

1uc commented 12 months ago

Here's the steps to reproduce on BB5:

module load unstable gcc cmake python hdf5
cmake -DCMAKE_INSTALL_PREFIX=build/install -DMorphIO_WERROR=Off -B build
cmake --build build --parallel --target install

which fails with:

CMake Error at binds/python/cmake_install.cmake:60 (file):
  file cannot create directory:
  /gpfs/bbp.cscs.ch/ssd/apps/bsd/2023-02-23/stage_externals/install_gcc-12.2.0-skylake/python-3.10.8-rvn6l5/lib/python3.10/site-packages/morphio.
  Maybe need administrative privileges.
Call Stack (most recent call first):
  cmake_install.cmake:80 (include)
mgeplf commented 12 months ago

thanks for finding this, and the fix.

1uc commented 12 months ago

(For posterity.)

Here's a variation that seems to show that even if there isn't a permission issue the lines:

install(
  TARGETS _morphio
  LIBRARY DESTINATION ${Python3_SITEARCH}/morphio/
)

are likely not what we expect. Here's the required step on BB5:

module load unstable gcc cmake python hdf5
# Ensure that we have write permissions during installation of `_morphio*.so`.
python -m venv venv
source venv/bin/activate
cmake -DCMAKE_INSTALL_PREFIX=build/install -DMorphIO_WERROR=Off -B build

We can now run and observe:

$ cmake --build build --parallel --target install
...
-- Installing: /gpfs/bbp.cscs.ch/home/groshein/MorphIO/venv/lib/python3.10/site-packages/morphio/_morphio.cpython-310-x86_64-linux-gnu.so

$ ls /gpfs/bbp.cscs.ch/home/groshein/MorphIO/venv/lib/python3.10/site-packages/morphio/
_morphio.cpython-310-x86_64-linux-gnu.so

$ which python 
~/MorphIO/venv/bin/python

$ cd # to avoid the folder `morphio`.
$ python -c "import morphio"
$ python -c "import morphio; morphio.Soma"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AttributeError: module 'morphio' has no attribute 'Soma'
$ pip uninstall morphio
WARNING: Skipping morphio as it is not installed.

To summarize, even if we have permissions, those lines would install a version of morphio which can be imported, but not used. Moreover, it can't be uninstalled via pip essentially breaking our virtual environment.