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 all commits
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
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
4 changes: 1 addition & 3 deletions src/core/helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
#include <string.h>
#include <unistd.h>

extern const char *strerrordesc_np(int errnum);

#define bf_packed __attribute__((packed))
#define bf_aligned(x) __attribute__((aligned(x)))
#define bf_unused __attribute__((unused))
Expand Down Expand Up @@ -80,7 +78,7 @@ extern const char *strerrordesc_np(int errnum);
*
* @param v Error code, can be positive or negative.
*/
#define bf_strerror(v) strerrordesc_np(abs(v))
#define bf_strerror(v) strerror(abs(v))

/**
* Swap two values.
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
Loading