CMake project config which suports non-ROS build

[vebjornjr]

Back again this year with a PR to my issue #16 from last year.

It installs and works as expected with a plain non-ROS build (pure C++ lib).

I also tested with ROS Melodic; the launch file, tests and using it in another ROS package. (However, I'm not that experienced with ROS so you should check it ourselves that it installs and everything as expected.)

The pure C++ lib part of the CMake is mostly according to modern CMake best practices, the ROS part is just adopted from the original configuration. The if statement with BUILD_WITH_ROS was needed twice since the catkin_package() was needed before add_library().

I guess a version bump is needed if this is merged. What version do you want?

It would be nice if @Timple also could take a look at it.

[Timple]

Thank you for asking my input. I took a quick swing at it before diving into the changes:

/usr/bin/ld: /home/timple/ros/devel/.private/steering_functions/lib/libsteering_functions.a(cc00_dubins_state_space.cpp.o): relocation R_X86_64_PC32 against symbol `_ZTVN8steering23CC00_Dubins_State_SpaceE' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/utils.dir/build.make:285: /home/tim/ros/devel/.private/harvey/lib/harvey_utils.so] Error 1
make[1]: *** [CMakeFiles/Makefile2:7915: CMakeFiles/utils.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:141: all] Error 2

Does this look familiar or do you have some easy things to check for me?

Adding this fixes the issue:

target_compile_options(${PROJECT_NAME}
    PRIVATE
        -fPIC
)

But I'm not sure if this is desired or common practice.

[vebjornjr]

Thank you for asking my input. I took a quick swing at it before diving into the changes:

/usr/bin/ld: /home/timple/ros/devel/.private/steering_functions/lib/libsteering_functions.a(cc00_dubins_state_space.cpp.o): relocation R_X86_64_PC32 against symbol `_ZTVN8steering23CC00_Dubins_State_SpaceE' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/utils.dir/build.make:285: /home/tim/ros/devel/.private/harvey/lib/harvey_utils.so] Error 1
make[1]: *** [CMakeFiles/Makefile2:7915: CMakeFiles/utils.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:141: all] Error 2

Does this look familiar or do you have some easy things to check for me?

Adding this fixes the issue:

target_compile_options(${PROJECT_NAME}
    PRIVATE
        -fPIC
)

But I'm not sure if this is desired or common practice.

Hmm. I think the idiomatic CMake way to set the flag is:

option(CMAKE_POSITION_INDEPENDENT_CODE "Build position independent code." ON)

or

set_property(TARGET ${PROJECT_NAME} PROPERTY POSITION_INDEPENDENT_CODE ON)

However, this flag is always enabled by default in CMake when building a shared library (creating an libsteering_functions.so file), e.g. by setting BUILD_SHARED_LIBS to ON.

It seems you are building this library as static (libsteering_functions.a file), but your library harvey_utils.so is built as shared library. Just out of curiosity, can you try to build and link to the steering_functions lib on master branch without altering your harvey_utils toolchain setup?

[Timple]

Just out of curiosity, can you try to build and link to the steering_functions lib on master branch without altering your harvey_utils toolchain setup?

It does.

Comparing compilation with the master branch, I notice that the ros node is also binary different. Perhaps this gives an indication?

edit: option(CMAKE_POSITION_INDEPENDENT_CODE "Build position independent code." ON) also works.

[vebjornjr]

Could you take a look now?

I noticed that catkin builds the shared library of the master branch so by introducing the explicit CMake option (and defaulting it to OFF), I had changed the behaviour compared to maste branch. I removed the explicit option and now this branch also defaults to shared library with catkin.

I also changed back to the original behavior of the ROS node executable. It builds with all the source files instead of linking to the (shared) library steering_functions. Which is a rather strange choice, but to avoid breaking things I reverted back. (Doesn't matter to me since I don't use ROS).

[Timple]

It works now. Only the removal of option(BUILD_SHARED_LIBS "Build as shared library" OFF) seemed to be enough.

Personally I would keep the restructure you had. That the node actually uses the library. You are right that this makes most sense.

[Timple]

Still works :slightly_smiling_face:. So no objections for me.

I'm a user however, not a maintainer, so this is up to @hbanzhaf

[hbanzhaf]

@vebjornjr Could you please also update the README with how to build this package with and without ROS?

[vebjornjr]

Done. I guess a version bump is needed too. What version do you want?

[hbanzhaf]

@Timple To which version number should we increment?

[Timple]

As far as I'm concerned it's a backwards compatible change. So 0.2.0?

[hbanzhaf]

@vebjornjr Happy to merge this PR once the other minor comments are also resolved.

[vebjornjr]

Which comments to you refer to, @hbanzhaf? I was under the impression that I had resolved all.

[vebjornjr]

Friendly ping @hbanzhaf

[vebjornjr]

Hmm weird, these comments were hidden for me until now. Anyway, all should be fixed now.