Always pass --root to setuptools install.

[mikepurvis]

I believe this is the minimal change resolving #1060, which arises from the change made in #1048.

For the non-DESTDIR case:

$ cmake .. -DCMAKE_INSTALL_PREFIX=./install -DPYTHON_EXECUTABLE=/usr/bin/python3
$ make install
$ find install -name *.py -o -name *.pth
install/_setup_util.py
install/lib/python3/dist-packages/catkin/builder.py
install/lib/python3/dist-packages/catkin/terminal_color.py
install/lib/python3/dist-packages/catkin/package_version.py
install/lib/python3/dist-packages/catkin/environment_cache.py
install/lib/python3/dist-packages/catkin/__init__.py
install/lib/python3/dist-packages/catkin/test_results.py
install/lib/python3/dist-packages/catkin/workspace.py
install/lib/python3/dist-packages/catkin/init_workspace.py
install/lib/python3/dist-packages/catkin/cmake.py
install/lib/python3/dist-packages/catkin/find_in_workspaces.py
install/lib/python3/dist-packages/catkin/workspace_vcs.py
install/lib/python3/dist-packages/catkin/tidy_xml.py
install/share/catkin/cmake/parse_package_xml.py
install/share/catkin/cmake/order_paths.py
install/share/catkin/cmake/interrogate_setup_dot_py.py
install/share/catkin/cmake/test/download_checkmd5.py
install/share/catkin/cmake/test/remove_test_results.py
install/share/catkin/cmake/test/run_tests.py

For the DESTDIR case:

$ cmake .. -DCMAKE_INSTALL_PREFIX=/opt/ros/banana -DPYTHON_EXECUTABLE=/usr/bin/python3
$ DESTDIR=$(pwd)/install make install
$ find install -name *.py -o -name *.pth
install/opt/ros/banana/_setup_util.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/builder.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/terminal_color.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/package_version.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/environment_cache.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/__init__.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/test_results.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/workspace.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/init_workspace.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/cmake.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/find_in_workspaces.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/workspace_vcs.py
install/opt/ros/banana/lib/python3/dist-packages/catkin/tidy_xml.py
install/opt/ros/banana/share/catkin/cmake/parse_package_xml.py
install/opt/ros/banana/share/catkin/cmake/order_paths.py
install/opt/ros/banana/share/catkin/cmake/interrogate_setup_dot_py.py
install/opt/ros/banana/share/catkin/cmake/test/download_checkmd5.py
install/opt/ros/banana/share/catkin/cmake/test/remove_test_results.py
install/opt/ros/banana/share/catkin/cmake/test/run_tests.py

The non-DESTDIR case is what's currently broken in 0.8.0. It looks like this without the change proposed here— the shared easy-install.pth and site.py files are the core of the problem:

$ find install -name *.py -o -name *.pth
install/_setup_util.py
install/lib/python3/dist-packages/easy-install.pth
install/lib/python3/dist-packages/site.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/builder.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/terminal_color.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/package_version.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/environment_cache.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/__init__.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/test_results.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/workspace.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/init_workspace.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/cmake.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/find_in_workspaces.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/workspace_vcs.py
install/lib/python3/dist-packages/catkin-0.8.0-py3.5.egg/catkin/tidy_xml.py
install/share/catkin/cmake/parse_package_xml.py
install/share/catkin/cmake/order_paths.py
install/share/catkin/cmake/interrogate_setup_dot_py.py
install/share/catkin/cmake/test/download_checkmd5.py
install/share/catkin/cmake/test/remove_test_results.py
install/share/catkin/cmake/test/run_tests.py

I haven't made any alterations to the Windows version of this logic as I'm not sure if anything is needed there or how to properly test it.

FYI @dirk-thomas @sloretz

[sloretz]

LGTM. With this change I no longer see an easy-install.pth or site.py building currently released Noetic repos using either catkin_make install or catkin_make_isolated install. I did not test with catkin-tools.

python3 src/catkin/bin/catkin_make install --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCMAKE_BUILD_TYPE=Release

python3 src/catkin/bin/catkin_make_isolated --catkin-make-args install all tests --cmake-args -DBUILD_TESTING=1 -DCATKIN_ENABLE_TESTING=1 -DCATKIN_SKIP_TESTING=0 -DCMAKE_BUILD_TYPE=Release

[mikepurvis]

Great, thanks for looking, @sloretz. I don't know if anything breaks in the catkin_make case; it would depend if something protects multiple catkin_setup_python-driven setuptools invocations from happening concurrently (or if easy-install is under the hood somehow capable of managing that scenario).

But this definitely is an issue with explicitly-parallel tools like catkin_tools. Perhaps the only argument against making the change here is that those tools could be responsible to themselves pass in DESTDIR=/, but other than that I dislike the idea that the in-ws installspace is no longer expected to be always identical to what ends up in the shipped version of a package.

Obviously this change ripples out and affects all catkin python packages that have been recently switched to setuptools, but it also nicely brings python_qt_binding into alignment with the others; that one has been a setuptools package since ros-visualization/python_qt_binding@0a2e678, and had the weird egg/pth installation in our bundle workspace. It didn't seem to be hurting anything since it was the only one, but I'm glad to have that loose end tied up:

install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/__init__.py
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/QtBindingHelper.py
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/__pycache__
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/__pycache__/__init__.cpython-35.pyc
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/__pycache__/binding_helper.cpython-35.pyc
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/__pycache__/QtBindingHelper.cpython-35.pyc
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding/binding_helper.py
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding-0.3.6-py3.5.egg-info
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding-0.3.6-py3.5.egg-info/top_level.txt
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding-0.3.6-py3.5.egg-info/PKG-INFO
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding-0.3.6-py3.5.egg-info/SOURCES.txt
install/python_qt_binding/lib/python3/dist-packages/python_qt_binding-0.3.6-py3.5.egg-info/dependency_links.txt

This was in a workspace that also included catkin, built with catkin_tools, so that ticks that box as well.

[dirk-thomas]

@seanyen Can you please try this patch on Windows.

[mikepurvis]

This patch as it stands should have no impact on Windows at all, as it only modifies the sh setuptools wrapper. What I believe needs to be checked on Windows is whether this invocation (or the bat equivalent) produces the stray site.py and easy-install.pth files in the install tree:

# from inside a catkin checkout
mkdir build; cd build
cmake .. -DCMAKE_INSTALL_PREFIX=./install -DPYTHON_EXECUTABLE=/usr/bin/python3
make install

If Windows also has the issue that this patch seeks to correct, then something similar may need to be applied to the bat version of the script.

[dirk-thomas]

@mikepurvis Good point, thanks for clarifying.

@seanyen Please double check the behavior on Windows and if necessary contribute a pull request with a similar patch for the .bat file.

[dirk-thomas]

Thanks for the patch!

[dirk-thomas]

Note to myself: this might need to be backported to kinetic-devel since several packages have switched to setuptools which don't have Noetic-specific branches.

[dirk-thomas]

Cherry-picked to kinetic-devel: f953c43.

[seanyen]

@seanyen Please double check the behavior on Windows and if necessary contribute a pull request with a similar patch for the .bat file.

Sorry I missed this ping. I am aware of setuptools migration causing the similar problems (egg files, and easy-install.pth stuff) on Windows build for melodic. Looking into that now...