yhirose / cpp-peglib

A single file C++ header-only PEG (Parsing Expression Grammars) library
MIT License
880 stars 112 forks source link

Reorganize as modern CMake with an interface target #247

Closed kfsone closed 1 year ago

kfsone commented 1 year ago

Cleanup/reorganize the cmake lists files to present peglib as an interface target and default-disable building of linter, examples and test when peglib is not the primary build tree.

The aim here was to make it easier to integrate peglib into other code bases as a header-only library, without it potentially contaminating the compiler flags / linker settings / include paths of other targets/projects.

To use with this new construct, one needs to:

add_subdirectory(cpp-peglib)

add_executable(my_executable main.cpp)  # or add_library

target_link_libraries(my_executable CppPegLib)

Doing so will inherit the include path, necessary compiler flags, and any linker flags to your target.

If you are building a library that pulls in CppPegLib, you may want to make the inclusion PRIVATE so that the compiler/link flags, include paths, etc do not leak upwards for you.

add_library(my-middleware library.cpp grammar.cpp ...)
target_link_libraries(my-middleware PRIVATE CppPegLib)
kfsone commented 1 year ago

The appveyor may fail, I'm unfamiliar with appveyor and it's possible that the config needs tweaking to support the changes here.

kfsone commented 1 year ago

Ah, you have .cmake files in the .gitignore, fixed.

Do you usually build in-directory rather than into a bin directory?

When I'm command-line hacking, my cmake workflow is usually something like:

cd cpp-peglib
cmake -G Ninja -B build .  # write cmake files to ./build/
cmake --build build  # have cmake launch whatever it configured as the compiler;
# add --target <target name> or --config <Release|Debug> to suit.
yhirose commented 1 year ago

Thanks for the pull request, but I would like to keep the current simple CMakeList.txt as it is, since I am not a CMake expert and I don't have any need to enhance it for now. Thanks for your understanding!

kfsone commented 1 year ago

Totally understand, the motivation was that the current implementation modifies CMake globals which can then incorrectly propagate to other targets.

I'll take a shot at a much simpler change separately.