| Message ID | 20230428152211.20394-2-flaniel@linux.microsoft.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Bump sysdig and falco libs | expand |
Hello Francis (and perhaps Angelo who can help?), On Fri, 28 Apr 2023 16:22:10 +0100 Francis Laniel <flaniel@linux.microsoft.com> wrote: > - Remove upstream patch as it is no more needed. Actually I had to remove it from current master, because sysdig was bumped to 0.29.3 already, which includes the patch... and so the patch in Buildroot doesn't apply anymore. I tested your version bump, and it fails to build with: CMake Error at /home/thomas/projets/buildroot/output/build/falcosecurity-libs-e5c53d648f3c4694385bbe488e7d47eaa36c229a/userspace/libscap/CMakeLists.txt:131 (add_subdirectory): The binary directory /home/thomas/projets/buildroot/output/build/sysdig-0.31.4/buildroot-build/driver is already used to build a source directory. It cannot be used to build source directory /home/thomas/projets/buildroot/output/build/falcosecurity-libs-e5c53d648f3c4694385bbe488e7d47eaa36c229a/driver Specify a unique binary directory name. during the configuration step of sysdig. Configuration tested: BR2_arm=y BR2_cortex_a9=y BR2_ARM_ENABLE_VFP=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y BR2_INIT_NONE=y BR2_SYSTEM_BIN_SH_NONE=y BR2_LINUX_KERNEL=y BR2_LINUX_KERNEL_CUSTOM_VERSION=y BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.1" BR2_LINUX_KERNEL_DEFCONFIG="vexpress" # BR2_PACKAGE_BUSYBOX is not set BR2_PACKAGE_SYSDIG=y BR2_PACKAGE_LUA=y BR2_PACKAGE_LUA_5_1=y # BR2_TARGET_ROOTFS_TAR is not set it would be good to have a runtime test for sysdig in support/testing/, as it's not trivial to build, and the autobuilders never caught the patching issue. > SYSDIG_CONF_OPTS += -DFALCOSECURITY_LIBS_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR) \ > + -DDRIVER_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR)/driver \ So apparently something goes wrong with this. Perhaps because falcosecurity-libs needs to be bumped first? > -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson \ One thing that is a bit annoying with the packaging here is the fact that sysdig needs to look into the source directory of falcosecurity-libs and the source tree of valijson. Packages should normally not need to access the source/build tree of other packages. Not a strict requirement for this version bump, but would be good to address on the long run. By the way -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson is truly horrible, because if valijson gets updated to another version... like it has: VALIJSON_VERSION = 0.7 then this doesn't work anymore. It needs to be VALIJSON_SRCDIR. Curious that we can build sysdig today (I verified, it builds) with this mistake. Probably means this option is irrelevant. Could you have a look at all those issues? Thanks! Thomas
On Mon, 31 Jul 2023 22:18:43 +0200 Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > By the way > -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson is truly > horrible, because if valijson gets updated to another version... like > it has: > > VALIJSON_VERSION = 0.7 > > then this doesn't work anymore. > > It needs to be VALIJSON_SRCDIR. > > Curious that we can build sysdig today (I verified, it builds) with > this mistake. Probably means this option is irrelevant. (1) There is no build dependency on valijson, so I don't see how it could be relevant anyway. (2) There is no reference to VALIJSON_INCLUDE anywhere in the code base, as far as I can see This really needs to be cleaned up. Very soon. Thomas
Hi. Le lundi 31 juillet 2023, 22:18:43 CEST Thomas Petazzoni a écrit : > Hello Francis (and perhaps Angelo who can help?), > > On Fri, 28 Apr 2023 16:22:10 +0100 > > Francis Laniel <flaniel@linux.microsoft.com> wrote: > > - Remove upstream patch as it is no more needed. > > Actually I had to remove it from current master, because sysdig was > bumped to 0.29.3 already, which includes the patch... and so the patch > in Buildroot doesn't apply anymore. > > I tested your version bump, and it fails to build with: > > CMake Error at > /home/thomas/projets/buildroot/output/build/falcosecurity-libs-e5c53d648f3c > 4694385bbe488e7d47eaa36c229a/userspace/libscap/CMakeLists.txt:131 > (add_subdirectory): The binary directory > > > /home/thomas/projets/buildroot/output/build/sysdig-0.31.4/buildroot-build/d > river > > is already used to build a source directory. It cannot be used to build > source directory > > > /home/thomas/projets/buildroot/output/build/falcosecurity-libs-e5c53d648f3c > 4694385bbe488e7d47eaa36c229a/driver > > Specify a unique binary directory name. > > during the configuration step of sysdig. > > Configuration tested: > > BR2_arm=y > BR2_cortex_a9=y > BR2_ARM_ENABLE_VFP=y > BR2_TOOLCHAIN_EXTERNAL=y > BR2_TOOLCHAIN_EXTERNAL_BOOTLIN=y > BR2_INIT_NONE=y > BR2_SYSTEM_BIN_SH_NONE=y > BR2_LINUX_KERNEL=y > BR2_LINUX_KERNEL_CUSTOM_VERSION=y > BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.1" > BR2_LINUX_KERNEL_DEFCONFIG="vexpress" > # BR2_PACKAGE_BUSYBOX is not set > BR2_PACKAGE_SYSDIG=y > BR2_PACKAGE_LUA=y > BR2_PACKAGE_LUA_5_1=y > # BR2_TARGET_ROOTFS_TAR is not set > it would be good to have a runtime test for sysdig in support/testing/, > as it's not trivial to build, and the autobuilders never caught the > patching issue. Good idea! I will check how I can do that! > > SYSDIG_CONF_OPTS += > > -DFALCOSECURITY_LIBS_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR) \> > > + -DDRIVER_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR)/driver \ > > So apparently something goes wrong with this. Perhaps because > falcosecurity-libs needs to be bumped first? As you advised, bumping first the libs then the binary removed the above problem, thank you! Regarding this, I am wondering if I should bump both of them in the same commit, as they are tightly coupled. What do you think? Note that, I had to make linux-menuconfig to add CONFIG_IPV6, to avoid some compile errors due to some missing IPv6 related fields while compiling the kernel module which is parts of the libs. > > -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson \ > > One thing that is a bit annoying with the packaging here is the fact > that sysdig needs to look into the source directory of > falcosecurity-libs and the source tree of valijson. Packages should > normally not need to access the source/build tree of other packages. > Not a strict requirement for this version bump, but would be good to > address on the long run. > > By the way > -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson is truly > horrible, because if valijson gets updated to another version... like > it has: > > VALIJSON_VERSION = 0.7 > > then this doesn't work anymore. > > It needs to be VALIJSON_SRCDIR. > > Curious that we can build sysdig today (I verified, it builds) with > this mistake. Probably means this option is irrelevant. > > Could you have a look at all those issues? I removed everything about VALIJSON and it builds fine. Glad making it simpler permits to build it! > Thanks! > > Thomas
diff --git a/package/sysdig/0001-cmake-Check-USE_BUNDLED_DEPS-before-getting-nlohmann.patch b/package/sysdig/0001-cmake-Check-USE_BUNDLED_DEPS-before-getting-nlohmann.patch deleted file mode 100644 index 3521bd3f8d..0000000000 --- a/package/sysdig/0001-cmake-Check-USE_BUNDLED_DEPS-before-getting-nlohmann.patch +++ /dev/null @@ -1,52 +0,0 @@ -From 0dbebd008c04d266dc41c4bec8280a0744fd5130 Mon Sep 17 00:00:00 2001 -From: Francis Laniel <flaniel@linux.microsoft.com> -Date: Wed, 13 Apr 2022 18:01:11 +0100 -Subject: [PATCH] cmake: Check USE_BUNDLED_DEPS before getting - nlohmann-json. - -Upstream: https://github.com/draios/sysdig/pull/1869 -Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com> ---- - cmake/modules/nlohmann-json.cmake | 29 +++++++++++++++++++---------- - 1 file changed, 19 insertions(+), 10 deletions(-) - -diff --git a/cmake/modules/nlohmann-json.cmake b/cmake/modules/nlohmann-json.cmake -index bb1279d7..feb0f071 100644 ---- a/cmake/modules/nlohmann-json.cmake -+++ b/cmake/modules/nlohmann-json.cmake -@@ -16,13 +16,22 @@ - # limitations under the License. - # - --set(NJSON_SRC "${PROJECT_BINARY_DIR}/njson-prefix/src/njson") --message(STATUS "Using bundled nlohmann-json in '${NJSON_SRC}'") --set(NJSON_INCLUDE_DIR "${NJSON_SRC}/single_include") --ExternalProject_Add( -- njson -- URL "https://github.com/nlohmann/json/archive/v3.3.0.tar.gz" -- URL_HASH "SHA256=2fd1d207b4669a7843296c41d3b6ac5b23d00dec48dba507ba051d14564aa801" -- CONFIGURE_COMMAND "" -- BUILD_COMMAND "" -- INSTALL_COMMAND "") -+if(NOT USE_BUNDLED_DEPS) -+ find_path(NJSON_INCLUDE_DIR NAMES nlohmann/json.hpp) -+ if(NJSON_INCLUDE_DIR) -+ message(STATUS "Found njson: include: ${NJSON_INCLUDE_DIR}") -+ else() -+ message(FATAL_ERROR "Couldn't find system njson") -+ endif() -+else() -+ set(NJSON_SRC "${PROJECT_BINARY_DIR}/njson-prefix/src/njson") -+ message(STATUS "Using bundled nlohmann-json in '${NJSON_SRC}'") -+ set(NJSON_INCLUDE_DIR "${NJSON_SRC}/single_include") -+ ExternalProject_Add( -+ njson -+ URL "https://github.com/nlohmann/json/archive/v3.3.0.tar.gz" -+ URL_HASH "SHA256=2fd1d207b4669a7843296c41d3b6ac5b23d00dec48dba507ba051d14564aa801" -+ CONFIGURE_COMMAND "" -+ BUILD_COMMAND "" -+ INSTALL_COMMAND "") -+endif() --- -2.25.1 - diff --git a/package/sysdig/sysdig.hash b/package/sysdig/sysdig.hash index cda3de5e7c..902f6f2b82 100644 --- a/package/sysdig/sysdig.hash +++ b/package/sysdig/sysdig.hash @@ -1,3 +1,3 @@ # sha256 locally computed -sha256 6b96797859002ab69a2bed4fdba1c7fe8064ecf8661621ae7d8fbf8599ffa636 sysdig-0.29.3.tar.gz +sha256 b8f43326506f85e99a3455f51b75ee79bf4db9dc12908ef43af672166274a795 sysdig-0.31.4.tar.gz sha256 a88fbf820b38b1c7fabc6efe291b8259e02ae21326f56fe31c6c9adf374b2702 COPYING diff --git a/package/sysdig/sysdig.mk b/package/sysdig/sysdig.mk index bafe534a16..aabd274557 100644 --- a/package/sysdig/sysdig.mk +++ b/package/sysdig/sysdig.mk @@ -4,7 +4,7 @@ # ################################################################################ -SYSDIG_VERSION = 0.29.3 +SYSDIG_VERSION = 0.31.4 SYSDIG_SITE = $(call github,draios,sysdig,$(SYSDIG_VERSION)) SYSDIG_LICENSE = Apache-2.0 SYSDIG_LICENSE_FILES = COPYING @@ -26,11 +26,17 @@ SYSDIG_DEPENDENCIES = \ # grpc_cpp_plugin is needed to build falcosecurity libs, so we give the host # one there. SYSDIG_CONF_OPTS += -DFALCOSECURITY_LIBS_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR) \ + -DDRIVER_SOURCE_DIR=$(FALCOSECURITY_LIBS_SRCDIR)/driver \ -DBUILD_DRIVER=OFF \ -DGRPC_CPP_PLUGIN=$(HOST_DIR)/bin/grpc_cpp_plugin \ -DDRIVER_NAME=$(FALCOSECURITY_LIBS_DRIVER_NAME) \ -DENABLE_DKMS=OFF \ -DUSE_BUNDLED_DEPS=OFF \ + -DUSE_BUNDLED_TBB=OFF \ + -DUSE_BUNDLED_B64=OFF \ + -DUSE_BUNDLED_JSONCPP=OFF \ + -DUSE_BUNDLED_VALIJSON=OFF \ + -DUSE_BUNDLED_RE2=OFF \ -DWITH_CHISEL=ON \ -DVALIJSON_INCLUDE=$(BUILD_DIR)/valijson-0.6/include/valijson \ -DSYSDIG_VERSION=$(SYSDIG_VERSION)
- Remove upstream patch as it is no more needed. Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com> --- ...BUNDLED_DEPS-before-getting-nlohmann.patch | 52 ------------------- package/sysdig/sysdig.hash | 2 +- package/sysdig/sysdig.mk | 8 ++- 3 files changed, 8 insertions(+), 54 deletions(-) delete mode 100644 package/sysdig/0001-cmake-Check-USE_BUNDLED_DEPS-before-getting-nlohmann.patch