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

CMake alias to nlohmann::json #1291

Closed
parnmatt opened this issue Oct 11, 2018 · 3 comments
Closed

CMake alias to nlohmann::json #1291

parnmatt opened this issue Oct 11, 2018 · 3 comments
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@parnmatt
Copy link

Currently you have the internal name nlohmann_json, and a library alias to nlohmann_json::nlohmann_json, and an export namespace of nlohmann_json:: as per regular convention

Thus when including this project, you can always refer to nlohmann_json::nlohmann_json.

Keep the PROJECT_NAME to nlohmann_json; but have your target name as json with a library alias prefix and export namespace as nlohmann::.

Thus nlohmann::json can be used; which is not only shorter to type, but also in line with the C++ namespacing.

In theory you can make a library alias and separate export group to the current form as well as the suggest form, to not break compatibility. Potentially being deprecated, and optionally removed in a much future version.

@nlohmann
Copy link
Owner

@parnmatt I am not a CMake expert, so I am calling out to @chuckatkins about this idea.

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Oct 12, 2018
@chuckatkins
Copy link
Contributor

TLDR: the current behavior should remain default but making it configurable would be a useful addition.

Long answer:
CMake namespacing is really intended to express multiple components of a single library and framework, which is pretty typical of C++ namespacing as well. Boost, for instance, creates the imported targets for boost::date_time, boost::filesystem, etc. that correspond respectively to libboost_datetime.so, libboost_filesystem.so, etc. So really the CMake namespacing model, and for that matter the typical C++ namespacing model (although no hard rules you can make it however you want), is framework_name::component_name. For a small library with a single component, these tend to be the same thing. So with that in mind, nlohmann doesn't really make sense as a framework / library name with a json component library.

That being said, the target name of nlohmann_json is already used as a variable instead of hard coded so it would be trivial to make both the target and target namespace user-editable cache variables. The intrastructure already references them this way via ${PROJECT_NAME}::${NLOHMANN_JSON_TARGET_NAME} so making them advanced cache variables and using ${NLOHMANN_JSON_TARET_NAMESPACE}::${NLOHMANN_JSON_TARGET_NAME} would allow you define the namespacing and target naming however you wanted. Making the namespacing configurable like that is atypical in cmake-world but not unheard of (i.e. not a typical usage but not an anti-pattern) and makes sense for this use case given that the library is commonly embedded as a private dependency that you want to try to make fit with the rest of your project infrastructure. We actually do this very thing same thing for some internal utility libraries used by CMake and VTK.

@stale
Copy link

stale bot commented Nov 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Nov 11, 2018
@stale stale bot closed this as completed Nov 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: please discuss please discuss the issue or vote for your favorite option state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

3 participants