[Buildroot] [PATCH] package/cryptopp: add a target build configuration
Thomas Petazzoni
thomas.petazzoni at bootlin.com
Mon Jul 6 16:24:58 UTC 2020
Hello,
Thanks for the patch! See below for some comments.
On Mon, 6 Jul 2020 17:31:28 +0200
Kamel Bouhara <kamel.bouhara at bootlin.com> wrote:
> package/Config.in | 1 +
> ...ied-SONAME-to-shared-object-for-Linu.patch | 27 ++++++++++++++
> package/cryptopp/Config.in | 4 +++
> package/cryptopp/Config.in.host | 4 +++
This file is added, but not used anywhere. In fact, we don't need it at
all.
> diff --git a/package/cryptopp/0001-Add-fully-qualified-SONAME-to-shared-object-for-Linu.patch b/package/cryptopp/0001-Add-fully-qualified-SONAME-to-shared-object-for-Linu.patch
> new file mode 100644
> index 0000000000..e7edc76313
> --- /dev/null
> +++ b/package/cryptopp/0001-Add-fully-qualified-SONAME-to-shared-object-for-Linu.patch
> @@ -0,0 +1,27 @@
> +From 78eb43f50978ffd780cf31b1cea6736dadc6b155 Mon Sep 17 00:00:00 2001
> +From: Kamel Bouhara <kamel.bouhara at bootlin.com>
> +Date: Mon, 6 Jul 2020 17:10:55 +0200
> +Subject: [PATCH] Add fully-qualified SONAME to shared object for Linux
> +
> +From: http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html
> +
> +Signed-off-by: Kamel Bouhara <kamel.bouhara at bootlin.com>
> +---
> + GNUmakefile | 1 +
> + 1 file changed, 1 insertion(+)
> +
> +diff --git a/GNUmakefile b/GNUmakefile
> +index e7b7b3a6..730e2a6f 100755
> +--- a/GNUmakefile
> ++++ b/GNUmakefile
> +@@ -1256,6 +1256,7 @@ ifneq ($(wildcard libcryptopp.so$(SOLIB_VERSION_SUFFIX)),)
> + $(CHMOD) 0755 $(DESTDIR)$(LIBDIR)/libcryptopp.so$(SOLIB_VERSION_SUFFIX)
> + ifeq ($(HAS_SOLIB_VERSION),1)
> + -$(LN) libcryptopp.so$(SOLIB_VERSION_SUFFIX) $(DESTDIR)$(LIBDIR)/libcryptopp.so
> ++ -$(LN) libcryptopp.so$(SOLIB_VERSION_SUFFIX) $(DESTDIR)$(LIBDIR)/libcryptopp.so$(SOLIB_COMPAT_SUFFIX)
Is there something creating the .so symlink then ?
> + A free C++ class library of cryptographic schemes
> diff --git a/package/cryptopp/cryptopp.mk b/package/cryptopp/cryptopp.mk
> index f1d19386ab..f62713a2cd 100644
> --- a/package/cryptopp/cryptopp.mk
> +++ b/package/cryptopp/cryptopp.mk
> @@ -12,26 +12,46 @@ CRYPTOPP_LICENSE_FILES = License.txt
> CRYPTOPP_INSTALL_STAGING = YES
>
> define HOST_CRYPTOPP_EXTRACT_CMDS
> - $(UNZIP) $(HOST_CRYPTOPP_DL_DIR)/$(CRYPTOPP_SOURCE) -d $(@D)
> + $(UNZIP) $(HOST_CRYPTOPP_DL_DIR)/$(CRYPTOPP_SOURCE) -d $(@D)
You've broken the indentation here: a TAB is correct, your change to
spaces is not. This is an issue globally.
> endef
>
> -HOST_CRYPTOPP_CXXFLAGS = $(HOST_CFLAGS) -fPIC
> -
> # _mm256_broadcastsi128_si256 has been added only in gcc 4.9
> ifneq ($(BR2_HOST_GCC_AT_LEAST_4_9),y)
So you're testing the host compiler capabilities...
> -HOST_CRYPTOPP_CXXFLAGS += -DCRYPTOPP_DISABLE_AVX2
> + CRYPTOPP_CXXFLAGS = -DCRYPTOPP_DISABLE_AVX2
... to then decide something compiled for the target ? Not good.
> endif
You need to keep separate HOST_CRYPTOPP_CXXFLAGS and CRYPTOPP_CXXFLAGS,
and test on the compiler version separately, as the host compiler and
target compiler can be different.
Note that on the target variant, you need to test a configuration with
static libraries only, because you're passing unconditionally -fPIC and
the Makefile seems to be building only shared libraries, so that will
likely fail on static only configuration. Perhaps a "depends on
!BR2_STATIC_LIBS" is needed in the Config.in.
> HOST_CRYPTOPP_MAKE_OPTS = \
> - $(HOST_CONFIGURE_OPTS) \
> - CXXFLAGS="$(HOST_CRYPTOPP_CXXFLAGS)"
> + $(HOST_CONFIGURE_OPTS) \
> + CXXFLAGS="$(HOST_CFLAGS) -fPIC $(CRYPTOPP_CXXFLAGS)"
>
> define HOST_CRYPTOPP_BUILD_CMDS
> - $(HOST_MAKE_ENV) $(MAKE) -C $(@D) $(HOST_CRYPTOPP_MAKE_OPTS) shared
> + $(HOST_MAKE_ENV) $(MAKE) -C $(@D) $(HOST_CRYPTOPP_MAKE_OPTS) shared
> endef
>
> define HOST_CRYPTOPP_INSTALL_CMDS
> - $(HOST_MAKE_ENV) $(MAKE) -C $(@D) PREFIX=$(HOST_DIR) install-lib
> + $(HOST_MAKE_ENV) $(MAKE) -C $(@D) PREFIX=$(HOST_DIR) install-lib
> +endef
Please fix the indentation. It seems like you didn't review the diff
you're sending.
> +CRYPTOPP_MAKE_OPTS = \
> + $(TARGET_CONFIGURE_OPTS) \
> + CXXFLAGS="$(TARGET_CFLAGS) -fPIC $(CRYPTOPP_CXXFLAGS)"
> +
> +define CRYPTOPP_EXTRACT_CMDS
> + $(UNZIP) $(CRYPTOPP_DL_DIR)/$(CRYPTOPP_SOURCE) -d $(@D)
> +endef
> +
> +define CRYPTOPP_BUILD_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) $(CRYPTOPP_MAKE_OPTS) shared
Ah, there's a "shared" target, so in fact perhaps it can also build a
static library ?
> +define CRYPTOPP_INSTALL_TARGET_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) PREFIX=$(TARGET_DIR) install-lib
> +endef
> +
> +define CRYPTOPP_INSTALL_STAGING_CMDS
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) PREFIX=$(STAGING_DIR)/usr libcryptopp.pc
> + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) PREFIX=$(STAGING_DIR)/usr install-lib
It is strange that the PREFIX is just $(TARGET_DIR) in one case, and
$(STAGING_DIR)/usr in the other. Could you explain ?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
More information about the buildroot
mailing list