search

ospray / anari-ospray

Translation layer from ANARI to OSPRay, ANARILibrary and ANARIDevice "ospray".
Apache License 2.0
17 stars 5 forks source link

Superbuild #1

Closed szellmann closed 2 years ago

szellmann commented 2 years ago

These are a couple of fixes to make the superbuild run more smoothly for me. As I'm on ARM Mac, I need to build TBB and Embree from source, so I added cmake options for that.

I had some issues with adding the (meta) project itself as an external dependency, as is done by the superbuild script. In particular, it would try to build the device library in-source, which is however deactivated by the project. I fixed that by specifying a BINARY_DIR inside the current build dir, but I'm not sure if this is desirable. That way, the project builds successfully for me, but solving the issue this way might be a bit controversial, so just let me know if you want this solved differently / want me to use different paths / naming conventions etc.

In regards to the latter, the docs say that if SOURCEDIR is specified cmake will deduce the other variables from that (cf. https://cmake.org/cmake/help/latest/module/ExternalProject.html, "If any of the above ..._DIR options are not specified, their defaults are computed as follows. [..]"), but I found that (seemingly) this is not the case, at least with my cmake version (which is 3.20.3).

johguenther commented 2 years ago

Hi Stefan, many thanks for your contributions!

This one looks good, indeed except some questions regarding the BINARY_DIR change; my understanding of CMake docu is that it's default value is computed independent of SOURCE_DIR (which is what we observe and also it is not set – SOURCE_SUBDIR is).

So, you need to override the CMAKE_DISABLE_IN_SOURCE_BUILD and build in-source? Does adding BUILD_IN_SOURCE ON to ExternalProject_Add do the trick (which could be an additional (hidden) option() in the superbuild)?

Cheers Johannes

szellmann commented 2 years ago

Hi Johannes, I've just been trying to repro that issue but am not able to right now. That is, when using the current change set but w/o the commit adding the BINARY_DIR change I can still successfully perform a superbuild, and the build dir's directory structure looks as follows:

total 56
drwxr-xr-x   9 stefan  staff   288B Jan 12 18:38 .
drwxr-xr-x  47 stefan  staff   1.5K Jan 12 18:37 ..
-rw-r--r--   1 stefan  staff    14K Jan 12 18:37 CMakeCache.txt
drwxr-xr-x  19 stefan  staff   608B Jan 12 18:41 CMakeFiles
-rw-r--r--   1 stefan  staff   5.4K Jan 12 18:38 Makefile
drwxr-xr-x   4 stefan  staff   128B Jan 12 18:37 anari-ospray-prefix
drwxr-xr-x   4 stefan  staff   128B Jan 12 18:37 anari-prefix
-rw-r--r--   1 stefan  staff   1.6K Jan 12 18:38 cmake_install.cmake
drwxr-xr-x   4 stefan  staff   128B Jan 12 18:37 ospray-prefix

I'm not sure what has changed in the meantime, as all the external libs are external cmake projects, but for lack of reproducibility I wonder if the issue is actually sitting in front of the screen :-)

Therefore proposing to not merge the commit in question for now (I can adapt the PR if you want me to), and I'll report back if I run into the same issue again and can provide a reproducer.

Cheers, Stefan

johguenther commented 2 years ago

I merge the first commit, but left the other two out: as you mentioned, the issue that BINARY_DIR should be solving cannot be reproduced; also, to install the ANARIViewer we must not use a custom INSTALL_COMMAND. Please comment if I missed something.