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 emptycaliper-gotcha
would provide externalgotcha
target
- When internal GOTCHA:
${CALIPER_INTERNAL_GOTCHA}
would provide OBJECT library sourcescaliper-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.