Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relative includes of json_fwd.hpp in detail/meta.hpp. [Develop branch] #928

Closed
przemkovv opened this issue Jan 21, 2018 · 5 comments · Fixed by #944
Closed

Relative includes of json_fwd.hpp in detail/meta.hpp. [Develop branch] #928

przemkovv opened this issue Jan 21, 2018 · 5 comments · Fixed by #944
Assignees
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@przemkovv
Copy link

przemkovv commented Jan 21, 2018

Bug Report

  • What is the issue you have?
    I have nlohmann/json library added using ExternalProject_Add in CMakeLists.txt.
    In my project, the <INSTALL_DIR>/include/ folder is added as the system include directories.
    During compilation, I've an error of missing json_fwd.hpp included from detail/meta.cpp.
    The file json_fwd.hpp is "missing" because it is located in include/nlohmann and only include/ is added to 'global' include path.

I'm using JSON_MultipleHeaders=ON flag in CMake.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?
    File CMakeLists.txt
cmake_minimum_required(VERSION 3.10)
project(jj)

include(ExternalProject)
ExternalProject_Add(json
    GIT_REPOSITORY https://github.com/nlohmann/json.git
    GIT_TAG develop
    INSTALL_DIR ${PROJECT_BINARY_DIR}
    CMAKE_ARGS -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> -DBUILD_TESTING=0 -DJSON_MultipleHeaders=ON
    UPDATE_COMMAND ""
    )

include_directories(SYSTEM ${PROJECT_BINARY_DIR}/include)
add_executable(jj src/test.cpp)
add_dependencies(jj json)

File src/test.cpp:

#include <nlohmann/json.hpp>

int main() {
  return 0;
}
  • What is the expected behavior?
    json_fwd.hpp should be visible for files in detail/ of the json library.

  • And what is the actual behavior instead?
    The relative path in meta.cpp to the json_fwd.hpp doesn't allow to compile.
    During compilation, I've got the following error:

In file included from /home/przemkovv/projects/sandbox/json_bug/src/test.cpp:2:
In file included from /home/przemkovv/projects/sandbox/json_bug/build/include/nlohmann/json.hpp:46:
/home/przemkovv/projects/sandbox/json_bug/build/include/nlohmann/detail/meta.hpp:9:10: fatal error: 'json_fwd.hpp' file not found
#include "json_fwd.hpp"
         ^~~~~~~~~~~~~~
  • Which compiler and operating system are you using? Is it a supported compiler?
    Arch Linux, Clang 7.0.0

  • Did you use a released version of the library or the version from the develop branch?
    develop

  • If you experience a compilation error: can you compile and run the unit tests?
    Cannot compile.

@nlohmann
Copy link
Owner

nlohmann commented Jan 21, 2018

Related: #923, #925

@theodelrieu
Copy link
Contributor

I believe we should rename the develop folder to have the same name as the installation target: nlohmann.

Then, stop using relative includes in the sources (they were only useful with the C++ amalgamate tool).

We have to keep including json.hpp (no prefix) in the tests for now, to switch easily between amalgamated/non-amalgamated versions.

I don't know how the python script decides which headers to include, but if it has the same logic about ""-quoted headers, we have to modify it to only include <nlohmann/*> headers.

@nlohmann
Copy link
Owner

The issue could be solved by changing the include line from

include_directories(SYSTEM ${PROJECT_BINARY_DIR}/include)

to

include_directories(SYSTEM ${PROJECT_BINARY_DIR}/include ${PROJECT_BINARY_DIR}/include/nlohmann)

@nlohmann
Copy link
Owner

Forget my proposal. This will be fixed properly with #944.

@nlohmann nlohmann added this to the Release 3.1.0 milestone Jan 31, 2018
@przemkovv
Copy link
Author

Thanks! Your library is always awesome. )

@nlohmann nlohmann self-assigned this Feb 1, 2018
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants