ospray_create_library broken when including ospray as library

[pnav]

When moving from OSPRay 1.7.3 to 1.8.5, we now get these errors for ospray_install_library(), which is called by ospray_create_library():

 CMake Error at third-party/ospray/install/lib/cmake/ospray-1.8.5/macros.cmake:204 (install):
   install EXPORT given no DESTINATION!
 Call Stack (most recent call first):
   third-party/ospray/install/lib/cmake/ospray-1.8.5/macros.cmake:295 (ospray_install_library)
   src/ospray/CMakeLists.txt:71 (ospray_create_library)

It appears that we hit this because OSPRAY_CMAKECONFIG_DIR is not set (see macros.cmake line 205). I'm not sure we should need to define this, since we're not installing OSPRay cmake foo, rather just our own (or maybe we should be?).

If I set OSPRAY_CMAKECONFIG_DIR to a reasonable value, the cmake config succeeds, but at makefile generation I get errors about all our non-OSPRay components:

CMake Error: install(EXPORT "ospray_Exports" ...) includes target "gxy_renderer" which requires target "gxy_data" that is not in the export set.

 CMake Error: install(EXPORT "ospray_Exports" ...) includes target "gxy_renderer" which requires target "gxy_framework" that is not in the export set.

 CMake Error: install(EXPORT "ospray_Exports" ...) includes target "gxy_sampler" which requires target "gxy_data" that is not in the export set.

 CMake Error: install(EXPORT "ospray_Exports" ...) includes target "gxy_sampler" which requires target "gxy_framework" that is not in the export set.

It seems to me that the ospray_create_library() macro now has some OSPRay-only assumptions built into it, such that external components can no longer use it.

[pnav]

I emailed this issue to Jeff and Carson a while back, but adding it here so that this use case doesn't get lost. I'm not sure whether OSPRay claims to follow semantic versioning principles, but it is frustrating as a user for the behavior of install-exposed functionality (e.g. the macros included in lib/cmake/ospray-x.y.z) to change in a backward-incompatible way on a minor version increment. I understand that under the new cmake implementation, much has changed. I would appreciate if including libospray etc within another application via cmake find_target was included in your regression testing to catch cases like this, or if the breaking was intentional and the macros are now deprecated, that they be marked as such and that backwards-incompatible changes are saved for major releases (like the upcoming 2.0 :-) )

[pnav]

Here is a minimal reproducer that will succeed with OSPRay 1.7.x and fail with OSPRay 1.8.x.

CMakeLists.txt:

cmake_minimum_required(VERSION 3.9)
project(cmake-reproducer VERSION 0.0.1)

find_package(ospray)
if (ospray_DIR)
  set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${OSPRAY_CMAKE_ROOT})
  include(macros)
  include(ospray_macros)
  include(ispc)
  include_directories(${OSPRAY_INCLUDE_DIRS})

  ospray_create_library( cmake-reproducer CmakeReproducer.cpp
                         LINK ${OSPRAY_LIBRARIES}
                         COMPONENT lib
                       )
endif(ospray_DIR)

CmakeReproducer.cpp:

#include <iostream>

#include <ospray/ospray.h>

void
force_ospray_library_load() {}

int initialize (int argc, char** argv)
{
  std::cout << "cmake reproducer initializer called with " << argc << " arguments\n";

  force_ospray_library_load();
  std::cout << "ospray library loaded\n";

  return 0;
}

[jeffamstutz]

Hi Paul,

Yes, semantic versioning is what represents our published public C-API: everything found in ospray.h follows this versioning scheme.

Once you start depending on our internals (CMake, C++, and ISPC), backward compatibility is much less stable as this is more difficult to guarantee. We use ospray_create_library() mostly to setup install target components correctly so our RPMs and installers contain the right division of installed contents, which is targeted at that use case only.

Consuming OSPRay itself does not require anything special with CMake in order to use, and we added modern CMake targets with v1.8.0 to make that even easier (usage documented here). We recommend that downstream projects use "plain CMake" as much as possible. The CMake targets we export are the most "future proof" way to accept OSPRay internals beyond the main ospray::ospray C-API target.

With this in mind, you can compile your reproducer with:

cmake_minimum_required(VERSION 3.9)
project(cmake-reproducer VERSION 0.0.1)

find_package(ospray REQUIRED)

add_library(cmake-reproducer CmakeReproducer.cpp)
target_link_libraries(cmake-reproducer ospray::ospray)

---

A separate issue is using OSPRay for its ISPC macros to compile for potentially N different architectures, and whether you depend on OSPRay's compiled in targets or are doing this all application-side (what I think Galaxy is probably looking for).

The easiest way to get at our internals from an install is to include ${OSPRAY_USE_FILE}, which looks like:

cmake_minimum_required(VERSION 3.9)
project(cmake-reproducer VERSION 0.0.1)

find_package(ospray REQUIRED)
include(${OSPRAY_USE_FILE})

add_library(cmake-reproducer CmakeReproducer.cpp)
target_link_libraries(cmake-reproducer ospray::ospray)

This will bring in the ispc and macros files that we use to encapsulate some of these things. I recommend that you use the ISPC targets compiled into OSPRay (that come into scope with ${OSPRAY_USE_FILE}) as you have to keep your targets compatible with whatever Embree was compiled with, which OSPRay takes care of.

However, if you want to have a different target list than what OSPRay was configured with (at your own risk), then follow the same procedure we do when configuring OSPRay based on what Embree we found at CMake time (code here).

Hope this helps, Jeff

[pnav]

Jeff,

Thank you for the quick reply and the potential fix. I will verify that this works in the larger Galaxy build without unintended side-effects. Also, thank you for the tip about OSPRAY_USE_FILE, it is certainly our intent to use the same ISPC / Embree that OSPRay uses!

I am glad to hear that OSPRay does follow semantic versioning principles. Please accept a bit of feedback from an OSPRay customer: to us, everything that is deployed by make install is part of your interface, whether intentional or not, and this includes the cmake that is deployed in lib/cmake/ospray-x.y.z. Indeed, that is a customary spot for software to deploy helpful cmake scripts, so I am surprised that you characterize them as "your internals". You are making an affirmative decision to deploy them via install, after all :-) If they are only needed at OSPRay build time within the OSPRay cmake environment (which, by your discussion above, appears to be your intent), may I suggest removing them from the OSPRay install so that your customers do not inadvertently use them in a way you do not intend to support. My $0.02.

All best,

Paul

[jeffamstutz]

Yeah, I definitely hear you loud and clear. I mostly default to "the customer is always right", so feedback well taken. :)

What Galaxy depends on is really the infrastructure used to extend ospray::ospray_module_ispc from an install, which we have no documentation on. Simply documenting "this is how you get at our ISPC macros with the least likely chance of us breaking you" could have gone a long way here.

Given how much CMake refactoring that has been done for OSRPay v2.0 (what's on devel thus far), depending on our CMake is probably easier to keep stable between versions. So maybe this can be something to officially track in future releases (and document!).

What REALLY needs to happen way more than anything else: ISPC needs to have 1st-class integration into CMake so you can just add .ispc files to add_library() and add_executable() and not bother with CMake macros that do this manually. Similar work was done to do this with CUDA .cu files recently (done by the one and only @robertmaynard), so it's totally doable as long as someone actually sits down and does it. (!) Then everyone could benefit equally, not just OSPRay. :)

For now though, the contents of ispc.cmake and use of the ospray_add_library() (which does the cpp/ispc compile split, a terribly named macro) are the smallest necessary pieces to work with. Perhaps only working with ospray_ispc_compile() is even safer.

[pnav]

Thanks for the additional suggestions, we'll look to streamline our dependence on these. I appreciate that much of this is work-around to integrate ISPC into cmake... do they have a ticketing system where I could pester them instead of you? 🤔 😄 😉

[jeffamstutz]

Closing this here as ospray_create_library() was removed in v2.0.0-alpha.

The new ispc_target_add_sources() macro is much more flexible and minimally invasive (as well as better named).

[jeffamstutz]

Note that ispc_target_add_sources() appends sources already specified on the given target name, and can contain both C++ and ISPC source files (i.e. you can omit sources when creating the CMake target and specify all of them through this macro, if you so choose).