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.

Sign up for GitHub

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: modify build system for OpenWrt compatibility #172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

cmake: modify build system for OpenWrt compatibility #172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
809e803
cmake: modify build system for OpenWrt compatibility
liudf0716 Nov 10, 2024
f7c62f2
helper: replace strerrordesc_np with strerror for error handling
liudf0716 Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
cmake: modify build system for OpenWrt compatibility 
- Disable testing and documentation builds by default
- Add conditional inclusion of tests and documentation
- Handle argp dependency with find_library
- Make development tools (clang-tidy, clang-format) optional
- Add flexible bpftool handling with fallback paths
- Improve build system modularity and configurability

This modification adjusts the build system to ensure compatibility
with the OpenWrt environment, making necessary changes for a successful build.

Signed-off-by: Dengfeng Liu <liudf0716@gmail.com>
  • Loading branch information
@liudf0716
liudf0716 committed Nov 14, 2024
commit 809e8033a570bc42a3abe60cff0304774698d91f
60 changes: 50 additions & 10 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,31 @@ project(bpfilter

include(GNUInstallDirs)

find_package(Doxygen REQUIRED)
option(BUILD_TESTING "Enable testing" OFF)
option(BUILD_DOCS "Enable building documentation" OFF)
Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

Those should be enabled by default to keep the current behaviour. Use WITH_XXX instead of BUILD_XXX similarly to #174.


if(BUILD_DOCS)
find_package(Doxygen REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this in docs/CMakeLists.txt, and modify add_subdirectory(docs) below to:

if (WITH_DOCS)
    add_subdirectory(docs)
endif ()

endif()
find_package(PkgConfig REQUIRED)
find_package(FLEX REQUIRED)
find_package(BISON REQUIRED)

pkg_check_modules(bpf REQUIRED IMPORTED_TARGET libbpf)
pkg_check_modules(elf REQUIRED IMPORTED_TARGET libelf)
pkg_check_modules(cmocka REQUIRED IMPORTED_TARGET cmocka)
if(BUILD_TESTING)
pkg_check_modules(cmocka REQUIRED IMPORTED_TARGET cmocka)
endif()
Comment on lines +26 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

pkg_check_modules(nl REQUIRED IMPORTED_TARGET libnl-3.0)

find_library(ARGP_LIBRARY
NAMES argp
PATHS ${CMAKE_SYSROOT}/usr/lib
Copy link
Contributor

Choose a reason for hiding this comment

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

According to CMake's doc, search path are automatically prefixed with CMAKE_SYSROOT, so there is no need to add it here.

REQUIRED)
if(NOT ARGP_LIBRARY)
message(FATAL_ERROR "libargp not found")
endif()
Comment on lines +35 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

ARGP_LIBRARY is REQUIRED above, if not found, CMake will fail the generation and print an error message, no need for this condition.


# Required to get CMake to pass PIE flags to the compiler/linker
include(CheckPIESupported)
check_pie_supported()
Expand All @@ -30,13 +45,34 @@ if(NOT CMAKE_C_LINK_PIE_SUPPORTED)
"If PIE is enabled by default, test auto discovery might fail.")
endif()

find_program(SPHINX_BIN sphinx-build REQUIRED)
find_program(LCOV_BIN lcov REQUIRED)
find_program(GENHTML_BIN genhtml REQUIRED)
find_program(CLANG_TIDY_BIN NAMES clang-tidy-18 clang-tidy REQUIRED)
find_program(CLANG_FORMAT_BIN NAMES clang-format-18 clang-format REQUIRED)
if(BUILD_DOCS)
find_program(SPHINX_BIN sphinx-build REQUIRED)
find_program(LCOV_BIN lcov REQUIRED)
find_program(GENHTML_BIN genhtml REQUIRED)
find_program(CLANG_TIDY_BIN NAMES clang-tidy-18 clang-tidy REQUIRED)
find_program(CLANG_FORMAT_BIN NAMES clang-format-18 clang-format REQUIRED)
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

Those two binaries are not part of the documentation generation, but the testing logic.

endif()
Comment on lines +48 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

All those find_program should be located in docs/CMakeLists.txt.


# Replace mandatory REQUIRED flag
find_program(BPFTOOL_BIN bpftool)

# Add fallback paths
if(NOT BPFTOOL_BIN)
find_program(BPFTOOL_BIN bpftool
PATHS
${CMAKE_SYSROOT}/usr/sbin
${CMAKE_SYSROOT}/sbin
/usr/sbin
/sbin
)
endif()

# Make it optional with warning
if(NOT BPFTOOL_BIN)
message(WARNING "bpftool not found - some features may be disabled")
set(BPFTOOL_BIN "bpftool") # Set default value
endif()
Comment on lines +56 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this to tests/bpf/CMakeLists.txt, where bpftool is actually used. It's not ideal, but better than the current situation and probably sufficient for your use case.

find_program(CLANG_BIN clang REQUIRED)
find_program(BPFTOOL_BIN bpftool REQUIRED)
find_program(JQ_BIN jq REQUIRED)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
Expand Down Expand Up @@ -84,8 +120,12 @@ target_link_options(bf_global_flags
)

add_subdirectory(src)
add_subdirectory(tests)
add_subdirectory(doc)
if(BUILD_DOCS)
add_subdirectory(doc)
endif()
if(BUILD_TESTING)
add_subdirectory(tests)
endif()

install(TARGETS bpfilter libbpfilter_a libbpfilter_so bfcli)

Expand Down
1 change: 1 addition & 0 deletions src/bfcli/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ target_link_libraries(bfcli
bf_global_flags
core
libbpfilter_a
${ARGP_LIBRARY}
)
1 change: 1 addition & 0 deletions src/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ target_link_libraries(core
PUBLIC
bf_global_flags
PkgConfig::bpf
${ARGP_LIBRARY}
)

add_library(core.pic
Expand Down
1 change: 1 addition & 0 deletions tests/benchmark/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ target_link_libraries(benchmark_bin
PRIVATE
PkgConfig::bpf
PkgConfig::git2
${ARGP_LIBRARY}
benchmark::benchmark
)

Expand Down
1 change: 1 addition & 0 deletions tests/e2e/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ target_link_libraries(e2e_bin
PRIVATE
harness
libbpfilter_a
${ARGP_LIBRARY}
)

add_custom_target(e2e
Expand Down
1 change: 1 addition & 0 deletions tests/unit/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ target_link_libraries(unit_tests
bf_global_flags
harness
gcov
${ARGP_LIBRARY}
)

add_custom_target(test
Expand Down