Skip to content

add include in cmake lists#284

Open
Lixue9jiu wants to merge 2 commits into
lsalzman:masterfrom
Lixue9jiu:master
Open

add include in cmake lists#284
Lixue9jiu wants to merge 2 commits into
lsalzman:masterfrom
Lixue9jiu:master

Conversation

@Lixue9jiu

@Lixue9jiu Lixue9jiu commented Jul 5, 2026

Copy link
Copy Markdown

This allows the end user to do

add_subdirectory(enet)

target_link_libraries(Project
...
        enet
)

and get the include directories set up by CMake, without defining the include directory manually.
It provides better integration with the CMake build system.

@Lixue9jiu
Lixue9jiu marked this pull request as ready for review July 5, 2026 03:28
Comment thread CMakeLists.txt
Comment on lines +105 to +108
target_include_directories(enet
PUBLIC
${PROJECT_SOURCE_DIR}/include
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, this is a welcome modernization! One suggestion: wrap the path in a BUILD_INTERFACE generator expression. As written it causes no error today because the install(TARGETS enet ...) rule has no EXPORT, but as soon as an export set is added, CMake will fail with "INTERFACE_INCLUDE_DIRECTORIES property contains a path prefixed in the source directory". Adding the generator expressions now costs nothing and keeps that door open (they are supported at the declared CMake 2.8.12 minimum).

Suggested change
target_include_directories(enet
PUBLIC
${PROJECT_SOURCE_DIR}/include
)
target_include_directories(enet PUBLIC
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
$<INSTALL_INTERFACE:include>
)

This also matches the indentation style used elsewhere in the file, such as the install(TARGETS enet ...) block.

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