Re: [Buildroot] [PATCH 1/1] package/wolftpm: new package

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Dimitar,

On Wed,  1 Jun 2022 22:47:46 +0300
Dimi Tomov <dimi@tpm.dev> wrote:

> From: Dimitar Tomov <dimi@tpm.dev>
> 
> wolfTPM is an open-source TPM 2.0 stack with backward API compatibility,
> designed for embedded use. It is highly portable, and has native support
> for Linux. wolfTPM has a compact code size with low resource usage.
> 
> Signed-off-by: Dimitar Tomov <dimi@tpm.dev>

I've applied to our next branch, but after doing several additional
fixes. Also, there is something to be fixed upstream, see below.

> diff --git a/DEVELOPERS b/DEVELOPERS
> index 71cc3da6d7..c123d1b915 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -3072,3 +3072,6 @@ F:	package/quazip/
>  F:	package/shapelib/
>  F:	package/simple-mail/
>  F:	package/tinc/
> +
> +N:	Dimi Tomov <dimi@tpm.dev>
> +F:	package/wolftpm/

Entries in this file are alphabetically sorted, so you shouldn't have
added yourself at the end, but at the "right" place.

> diff --git a/package/wolftpm/Config.in b/package/wolftpm/Config.in
> new file mode 100644
> index 0000000000..23932a4170
> --- /dev/null
> +++ b/package/wolftpm/Config.in
> @@ -0,0 +1,15 @@
> +config BR2_PACKAGE_WOLFTPM
> +	bool "wolftpm"
> +	depends on BR2_TOOLCHAIN_HAS_THREADS

You forgot:

	depends on !BR2_STATIC_LIBS

which you need to replicate because you select BR2_PACKAGE_WOLFSSL_ALL.

> +	select on BR2_PACKAGE_WOLFSSL
> +	select on BR2_PACKAGE_WOLFSSL_ALL

I'm wondering if you tested this, because "select on" doesn't exist in
Kconfig. It's either "select" or "depends on", but not a mix of both.

> +	help
> +	  wolfTPM is a portable, open-source TPM 2.0 stack with
> +	  backward API compatibility, designed for embedded use.
> +	  No external dependencies, compact code size with low
> +	  resource usage.
> +
> +	  https://www.wolfssl.com/
> +
> +comment "wolftpm needs a toolchain w/ threads"
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS

The comment had to be adjusted due to the !BR2_STATIC_LIBS dependency.

> diff --git a/package/wolftpm/wolftpm.hash b/package/wolftpm/wolftpm.hash
> new file mode 100644
> index 0000000000..6dbf143ffe
> --- /dev/null
> +++ b/package/wolftpm/wolftpm.hash
> @@ -0,0 +1,2 @@
> +# Hash from https://github.com/wolfSSL/wolfTPM/archive/refs/tags/v2.3.1.tar.gz
> +sha256  f0d7c095491ac2cc9e44aa4ac3c22febf15942ef080431d8b43a9d0312ca6567  wolftpm-2.3.1.tar.gz

Gaah, I'm noticing now that the hash of the LICENSE file is missing,
and I didn't realize before applying. I will fix this up.

> diff --git a/package/wolftpm/wolftpm.mk b/package/wolftpm/wolftpm.mk
> new file mode 100644
> index 0000000000..1e1ddc13e3
> --- /dev/null
> +++ b/package/wolftpm/wolftpm.mk
> @@ -0,0 +1,29 @@
> +################################################################################
> +#
> +# wolftpm
> +#
> +################################################################################
> +
> +WOLFTPM_VERSION = 2.3.1
> +WOLFTPM_SITE = $(call github,wolfSSL,wolfTPM,v$(WOLFTPM_VERSION))
> +WOLFTPM_INSTALL_STAGING = YES
> +WOLFTPM_LICENSE = GPL-2.0+
> +WOLFTPM_LICENSE_FILES = LICENSE
> +WOLFTPM_CPE_ID_VENDOR = wolfssl
> +
> +WOLFTPM_DEPENDENCIES = host-pkgconf

I've added:

WOLFTPM_CONFIG_SCRIPTS = wolftpm-config

so that the wolftpm-config script installed in $(STAGING_DIR)/usr/bin
returns correct results.

> +# wolfTPM's source code is released without a configure script,
> +# therefore we need autoreconf
> +WOLFTPM_AUTORECONF = YES
> +
> +WOLFTPM_CONF_OPTS = --disable-examples --enable-devtpm

With just this, the build was failing for me, as wolftpm couldn't find
wolfssl. I had to add:

        --with-wolfcrypt=$(STAGING_DIR)/usr

> +define WOLFTPM_CONFIG_RPATH

I renamed the hook to WOLFTPM_TOUCH_CONFIG_RPATH

> +    mkdir $(@D)/build-aux

Changed to "mkdir -p" so that the hook can be re-executed without
failing.

> +    touch $(@D)/build-aux/config.rpath
> +endef
> +# Fix for autoconf bug with config.rconf

There is no autoreconf bug. The bug is in the code of wolftpm. In the
configure.ac script line 165, it uses the AC_LIB_HAVE_LINKFLAGS m4
macro, which comes from gnulib. This macro is documented at, which
specifies:

Example of using AC_LIB_LINKFLAGS

Suppose you want to use libz, the compression library.

   (1)  In configure.ac you add the line

      AC_CONFIG_AUX_DIR([build-aux])
      AC_LIB_LINKFLAGS([z])

    Note that since the AC_LIB_LINKFLAGS invocation modifies the
    CPPFLAGS, it should precede all tests that check for header files,
    declarations, structures or types.


    (2) To the package’s build-aux directory you add the file
    config.rpath, also part of the Gnulib havelib module. (gnulib-tool
    will usually do this for you automatically.)


    (3) In Makefile.in you add @LIBZ@ to the link command line of your
    program. Or, if you are using Automake, you add $(LIBZ) to the
    LDADD variable that corresponds to your program. 

See point (2) ? This is what wasn't done correctly in wolftpm when
integrating this gnulib m4 macro.

Ideally this should be fixed in the upstream wolftpm code. However, to
be honest, I'm not even sure why your configure.ac file is using
AC_LIB_HAVE_LINKFLAGS(). You should probably just migrate to use
pkg-config.

Thanks for your contribution!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot