Add extra step to Makefile config target to force CMake rescan installed packages #4305
Conversation
… once building with BUILD_MISSING_DEPS=1 completes. This is necessary as CMake does not detect Imath include directory and errors during build. Signed-off-by: kuba <kuba456@gmail.com>
kaarrot
changed the title
| @@ -207,7 +207,7 @@ ifneq (${CLANG_FORMAT_EXCLUDES},) | |||
| endif | |||
|
|
|||
| ifneq (${BUILD_MISSING_DEPS},) | |||
| MY_CMAKE_FLAGS += -DBUILD_MISSING_DEPS:BOOL=${BUILD_MISSING_DEPS} | |||
| MY_CMAKE_FLAGS += -DOpenImageIO_BUILD_MISSING_DEPS:STRING=all | |||
There was a problem hiding this comment.
Good catch, thanks.
Also note that since these use my new set_cache() macro, you can also set this via environment variable... and therefore, via make argument. That is,
make OpenImageIO_BUILD_MISSING_DEPS=all
works and doesn't need any special logic in the Makefile.
Maybe we should eliminate BUILD_MISSING_DEPS entirely and just use the new name?
There was a problem hiding this comment.
Maybe we should eliminate BUILD_MISSING_DEPS entirely and just use the new name?
I agree - also nice we can reuse same argument names in both places.
I'll address that.
| if [ "${BUILD_MISSING_DEPS}" = "1" ] ; then \ | ||
| ${CMAKE} ${working_dir} ; \ | ||
| fi ; \ |
There was a problem hiding this comment.
I'm fine with this change, at worst it's harmless, but I'm not sure I understand why a second invocation is needed. Can you ELI5?
There was a problem hiding this comment.
I may actually got to the bottom of the problem which makes the above change redundant.
The libutil target - (setup_oiio_util_library function) does not account for the include path to the "missing required" dependencies just built. At the build time, they are located under ./deps/dist/include). The only include path currently set there is the final install prefix location.
So adding $<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/deps/dist/include> seems to address it.
|
I'm going to close this one and continue in #4306 |
This is the continuation of [4305 ](#4305 extra step to Makefile config target to force CMake to rescan). The error occurs when configuring with `make OpenImageIO_BUILD_MISSING_DEPS=all` option which fails the build as `libuitil` does not have `./dep/dist/include` on the path. The original solution was to rerun cmake and let it rescan paths. This step was only required to run once - after the initial CMake configuration step. This kind off worked but I'm still unsure why. So instead of that we can just add the missing location to CMakeLists. Signed-off-by: kuba <kuba456@gmail.com>
Something for consideration as I'm not sure what would be the best way to handle this.
Building with -DOpenImageIO_BUILD_MISSING_DEPS=all fails on Linux with the following error:
Steps to reproduce:
Initially I thought this is related with OpenImageIO/Imath.h expecting system includes:
include_directories(BEFORE ${IMATH_INCLUDES} ${OPENEXR_INCLUDES})As it turns out this was redherring and the problem could be with CMake "state?" (Maybe a specific CMake version
3.27.7to blame).A potential fix - after the initial configuration completes - is to reconfigure the project one more time:
cmake ..which resolves the error.The second reconfiguration command is also protected by
${build_dir}/${BUILDSENTINEL}hence runs only once - during the initial configuration. If anyone is able to reproduce this or have any thoughts what the root problem may be - please chime in.