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

Re-enabled tracing for external project builds #140

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jellehierck
Copy link

In 01c9f0e, the tracing functionality was wrapped with an "project name guard" (if that is a real name) preventing it from running when embedded in another project:

if(${CMAKE_PROJECT_NAME} STREQUAL ${PROJECT_NAME})
  if (ENABLE_TRACING)
    ...
  endif()
endif()

However, this caused tracing to be disabled for any projects which include cactus_rt in CMake with FetchContent as suggested in the README. This is IMO one of the best features of cactus_rt and I would love to have it back.

This pull request reverts the "project name guard" around the tracing sources and protobuf folder include. I also included a status message indicating that tracing will also be built.

I assume this functionality was added because of ROS 2 support (very excited about that BTW!) since the tracing might interfere? If so, perhaps another option is to enable/disable the tracing based on the ENABLE_ROS2 flag instead of the project name guard, making it configurable.

@jellehierck
Copy link
Author

As an addition which might be included in the README:

You can set/unset the enable options in CMake with the following code by calling set() BEFORE FetchContent_MakeAvailable():

include(FetchContent)
FetchContent_Declare(
    cactus_rt
    GIT_REPOSITORY [email protected]:jellehierck/cactus-rt.git
    GIT_TAG d0a0854feaef7750c24a703db7e29dc71f019123 # latest commit as of 02-10-2024
)
set(ENABLE_TRACING OFF)  # <-- Disable tracing BEFORE FetchContent_MakeAvailable
FetchContent_MakeAvailable(cactus_rt)

This was inspired by this stackoverflow question

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

Successfully merging this pull request may close these issues.

2 participants