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 install destination #2345

Merged
merged 9 commits into from Oct 31, 2018
Merged

Fix install destination #2345

merged 9 commits into from Oct 31, 2018

Conversation

@YutoUchimi
Copy link
Contributor

YutoUchimi commented Oct 17, 2018

When I installed jsk_recognition via apt(i.e. rosdep install) and ran source /opt/ros/${ROS_DISTRO}/setup.bash, some sample launch files could not find in /opt/ros/${ROS_DISTRO}/share.

This PR will fix this problem.

After this PR, install ...

  • programs in scripts/ into ${CATKIN_PACKAGE_BIN_DESTINATION}
  • sample and test dir into ${CATKIN_PACKAGE_SHARE_DESTINATION}

cf.
http://wiki.ros.org/catkin/CMakeLists.txt
http://www.ros.org/reps/rep-0122.html

@YutoUchimi YutoUchimi self-assigned this Oct 17, 2018
@k-okada k-okada added this to the 1.2.6 milestone Oct 17, 2018
@YutoUchimi YutoUchimi changed the title Fix installation [WIP] Fix installation Oct 20, 2018
@YutoUchimi

This comment has been minimized.

Copy link
Contributor Author

YutoUchimi commented Oct 23, 2018

@knorth55 Could you please review this if you have time?

jsk_pcl_ros/CMakeLists.txt Outdated Show resolved Hide resolved
@knorth55

This comment has been minimized.

Copy link
Member

knorth55 commented Oct 23, 2018

please format directory name as node_scripts/ -> node_scripts
these two styles are mixed now.

@YutoUchimi

This comment has been minimized.

Copy link
Contributor Author

YutoUchimi commented Oct 23, 2018

Actually, I use node_scripts/ style on purpose.

On debian distibutions, ${CATKIN_PACAKGE_BIN_DESTINATION} and ${CATKIN_PACAKGE_LIB_DESTINATION} are both set to ${CMAKE_INSTALL_PREFIX}/lib/${PROJECT_NAME}.
(cf. http://docs.ros.org/jade/api/catkin/html/user_guide/variables.html)

Unlike share space, in almost all packages, executable programs are located directly in lib space.
e.g.

$ ls /opt/ros/indigo/lib/rosbag/
bag2png.py    fix_md5sums.py         fixbag.py        play        topic_renamer.py
bagsort.py    fix_moved_messages.py  fixbag_batch.py  record
fastrebag.py  fix_msg_defs.py        makerule.py      savemsg.py
$ ls /opt/ros/indigo/lib/roscpp_tutorials/
add_two_ints_client        custom_callback_processing  listener_threaded_spin        notify_connect
add_two_ints_server        listener                    listener_unreliable           parameters
add_two_ints_server_class  listener_async_spin         listener_with_tracked_object  talker
anonymous_listener         listener_class              listener_with_userdata        time_api_sleep
babbler                    listener_multiple           node_handle_namespaces        timers
$ ls /opt/ros/indigo/share/rosbag/
cmake  package.xml
$ ls /opt/ros/indigo/share/roscpp_tutorials/
cmake  launch  package.xml  srv

So I followed this location style.

@knorth55

This comment has been minimized.

Copy link
Member

knorth55 commented Oct 23, 2018

ok, then can you add comment for that?

@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 24, 2018

Unlike share space, in almost all packages, executable programs are located directly in lib space.

this is because they are using catkin_python_setup and catkin_python_install . This is slightly differnt in our use case. https://github.com/ros/ros_comm/blob/melodic-devel/tools/rosbag/CMakeLists.txt#L63

ls /opt/ros/indigo/lib/roscpp_tutorials/

this is because it is binary file.

When we have a scripts/ directory within source tree, and you want to move to that directory. You will type roscd <your package>; cd scripts. Please remember how do you feel when you first recognized that it does not happens in deb environment....

I think we should put script/ node_scripts/ directory under share, in most of cases.

@YutoUchimi

This comment has been minimized.

Copy link
Contributor Author

YutoUchimi commented Oct 30, 2018

I see. Then I will follow below guideline (JSK original).

  • If a package has setup.py at the top level of the package and modules under src/${PROJECT_NAME}/, then each python executable should be installed into ${CATKIN_PACKAGE_BIN_DESTINATION}.
    This respects ordinal installation style.
  • If not, directories including python executables such as scripts and node_scripts should be installed into ${CATKIN_PACKAGE_SHARE_DESTINATION}.
    This is for ROS-beginners who want to find that executables easily with roscd <your package>; cd scripts.
@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented Oct 31, 2018

https://travis-ci.org/jsk-ros-pkg/jsk_recognition/jobs/448650345

jsk_pcl_ros/test_results/jsk_pcl_ros/rosunit-test_octree_voxel_grid.xml: 1 tests, 0 errors, 1 failures, 0 skipped
@YutoUchimi

This comment has been minimized.

Copy link
Contributor Author

YutoUchimi commented Oct 31, 2018

Travis passed!

@k-okada k-okada merged commit a4c1bad into jsk-ros-pkg:master Oct 31, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@YutoUchimi YutoUchimi deleted the YutoUchimi:fix_installation branch Oct 31, 2018
@k-okada

This comment has been minimized.

Copy link
Member

k-okada commented on resized_image_transport/CMakeLists.txt in 2ee82c4 Nov 2, 2018

I think you can remove this line because we've moved from svn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.