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

Separate third party libraries; resolves #79 #119

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

liuzicheng1987
Copy link
Contributor

No description provided.

@liuzicheng1987 liuzicheng1987 merged commit 0342e5c into main Jun 12, 2024
12 checks passed
@liuzicheng1987 liuzicheng1987 deleted the f/separate_thirdparty branch June 12, 2024 22:37
add_library(reflectcpp STATIC src/yyjson.c)
endif()
else()
if (REFLECTCPP_BUILD_SHARED)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably instead of SHARED or STATIC, INTERFACE would be the choice for Header-Only-Lib
https://cmake.org/cmake/help/latest/command/add_library.html#interface-libraries


set_target_properties(reflectcpp PROPERTIES LINKER_LANGUAGE CXX)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, probably could use the LANGUAGES section of project
https://cmake.org/cmake/help/latest/command/project.html

But setting it just to CXX, as this is a C++ project, gives trouble with the yyjson.c .
I guess it could be a C++ wrapper around it.
Maybe like

// yyjson.cpp
extern C {
  #include "yyjson.c"
}

Quick test shows, that at least clangd is happy with it, and uses the right compiler and flags.

target_compile_features(reflectcpp PUBLIC cxx_std_20)

target_include_directories(reflectcpp PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include> )
if(REFLECTCPP_USE_BUNDLED_DEPENDENCIES)
target_include_directories(reflectcpp PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include> $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/rfl/thirdparty>)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the thirdparty folder as set as extra include here, there should be no need to have the checks in the header files. I guess. Compiler should use the first header it finds, so if thirdparty is first, it should be okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants