search

ros-melodic-arch / ros-melodic-desktop-full

ros-melodic-desktop-fullAUR package
2 stars 0 forks source link

Build fails due to hardcoded python3.7 paths #8

Closed acxz closed 4 years ago

acxz commented 5 years ago

Pretty much every PKGBUILD has hardcoded python paths, as noticed by https://github.com/ros-melodic-arch/ros-build-tools-py3/issues/1. This is causing failures to build when a new python version is released.

I have been going through building ros-melodic-desktop-full from scratch and have been fixing python issues as they come up. Surprisingly even though every package has hardcoded python paths, only some of them use these python environment variables we are setting in the PKGBUILDS.

To future proof this issue we need to get rid python variables in the PKGBUILDs that do not even use them and for the ones that do use them we either 1) autodetect and compile based on the installed python version, instead of hardcoding or 2) make upstream patches so we do not need to specify these cmake variables in the first place. I mean it seems like every other package is doing that besides the ones listed below.

As of now here is a list of PRs that need to be merged for ros-melodic-desktop-full to build again with py3.8. The rest of the packages do not even need to be changed from py3.7 to py3.8 since they do not even use these variables. We will still have the same problem when we move to py3.9 but right now I want to focus on having a successful ROS installation ASAP then finding the best solution (which we should still be working on).

For ros-melodic-ros-base:

For ros-melodic-desktop:

For ros-melodic-desktop-full:

We will close this issue as they get rolled out to AUR.

jwhendy commented 4 years ago

I manually built these to verify, then merged all of them. Based on your investigation, then, would you propose we go through and remove these lines from all other packages? As in, no other packages use these lines at all???

        cmake ${srcdir}/${_dir} \
                -DCMAKE_BUILD_TYPE=Release \
                -DCATKIN_BUILD_BINARY_PACKAGE=ON \
                -DCMAKE_INSTALL_PREFIX=/opt/ros/melodic \
                -DPYTHON_EXECUTABLE=/usr/bin/python3 \
### delete:                -DPYTHON_INCLUDE_DIR=/usr/include/python3.8 \
### delete:                -DPYTHON_LIBRARY=/usr/lib/libpython3.8.so \
### delete:                -DPYTHON_BASENAME=.cpython-38 \
                -DSETUPTOOLS_DEB_LAYOUT=OFF
        make

Or are others also candidates for removal?

And I assume this is independent from this line, which would stay for all packages?

# Fix Python2/Python3 conflicts.
        /usr/share/ros-build-tools/fix-python-scripts.sh -v 3 ${srcdir}/${_dir}
acxz commented 4 years ago

Yep, at the very least those are not needed. Maybe the -DPYTHON_EXECUTABLE=/usr/bin/python3 line can also go. Yeah the conflict link is prob separate as well, but not sure if we need that either. @bionade24 can provide a better recommendation moving forward, but that is what I believe the way forward is.

jwhendy commented 4 years ago

I was building my own ~/catkin_ws and got an error with moveit_tutorials. It complained that ros_pcl pointed to a non-existent 3.7 lib. I went to rebuild that (updated ros-melodic-pcl-ros to 3.8) and it complained instead about ros-melodic-pcl-conversions. Updating that to 3.8 and then rebuilding ros-melodic-pcl-ros (but reverting to the unused 3.7 commands) worked.

Edit: only needed to rebuild these, not update to 3.8 tl;dr:

acxz commented 4 years ago

Did you try to just rebuild ros-melodic-pcl-conversions before changing the PKGBUILD for it to py3.8?

jwhendy commented 4 years ago

Dangit. Didn't even dawn on me! So basically we don't necessarily have to specify these 3.8 paths, but sometimes the packages will still depend on a version-specific lib location?

Also... should I just leave the above since it likely doesn't matter? Or commit/push a reversion back to 3.7?

Sorry about that!

acxz commented 4 years ago

lol its fine, but yeah a bunch of packages on the official repos got updated so they would be recompiled. For AUR packages, users have to recompile them specifically, which is why I started with a clean ros build.

Eh, it doesn't really matter, but I think it is good to revert the commit. So that we know only the ones we changed to 3.8 are the ones we need to focus on and the rest we can delete those compile flags.

jwhendy commented 4 years ago

Ok, pushed a new commit which puts it back to 3.7. Good point that in the end, it sounds like we'll probably move to removing these python settings for packages that don't need them. Thus, searching for 3.8 at that point will be an easy way to track down which need updates down the road (and having a false one would confuse things).

Thanks!

bionade24 commented 4 years ago

I think we can remove both statements. But I'll do it in two steps, so that I directly know the cause of the failed build.

acxz commented 4 years ago

Closing this issue since these packages have been synced to the AUR.

jwhendy commented 4 years ago

Shall I go through and remove the python lines from all non-3.8 affected packages?

bionade24 commented 4 years ago

No need for it. I'll do when all packages have built. Same appears fixing the arch array. Or is there any purpose why we should do it ASAP?

Am Donnerstag, 21. November 2019 schrieb John Hendy:

Shall I go through and remove the python lines from all non-3.8 affected packages?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/ros-melodic-arch/ros-melodic-desktop-full/issues/8#issuecomment-55660086

jwhendy commented 4 years ago

@bionade24 definitely not urgent, it's just that this issue held all of this and is now closed. Without activity here, I think I'll likely forget about it. Maybe it should be a project?

At the time, I thought I could just take care of it while it was on my mind as I think a sed script would be pretty easy.

acxz commented 4 years ago

@jwhendy I have created another issue exactly for this discussion: https://github.com/ros-melodic-arch/ros-melodic-desktop-full/issues/11