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

fix shebang line for python3 #158

Open
wants to merge 1 commit into
base: noetic-devel
Choose a base branch
from

Conversation

mikaelarguedas
Copy link
Member

Currently in Noetic/Focal the script cannot be run with rosrun:

/usr/bin/env: ‘python’: No such file or directory

Signed-off-by: Mikael Arguedas <[email protected]>
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-noetic-ninjemys-release/14262/22

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/poll-kinetic-users-kinetic-eol-in-april-what-is-your-migration-plan/17731/10

@sloretz
Copy link

sloretz commented Dec 3, 2020

I would recommend fixing by using catkin_install_python here instead of changing the shebang.

install(PROGRAMS scripts/class_loader_headers_update.py

Installing python-is-python3 might be another workaround.

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Dec 3, 2020

This is a noetic-devel branch.
Why is it recommended to use one of the other workarounds ? Is it to support non debian platforms ?

@sloretz
Copy link

sloretz commented Dec 3, 2020

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Dec 3, 2020

I remember now why I used catkin_install_python on most PRs but not this one. This CMake is made to be working even in an environment without catkin installed. So we can't catkin_install_python outside a if(catkin_FOUND) block. But that means duplicating the install directive in the 2 branches of that if and use install(PROGRAMS in the else case (although that means that on a linux without catkin the installed file won't have the right shebang line...) WDYT?

@mikaelarguedas
Copy link
Member Author

@sloretz any feedback on the previous comment ?

@sloretz
Copy link

sloretz commented Dec 14, 2020

This CMake is made to be working even in an environment without catkin installed. So we can't catkin_install_python outside a if(catkin_FOUND) block. But that means duplicating the install directive in the 2 branches of that if and use install(PROGRAMS in the else case (although that means that on a linux without catkin the installed file won't have the right shebang line...) WDYT?

Hmm, the catkin_install_python() logic is a lot to reproduce in the non-catkin environment. Changing the shebang in the file won't work on Windows, but works for the non-catkin case on linux. Changing the install rule to catkin_install_python when catkin is found will work on windows, but won't help the non-catkin case.

I would lean towards using catkin_install_python if catkin is found and requiring non-catkin workflows to install python-is-python3 on Debian platforms, but a decision on what non-required platforms should be better supported is one the maintainer should make. @nuclearsandwich thoughts?

@hidmic
Copy link

hidmic commented Sep 10, 2021

I would lean towards using catkin_install_python if catkin is found and requiring non-catkin workflows to install python-is-python3 on Debian platforms, but a decision on what non-required platforms should be better supported is one the maintainer should make. @nuclearsandwich thoughts?

This is a sound path forward. @nuclearsandwich ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants