[Buildroot] [PATCH 1/1] package/multipath-tools: new package
Thomas Petazzoni
thomas.petazzoni at bootlin.com
Sat Aug 29 20:48:26 UTC 2020
Hello Alexander,
Again, thanks for your contribution! See below for a number of comments.
On Sat, 29 Aug 2020 10:26:32 +0200
Alexander Egorenkov <egorenar-dev at posteo.net> wrote:
> Signed-off-by: Alexander Egorenkov <egorenar-dev at posteo.net>
> ---
> package/Config.in | 1 +
> .../0001-Cross-compilation-fixes.patch | 45 +++++++++++++++++
> .../0002-Fix-parallel-make.patch | 49 +++++++++++++++++++
> .../multipath-tools/0003-systemd-fix.patch | 28 +++++++++++
> .../0004-udev-rules-path.patch | 13 +++++
> ...005-libdmmp_private.h-add-TRUE-FALSE.patch | 17 +++++++
> package/multipath-tools/Config.in | 12 +++++
> package/multipath-tools/S60multipathd | 39 +++++++++++++++
> package/multipath-tools/multipath-tools.hash | 3 ++
> package/multipath-tools/multipath-tools.mk | 34 +++++++++++++
> 10 files changed, 241 insertions(+)
As usual, we need an entry in the DEVELOPERS file.
> create mode 100644 package/multipath-tools/0001-Cross-compilation-fixes.patch
> create mode 100644 package/multipath-tools/0002-Fix-parallel-make.patch
> create mode 100644 package/multipath-tools/0003-systemd-fix.patch
> create mode 100644 package/multipath-tools/0004-udev-rules-path.patch
> create mode 100644 package/multipath-tools/0005-libdmmp_private.h-add-TRUE-FALSE.patch
All those patches should have a description, Signed-off-by line, and be
generated with git format-patch.
> diff --git a/package/multipath-tools/0001-Cross-compilation-fixes.patch b/package/multipath-tools/0001-Cross-compilation-fixes.patch
> new file mode 100644
> index 0000000000..8914b3bae7
> --- /dev/null
> +++ b/package/multipath-tools/0001-Cross-compilation-fixes.patch
> @@ -0,0 +1,45 @@
> +Index: multipath-tools-0.8.4/libmultipath/Makefile
> +===================================================================
> +--- multipath-tools-0.8.4.orig/libmultipath/Makefile
> ++++ multipath-tools-0.8.4/libmultipath/Makefile
> +@@ -20,21 +20,10 @@ ifdef SYSTEMD
> + endif
> + endif
> +
> +-ifneq ($(call check_func,dm_task_no_flush,/usr/include/libdevmapper.h),0)
> +- CFLAGS += -DLIBDM_API_FLUSH -D_GNU_SOURCE
> +-endif
> +-
> +-ifneq ($(call check_func,dm_task_set_cookie,/usr/include/libdevmapper.h),0)
> +- CFLAGS += -DLIBDM_API_COOKIE
> +-endif
> +-
> +-ifneq ($(call check_func,udev_monitor_set_receive_buffer_size,/usr/include/libudev.h),0)
> +- CFLAGS += -DLIBUDEV_API_RECVBUF
> +-endif
> +-
> +-ifneq ($(call check_func,dm_task_deferred_remove,/usr/include/libdevmapper.h),0)
> +- CFLAGS += -DLIBDM_API_DEFERRED
> +-endif
> ++CFLAGS += -DLIBDM_API_FLUSH -D_GNU_SOURCE
> ++CFLAGS += -DLIBDM_API_COOKIE
> ++CFLAGS += -DLIBUDEV_API_RECVBUF
> ++CFLAGS += -DLIBDM_API_DEFERRED
Hum, this is not the best solution. Can we find something that works
also for cross-compilation ?
> diff --git a/package/multipath-tools/0002-Fix-parallel-make.patch b/package/multipath-tools/0002-Fix-parallel-make.patch
> new file mode 100644
> index 0000000000..8d493659bd
> --- /dev/null
> +++ b/package/multipath-tools/0002-Fix-parallel-make.patch
This one can be submitted upstream. Did you do it already?
> diff --git a/package/multipath-tools/0003-systemd-fix.patch b/package/multipath-tools/0003-systemd-fix.patch
> new file mode 100644
> index 0000000000..15e708b2ca
> --- /dev/null
> +++ b/package/multipath-tools/0003-systemd-fix.patch
> @@ -0,0 +1,28 @@
> +Index: multipath-tools-0.8.4/Makefile.inc
> +===================================================================
> +--- multipath-tools-0.8.4.orig/Makefile.inc
> ++++ multipath-tools-0.8.4/Makefile.inc
> +@@ -36,13 +36,8 @@ ifndef RUN
> + endif
> +
> + ifndef SYSTEMD
> +- ifeq ($(shell pkg-config --modversion libsystemd >/dev/null 2>&1 && echo 1), 1)
> +- SYSTEMD = $(shell pkg-config --modversion libsystemd)
> +- else
> +- ifeq ($(shell systemctl --version >/dev/null 2>&1 && echo 1), 1)
> +- SYSTEMD = $(shell systemctl --version 2> /dev/null | \
> +- sed -n 's/systemd \([0-9]*\).*/\1/p')
> +- endif
> ++ ifeq ($(shell $(PKG_CONFIG) --modversion libsystemd >/dev/null 2>&1 && echo 1), 1)
> ++ SYSTEMD = $(shell $(PKG_CONFIG) --modversion libsystemd)
> + endif
I like that you're using $(PKG_CONFIG), but it's a bit annoying that
you're dropping the systemctl usage because it makes the patch probably
unsuitable for upstream. Should there be a way to avoid the systemctl
usage if we're cross-compiling ?
> diff --git a/package/multipath-tools/0004-udev-rules-path.patch b/package/multipath-tools/0004-udev-rules-path.patch
> new file mode 100644
> index 0000000000..a53be4004c
> --- /dev/null
> +++ b/package/multipath-tools/0004-udev-rules-path.patch
> @@ -0,0 +1,13 @@
> +Index: multipath-tools-0.8.4/Makefile.inc
> +===================================================================
> +--- multipath-tools-0.8.4.orig/Makefile.inc
> ++++ multipath-tools-0.8.4/Makefile.inc
> +@@ -49,7 +49,7 @@ prefix =
> + exec_prefix = $(prefix)
> + usr_prefix = $(prefix)
> + bindir = $(exec_prefix)/sbin
> +-libudevdir = $(prefix)/$(SYSTEMDPATH)/udev
> ++libudevdir = $(prefix)/lib/udev
Not sure why this change is needed. Perhaps to support non-systemd
situations ?
> diff --git a/package/multipath-tools/0005-libdmmp_private.h-add-TRUE-FALSE.patch b/package/multipath-tools/0005-libdmmp_private.h-add-TRUE-FALSE.patch
> new file mode 100644
> index 0000000000..4d6f71cb86
> --- /dev/null
> +++ b/package/multipath-tools/0005-libdmmp_private.h-add-TRUE-FALSE.patch
> @@ -0,0 +1,17 @@
> +--- a/libdmmp/libdmmp_private.h 2020-06-29 08:29:04.308560526 +0200
> ++++ b/libdmmp/libdmmp_private.h 2020-06-29 08:29:12.196605583 +0200
> +@@ -34,6 +34,14 @@
> +
> + #include "libdmmp/libdmmp.h"
> +
> ++#ifndef FALSE
> ++#define FALSE (0)
> ++#endif
> ++
> ++#ifndef TRUE
> ++#define TRUE (!FALSE)
> ++#endif
> ++
> + #ifdef __cplusplus
> + extern "C" {
> + #endif
This should be submitted upstream too.
> diff --git a/package/multipath-tools/Config.in b/package/multipath-tools/Config.in
> new file mode 100644
> index 0000000000..54f670089c
> --- /dev/null
> +++ b/package/multipath-tools/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_MULTIPATH_TOOLS
> + bool "multipath-tools"
> + depends on BR2_PACKAGE_LIBURCU_ARCH_SUPPORTS
> + depends on BR2_PACKAGE_HAS_UDEV
> + select BR2_PACKAGE_LIBURCU
So you need to replicate:
depends on BR2_TOOLCHAIN_HAS_THREADS
> + select BR2_PACKAGE_JSON_C
So you need to replicate:
depends on BR2_TOOLCHAIN_HAS_SYNC_4
> + select BR2_PACKAGE_LIBAIO
Is libaio directly used by multipath-tools, or just as a dependency of
lvm2 ?
> + select BR2_PACKAGE_READLINE
> + select BR2_PACKAGE_LVM2
You also need to replicate:
depends on BR2_TOOLCHAIN_HAS_THREADS
depends on BR2_USE_MMU # needs fork()
depends on !BR2_STATIC_LIBS # It fails to build statically
> + select BR2_PACKAGE_LVM2_STANDARD_INSTALL
And:
depends on !BR2_TOOLCHAIN_USES_MUSL
> + help
> + Makes a small dumpfile of kdump.
This description is wrong.
The upstream URL of the project at the end of the Config.in help text
is missing.
Hint: run "make check-package".
You're also missing a Config.in comment about the dependencies.
> diff --git a/package/multipath-tools/S60multipathd b/package/multipath-tools/S60multipathd
> new file mode 100644
> index 0000000000..1c687901c7
> --- /dev/null
> +++ b/package/multipath-tools/S60multipathd
Could you take package/busybox/S01syslogd as a template for the init
script ?
> diff --git a/package/multipath-tools/multipath-tools.hash b/package/multipath-tools/multipath-tools.hash
> new file mode 100644
> index 0000000000..7eaf246094
> --- /dev/null
> +++ b/package/multipath-tools/multipath-tools.hash
> @@ -0,0 +1,3 @@
> +# Locally computed:
> +sha256 ccd73bf67621161d9e42d1a770c3a7efff6e252433e8b8ed5f64a88cb5e7151d multipath-tools-0.8.4.tar.gz
> +sha256 b7993225104d90ddd8024fd838faf300bea5e83d91203eab98e29512acebd69c COPYING
> diff --git a/package/multipath-tools/multipath-tools.mk b/package/multipath-tools/multipath-tools.mk
> new file mode 100644
> index 0000000000..2572172895
> --- /dev/null
> +++ b/package/multipath-tools/multipath-tools.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# multipath-tools
> +#
> +################################################################################
> +
> +MULTIPATH_TOOLS_VERSION = 0.8.4
> +MULTIPATH_TOOLS_SITE = $(call github,openSUSE,multipath-tools,$(MULTIPATH_TOOLS_VERSION))
> +MULTIPATH_TOOLS_LICENSE = LGPL-2.0
> +MULTIPATH_TOOLS_LICENSE_FILES = COPYING
> +MULTIPATH_TOOLS_DEPENDENCIES = lvm2 json-c readline udev
In the Config.in file, you are selecting liburcu, libaio, but you don't
have them as build dependencies here. Could you check that ?
Also, since you're using pkg-config, you probably want host-pkgconf in
the dependencies.
> +MULTIPATH_TOOLS_MAKE_FLAGS =
An empty variable is quite useless.
> +
> +ifeq ($(BR2_PACKAGE_SYSTEMD),y)
> +MULTIPATH_TOOLS_DEPENDENCIES += systemd
> +endif
> +
> +define MULTIPATH_TOOLS_BUILD_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) CC="$(TARGET_CC)" PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
> + OPTFLAGS="$(TARGET_CFLAGS)"
Could you try to use $(TARGET_CONFIGURE_OPTS) to pass CC, PKG_CONFIG,
etc. ?
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the buildroot
mailing list