search

RenderKit / oidn

Intel® Open Image Denoise library
https://www.openimagedenoise.org/
Apache License 2.0
1.77k stars 164 forks source link

0.8.2 build fails in the presense of the previous version, it picks the pre-installed headers over the current ones #23

Closed yurivict closed 4 years ago

yurivict commented 5 years ago 
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:21:9: warning: 'OIDN_VERSION_PATCH' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION_PATCH 2
        ^
/usr/local/include/OpenImageDenoise/version.h:21:9: note: previous definition is here
#define OIDN_VERSION_PATCH 1
        ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:22:9: warning: 'OIDN_VERSION' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION 802
        ^
/usr/local/include/OpenImageDenoise/version.h:22:9: note: previous definition is here
#define OIDN_VERSION 801
        ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:23:9: warning: 'OIDN_VERSION_STRING' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION_STRING "0.8.2"
        ^
/usr/local/include/OpenImageDenoise/version.h:23:9: note: previous definition is here
#define OIDN_VERSION_STRING "0.8.1"
        ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:44:3: error: redefinition of enumerator 'OIDN_DEVICE_TYPE_DEFAULT'
  OIDN_DEVICE_TYPE_DEFAULT = 0, // select device automatically
  ^
/usr/local/include/OpenImageDenoise/oidn.h:44:3: note: previous definition is here
  OIDN_DEVICE_TYPE_DEFAULT = 0, // select device automatically
  ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:46:3: error: redefinition of enumerator 'OIDN_DEVICE_TYPE_CPU'
  OIDN_DEVICE_TYPE_CPU = 1, // CPU device
  ^
/usr/local/include/OpenImageDenoise/oidn.h:46:3: note: previous definition is here
  OIDN_DEVICE_TYPE_CPU = 1, // CPU device
  ^
atafra commented 5 years ago

Please provide the compiler, version, OS, compile flags, etc. too. I never saw this issue. It looks like you have two different versions of OIDN installed, and you include the headers from both versions, which of course will not work.

yurivict commented 5 years ago

It looks like you have two different versions of OIDN installed, and you include the headers from both versions, which of course will not work.

Yes, this was a problem that the previous version is installed. It should pick its current headers over the pre-installed headers, this is a bug in oidn.

atafra commented 5 years ago

The previous version being installed too in itself won't cause such an issue. If you look into oidn.h/hpp you can see that it includes its headers locally (#include "version.h"). I don't see how this could fail. But your error message indicates that you include oidn.h twice from two different versions in the same file, which will indeed fail. But what's the purpose of that?

atafra commented 5 years ago

Or is this error for building OIDN itself?

yurivict commented 5 years ago

I am just building the oidn port. When the previous version was installed, the build was failing with the above message.

yurivict commented 5 years ago

Or is this error for building OIDN itself?

Yes, this is the OIDN build error.

atafra commented 5 years ago

OK, thanks for clarifying, I'll look into it.

atafra commented 5 years ago

I can't reproduce the issue on my machine. Even though I have the previous headers in /usr/local/include, it still picks the current one during compilation. It would help a lot if you could provide the compiler and CMake versions, and the OS you use. Thanks!

yurivict commented 5 years ago

clang-6.0.1 cmake-3.13.4 OS: FreeBSD 11.2 amd64 Building it from the port graphics/oidn (version 0.8.2) while oidn-0.8.1 is installed.

atafra commented 5 years ago

We actually don't support/test OIDN on FreeBSD (yet), but I'll have a look. It works fine on Linux.

atafra commented 5 years ago

Could you please post the exact clang invocation (with all parameters) for denoise.cpp? You need to call make with VERBOSE=1.

yurivict commented 5 years ago 
[ 97%] Building CXX object examples/CMakeFiles/denoise.dir/denoise.cpp.o
cd /usr/ports/graphics/oidn/work/.build/examples && /usr/bin/c++  -DMKLDNN_THR=MKLDNN_THR_TBB -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -I/usr/ports/graphics/oidn/work/oidn-0.8.2 -I/usr/local/include -I/usr/ports/graphics/oidn/work/oidn-0.8.2/include -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -std=c++11 -fvisibility-inlines-hidden -fPIC -Wformat -Wformat-security -fstack-protector-all  -Wno-pass-failed -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -D_FORTIFY_SOURCE=2 -o CMakeFiles/denoise.dir/denoise.cpp.o -c /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp
--- examples/CMakeFiles/denoise.dir/image_io.cpp.o ---
[ 98%] Building CXX object examples/CMakeFiles/denoise.dir/image_io.cpp.o
cd /usr/ports/graphics/oidn/work/.build/examples && /usr/bin/c++  -DMKLDNN_THR=MKLDNN_THR_TBB -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -I/usr/ports/graphics/oidn/work/oidn-0.8.2 -I/usr/local/include -I/usr/ports/graphics/oidn/work/oidn-0.8.2/include -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -std=c++11 -fvisibility-inlines-hidden -fPIC -Wformat -Wformat-security -fstack-protector-all  -Wno-pass-failed -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -D_FORTIFY_SOURCE=2 -o CMakeFiles/denoise.dir/image_io.cpp.o -c /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/image_io.cpp
--- examples/CMakeFiles/denoise.dir/denoise.cpp.o ---
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:21:9: warning: 'OIDN_VERSION_PATCH' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION_PATCH 2
        ^
/usr/local/include/OpenImageDenoise/version.h:21:9: note: previous definition is here
#define OIDN_VERSION_PATCH 1
        ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:22:9: warning: 'OIDN_VERSION' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION 802
        ^
/usr/local/include/OpenImageDenoise/version.h:22:9: note: previous definition is here
#define OIDN_VERSION 801
        ^
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/examples/denoise.cpp:27:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/timer.h:19:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/common/platform.h:37:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.hpp:20:
In file included from /usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/oidn.h:23:
/usr/ports/graphics/oidn/work/oidn-0.8.2/include/OpenImageDenoise/version.h:23:9: warning: 'OIDN_VERSION_STRING' macro redefined [-Wmacro-redefined]
#define OIDN_VERSION_STRING "0.8.2"
        ^
/usr/local/include/OpenImageDenoise/version.h:23:9: note: previous definition is here
#define OIDN_VERSION_STRING "0.8.1"
        ^
atafra commented 5 years ago

Thanks, I could reproduce the issue. It seems that this is already fixed in the latest devel branch. Could you please check?

yurivict commented 5 years ago

The devel branch fails in a different way:

In file included from /usr/ports/graphics/oidn/work/oidn-4a0d15c/common/tasking.cpp:17:
/usr/ports/graphics/oidn/work/oidn-4a0d15c/common/tasking.h:22:10: fatal error: 'tbb/task_scheduler_init.h' file not found
#include "tbb/task_scheduler_init.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
atafra commented 5 years ago

It looks like a very old version of TBB is detected, but the detection code did not change, so this is strange. What is the value of the TBB_ROOT CMake option, and which version of TBB does it point to?

yurivict commented 5 years ago

tbb-2019.1_1 is used. TBB_ROOT isn't specifically set by the port. 0.8.2 detects tbb fine.

yurivict commented 5 years ago 
work/.build/CMakeCache.txt:TBB_ROOT:PATH=/usr/local
atafra commented 5 years ago

Could you please post the clang invocation for tasking.cpp? Does /usr/local/include/tbb contain task_scheduler_init.h?

yurivict commented 5 years ago

Could you please post the clang invocation for tasking.cpp?

--- common/CMakeFiles/common.dir/tasking.cpp.o ---
cd /usr/ports/graphics/oidn/work/.build/common && /usr/bin/c++  -DMKLDNN_THR=MKLDNN_THR_TBB -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -I/usr/ports/graphics/oidn/work/oidn-4a0d15c -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -std=c++11 -fvisibility-inlines-hidden -fPIC -Wformat -Wformat-security -fstack-protector-all  -Wno-pass-failed -O2 -pipe -fno-omit-frame-pointer -fstack-protector -fno-strict-aliasing -fno-omit-frame-pointer -D_FORTIFY_SOURCE=2 -o CMakeFiles/common.dir/tasking.cpp.o -c /usr/ports/graphics/oidn/work/oidn-4a0d15c/common/tasking.cpp

Does /usr/local/include/tbb contain task_scheduler_init.h?

Yes.

atafra commented 5 years ago

What happens if you try to compile the following code with c++ without any other parameters?

#include "tbb/task_scheduler_init.h"
int main() { return 0; }

This should compile without errors.

yurivict commented 5 years ago

c++ -I /usr/local/include/ -c x.cpp succeeds.

atafra commented 5 years ago

Why did you add -I /usr/local/include/? Isn't that one of the default include paths in FreeBSD?

yurivict commented 5 years ago

-I /usr/local/include is required for non-base includes on FreeBSD.

yurivict commented 5 years ago

In cmake, you need to add -I${CMAKE_INSTALL_PREFIX}/include as a compiler argument.

atafra commented 5 years ago

No, that's not the right solution. The missing -I must be the location of the TBB installation, which in your case happens to be in ${CMAKE_INSTALL_PREFIX}, but that could be different. What's strange is that on my Linux system the TBB include directory is added, but in your case it's not. Could you check what's the value of the TBB_INCLUDE_DIR advanced CMake option for OIDN devel?

yurivict commented 5 years ago

What's strange is that on my Linux system the TBB include directory is added, but in your case it's not.

On Linux packages are installed into /usr and it is also a default includes and libs base, but on BSDs this isn't the case.

No, that's not the right solution. The missing -I must be the location of the TBB installation, which in your case happens to be in ${CMAKE_INSTALL_PREFIX}, but that could be different.

It always installs all ports into /usr/local, which is the same as ${CMAKE_INSTALL_PREFIX}. It can't be different, practically speaking.

atafra commented 5 years ago

I'm not familiar with ports, but a library can be located anywhere, and the TBB_ROOT option must be respected, so ${CMAKE_INSTALL_PREFIX} cannot be used in general.

atafra commented 5 years ago

The proper solution is to make sure that TBB_INCLUDE_DIR is added when compiling. This happens on Linux, but not on FreeBSD, and I don't know why.

yurivict commented 5 years ago

Setting -DTBB_ROOT:STRING=/usr/local -DTBB_INCLUDE_DIR:STRING=/usr/local/include doesn't help, it still can't find "tbb/task_scheduler_init.h".

By the way, the correct way to include headers from the system folders is using <>, not "".

jmengintel commented 4 years ago

We have decided not to support FreeBSD at this point, and I have closed the issue for now. However, if you happen to come up with a fix and would like to submit it, we will certainly consider it. Thank you!

yurivict commented 4 years ago

Ok, I'll delete the port for odin from the FreeBSD repositories if you don't feel like fixing your bugs.