LLNL/Caliper

Rename internal GOTCHA target

jrmadsen opened this issue · 6 comments

  • Since Caliper creates a gotcha OBJECT library when using the internal GOTCHA instead of a SHARED library, it creates a problem for me when I want to include GOTCHA in my project as a submodule and include caliper as a submodule:
    • I cannot add GOTCHA first as a submodule and set Caliper to "find" an external GOTCHA because it hasn't been built yet
    • I cannot add Caliper first and then add GOTCHA as a submodule because the target name gotcha is already taken.

In general, this pseudo-code would work:

add_library(caliper-gotcha INTERFACE)

if(EXTERNAL_GOTCHA)
    target_link_libraries(caliper-gotcha INTERFACE gotcha)
    set(CALIPER_INTERNAL_GOTCHA )
else()
    add_subdirectory(ext/gotcha)
    set(CALIPER_INTERNAL_GOTCHA $<TARGET_OBJECTS:caliper-internal-gotcha>)
endif()

# ...
add_library(caliper ${CALIPER_INTERNAL_GOTCHA} ...)
target_link_libraries(caliper ... PRIVATE caliper-gotcha)
  • When external GOTCHA:
    • ${CALIPER_INTERNAL_GOTCHA} would be empty
    • caliper-gotcha would provide external gotcha target
  • When internal GOTCHA:
    • ${CALIPER_INTERNAL_GOTCHA} would provide OBJECT library sources
    • caliper-gotcha would provide nothing

Well, actually, you may want target_link_libraries(caliper ... PUBLIC caliper-gotcha) and just export it so that CMake projects linking to Caliper will get the linking to GOTCHA also.

@jrmadsen : so I was the original reason for there being an external Gotcha option, back when Caliper didn't bundle Gotcha that was (arguably) necessary. We might just want to say that Caliper uses the Gotcha it was downloaded with and get rid of external Gotcha, if we're already bundling it I don't know whether it makes sense to have searching capabilities. Not a knock on your implementation, just a design decision

Eh, I tend to also leave both options open in the event that the bundled one isn't up to date with the latest release or to leave open the possibility of testing a new dev feature in the secondary library. Like for caliper in timemory, there is a TIMEMORY_USE_CALIPER option and a TIMEMORY_BUILD_CALIPER options where the latter does a git submodule checkout and add subdirectory instead of find package. I find this simplified things more than doing custom builds like what y'all did

Shouldn't it be sufficient to just rename the internal gotcha target? I think that would solve at least the second problem ("gotcha" target name already taken). I'm putting something up to do that.

Definitely multiple ways to do it, only real issue is the external gotcha having a different target name so if you continue to support that, that's got to be handled somewhere

Saw the PR. Thanks by the way.