From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/1] package/multipath-tools: new package
Date: Sat, 29 Aug 2020 22:48:26 +0200 [thread overview]
Message-ID: <20200829224826.05bdb28b@windsurf.home> (raw)
In-Reply-To: <20200829082632.27414-1-egorenar-dev@posteo.net>
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@posteo.net> wrote:
> Signed-off-by: Alexander Egorenkov <egorenar-dev@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
next prev parent reply other threads:[~2020-08-29 20:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-29 8:26 [Buildroot] [PATCH 1/1] package/multipath-tools: new package Alexander Egorenkov
2020-08-29 20:48 ` Thomas Petazzoni [this message]
2020-08-31 18:45 ` Alexander Egorenkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200829224826.05bdb28b@windsurf.home \
--to=thomas.petazzoni@bootlin.com \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.