Test if json_fwd.hpp exists #8345
Conversation
|
Starting build on |
|
Build failed on mac1014/python3. Failing tests: |
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests: |
There was a problem hiding this comment.
As mentioned in my other comment, the default behavior when nothing is defined should be to include the full header. This is the more common way to handle issues like this and will work if for some reason whoever includes the header links without using the CMake target. That said, given the problem with installed nlohman_json not having the header, it's probably better not to try to include it at all.
graf3d/eve7/inc/ROOT/REveElement.hxx
Outdated
| @@ -18,7 +18,11 @@ | |||
|
|
|||
| #include <memory> | |||
|
|
|||
| #ifdef MISSING_JSON_FWD_HPP | |||
There was a problem hiding this comment.
This may not work after ROOT is compiled when someone includes this header. The logic should probably be inverted (fallback to json.hpp should be default if nothing is defined), and MISSING_JSON_FWD_HPP should probably be HAVE_JSON_FWD_HPP and be generated in a header and used from there rather than added to the command line with -D as is being done now.
| if(EXISTS "${dir}/nlohmann/json_fwd.hpp") | ||
| set(found_fwd true) | ||
| endif() | ||
| endforeach() |
There was a problem hiding this comment.
Is it really worth it going to such great lengths just to avoid including the full header? I am beginning to think it's probably not.
On some platforms external nlohmann/json.hpp installed without such special include. In this case full version has to be used. Provide special define when compile EVE7 which indicates if json_fwd.hpp can be used. In the macros such define is not exported and therefore full version of nlohmann/json.hpp will be used.
|
Starting build on |
|
I provide special define which enables usage of There was a reason why such "minified" version of json parser is used, |
|
Ok, this new way should work. However, it will only actually use the forward declaration header when building ROOT and when linking against ROOT using the CMake target. If one uses |
On some platforms external nlohmann/json.hpp installed without
such special include. In this case full version has to be used.
Provide special define when compile EVE7 which indicates if
json_fwd.hpp can be used. In the macros such define is not
exported and therefore full version of nlohmann/json.hpp
will be used.
Better solution then #8343