pyros-dev / catkin_pip

Catkin extension to play nice with usual python packages and workflow
10 stars 8 forks source link

system packages found before packages from PYTHONPATH (catkin workspaces) #60

Closed asmodehn closed 7 years ago

asmodehn commented 7 years ago

Not sure why yet, but system python packages are found before python packages setup in pythonpath... I had the case a few times (especially with funny stuff like submodules in celeros). Just recording the problem here, we need to investigate why.

asmodehn commented 7 years ago

https://github.com/asmodehn/catkin_pip/pull/61 might fix a few issues relatd with PYTHONPATH. including this one...

asmodehn commented 7 years ago

This is correct output for python -m site (<current_dir> denotes the current directory) :

sys.path = [
    <current_dir>,
    '/usr/lib/python2.7',
    '/usr/lib/python2.7/plat-x86_64-linux-gnu',
    '/usr/lib/python2.7/lib-tk',
    '/usr/lib/python2.7/lib-old',
    '/usr/lib/python2.7/lib-dynload',
    '/usr/local/lib/python2.7/dist-packages',
    '/usr/lib/python2.7/dist-packages',
    '/usr/lib/python2.7/dist-packages/PILcompat',
    '/usr/lib/python2.7/dist-packages/gst-0.10',
    '/usr/lib/python2.7/dist-packages/gtk-2.0',
    '/usr/lib/pymodules/python2.7',
    '/usr/lib/python2.7/dist-packages/ubuntu-sso-client',
    '/usr/lib/python2.7/dist-packages/wx-2.8-gtk2-unicode',
]
USER_BASE: '/home/alexv/.local' (exists)
USER_SITE: '/home/alexv/.local/lib/python2.7/site-packages' (doesn't exist)
ENABLE_USER_SITE: True

This is an incorrect output :

sys.path = [
    <current_dir>,
    '/usr/lib/python2.7/dist-packages',
    '/usr/lib/python2.7',
    '/usr/lib/python2.7/plat-x86_64-linux-gnu',
    '/usr/lib/python2.7/lib-tk',
    '/usr/lib/python2.7/lib-old',
    '/usr/lib/python2.7/lib-dynload',
    '/usr/local/lib/python2.7/dist-packages',
    '/usr/lib/python2.7/dist-packages/PILcompat',
    '/usr/lib/python2.7/dist-packages/gst-0.10',
    '/usr/lib/python2.7/dist-packages/gtk-2.0',
    '/usr/lib/pymodules/python2.7',
    '/usr/lib/python2.7/dist-packages/ubuntu-sso-client',
    '/usr/lib/python2.7/dist-packages/wx-2.8-gtk2-unicode',
]
USER_BASE: '/home/alexv/.local' (exists)
USER_SITE: '/home/alexv/.local/lib/python2.7/site-packages' (doesn't exist)
ENABLE_USER_SITE: True

Notice how '/usr/lib/python2.7/dist-packages', is before '/usr/lib/python2.7',.

This usually happens because there is an easy-install.pth somewhere on your system that modifies python's sys.path to put this in front.

This will break ROS python setup that relies on modifying PYTHONPATH, since this system python package path will be in front of the PYTHONPATH paths.

The manual fix in that case is to cleanup the easy-install.pth to not have this behavior.

We need to find out why this happens. Some package, or install process, is likely putting this there...

asmodehn commented 7 years ago

Using a virtual environment to not break the system.

This happens when using python setup.py develop with old setuptools

(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip --version
pip 1.5.4 from /home/yujin/.virtualenvs/yujin-ansible/local/lib/python2.7/site-packages (python 2.7)
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip list 
argparse (1.2.1)
pip (1.5.4)
setuptools (2.2)
wsgiref (0.1.2)
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ python setup.py develop
running develop
running galaxy
[...]
running egg_info
writing dependency_links to yujin_ansible.egg-info/dependency_links.txt
writing requirements to yujin_ansible.egg-info/requires.txt
writing yujin_ansible.egg-info/PKG-INFO
writing top-level names to yujin_ansible.egg-info/top_level.txt
reading manifest file 'yujin_ansible.egg-info/SOURCES.txt'
writing manifest file 'yujin_ansible.egg-info/SOURCES.txt'
running build_ext
Creating /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/yujin-ansible.egg-link (link to .)
Adding yujin-ansible 0.1.12 to easy-install.pth file
Installing yujin_access_setup.sh script to /home/yujin/.virtualenvs/yujin-ansible/bin
Installing yujin_ansible script to /home/yujin/.virtualenvs/yujin-ansible/bin

Installed /home/yujin/yujin_ansible
Processing dependencies for yujin-ansible==0.1.12
Searching for termcolor==1.1.0
Best match: termcolor 1.1.0
Processing termcolor-1.1.0-py2.7.egg
Adding termcolor 1.1.0 to easy-install.pth file

Using /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/termcolor-1.1.0-py2.7.egg
Finished processing dependencies for yujin-ansible==0.1.12
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
/home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/easy-install.pth
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ cat /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/easy-install.pth
import sys; sys.__plen = len(sys.path)
/home/yujin/yujin_ansible
./termcolor-1.1.0-py2.7.egg
import sys; new=sys.path[sys.__plen:]; del sys.path[sys.__plen:]; p=getattr(sys,'__egginsert',0); sys.path[p:p]=new; sys.__egginsert = p+len(new)

The easyinstall.pth file is still created when using recent setuptools :

(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip install pip --upgrade
Downloading/unpacking pip from https://pypi.python.org/packages/b6/ac/7015eb97dc749283ffdec1c3a88ddb8ae03b8fad0f0e611408f196358da3/pip-9.0.1-py2.py3-none-any.whl#md5=297dbd16ef53bcef0447d245815f5144
  Downloading pip-9.0.1-py2.py3-none-any.whl (1.3MB): 1.3MB downloaded
Installing collected packages: pip
  Found existing installation: pip 1.5.4
    Uninstalling pip:
      Successfully uninstalled pip
Successfully installed pip
Cleaning up...
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip install setuptools --upgrade
/home/yujin/.virtualenvs/yujin-ansible/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:318: SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name Indication) extension to TLS is not available on this platform. This may cause the server to present an incorrect TLS certificate, which can cause validation failures. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#snimissingwarning.
  SNIMissingWarning
/home/yujin/.virtualenvs/yujin-ansible/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
Collecting setuptools
  Downloading setuptools-32.3.1-py2.py3-none-any.whl (479kB)
    100% |████████████████████████████████| 481kB 1.2MB/s 
Installing collected packages: setuptools
  Found existing installation: setuptools 2.2
    Uninstalling setuptools-2.2:
      Successfully uninstalled setuptools-2.2
Successfully installed setuptools-32.3.1
/home/yujin/.virtualenvs/yujin-ansible/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip --version
pip 9.0.1 from /home/yujin/.virtualenvs/yujin-ansible/local/lib/python2.7/site-packages (python 2.7)
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip list 
DEPRECATION: The default format will switch to columns in the future. You can use --format=(legacy|columns) (or define a format=(legacy|columns) in your pip.conf under the [list] section) to disable this warning.
pip (9.0.1)
setuptools (32.3.1)
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ python setup.py develop
running develop
running galaxy
[...]
running egg_info
writing requirements to yujin_ansible.egg-info/requires.txt
writing yujin_ansible.egg-info/PKG-INFO
writing top-level names to yujin_ansible.egg-info/top_level.txt
writing dependency_links to yujin_ansible.egg-info/dependency_links.txt
reading manifest file 'yujin_ansible.egg-info/SOURCES.txt'
writing manifest file 'yujin_ansible.egg-info/SOURCES.txt'
running build_ext
Creating /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/yujin-ansible.egg-link (link to .)
Adding yujin-ansible 0.1.12 to easy-install.pth file
Installing yujin_access_setup.sh script to /home/yujin/.virtualenvs/yujin-ansible/bin
Installing yujin_ansible script to /home/yujin/.virtualenvs/yujin-ansible/bin

Installed /home/yujin/yujin_ansible
Processing dependencies for yujin-ansible==0.1.12
Searching for termcolor==1.1.0
Best match: termcolor 1.1.0
Processing termcolor-1.1.0-py2.7.egg
Adding termcolor 1.1.0 to easy-install.pth file

Using /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/termcolor-1.1.0-py2.7.egg
Finished processing dependencies for yujin-ansible==0.1.12
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
/home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/easy-install.pth
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ cat /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/easy-install.pth
/home/yujin/yujin_ansible
./termcolor-1.1.0-py2.7.egg

With recent pip and setuptools:

(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip install .
Processing /home/yujin/yujin_ansible
Requirement already satisfied: termcolor in /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/termcolor-1.1.0-py2.7.egg (from yujin-ansible==0.1.12)
Installing collected packages: yujin-ansible
  Running setup.py install for yujin-ansible ... done
Successfully installed yujin-ansible-0.1.12
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip uninstall .
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ rm -rf /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/yujin_ansible-0.1.12-py2.7.egg/
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ rm -rf /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/yujin-ansible.egg-link 
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ pip install -e .
Obtaining file:///home/yujin/yujin_ansible
Requirement already satisfied: termcolor in /home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/termcolor-1.1.0-py2.7.egg (from yujin-ansible==0.1.12)
Installing collected packages: yujin-ansible
  Found existing installation: yujin-ansible 0.1.12
    Uninstalling yujin-ansible-0.1.12:
      Successfully uninstalled yujin-ansible-0.1.12
  Running setup.py develop for yujin-ansible
Successfully installed yujin-ansible
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ find / -name easy-install.pth 2> /dev/null
/home/yujin/.virtualenvs/yujin-ansible/lib/python2.7/site-packages/easy-install.pth
(yujin-ansible)yujin@gocart-test:~/yujin_ansible$ 

So the usual python guidelines apply here. Or more specifically for our use case with ROS deployments :

Note catkin_pip should not suffer from this since we mostly follow the python guidelines, and we do pip installation into prefixes. However it is very useful to keep these in mind when looking for issues in python setup.

FYI @stonier

asmodehn commented 7 years ago

Since ROS setup is very sensitive to this, should catkin_pip check and warn the user ? We dont want to fix the user OS setup, but giving hint on likely broken setup might be useful. Question is what are exactly the necessary criteria we should check warn the user about ?

asmodehn commented 7 years ago

https://github.com/asmodehn/catkin_pip/pull/61 now addresses this with a set of tests, run on 4 different python project templates from cookiecutter, to make sure the sys.path is what we expect it to be.

Maybe we should mutate these tests into assertions during build time ? The envhook will attempt to fix these issues when sourced, except for python devel packages. Also pyros_setup will attempt to fix the sys.path if any problem is detected on runtime (when using pyros_setup)

We ll need a bit of time to find out if this is a proper strategy... But we should aim to keep catkin_pip simple, and combine with pyros_setup for pure python package runtime behavior... instead of implementing the same thing in two places.

asmodehn commented 7 years ago

Closing this issue for now as it has been addressed in 0.2. Lets see if we need to do more there after 0.2 is released. but we do not want to attempt to fix a broken python install on a user system...