Re: [Buildroot] [PATCH 2/2] package/libspdm: Initial commit

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Alistair, All,

On 2023-07-21 13:21 +1000, Alistair Francis spake thusly:
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
[--SNIP--]
> diff --git a/package/libspdm/Config.in b/package/libspdm/Config.in
> new file mode 100644
> index 0000000000..3051fc8c59
> --- /dev/null
> +++ b/package/libspdm/Config.in
> @@ -0,0 +1,8 @@
> +config BR2_PACKAGE_LIBSPDM
> +	bool "libspdm"
> +	depends on BR2_PACKAGE_OPENSSL

You can select BR2_PACKAGE_OPENSSL. That will ensure that either openssl
(libopenssl) or libressl are enabled. If your package can work with
either, then you're all set with just that:

    select BR2_PACKAGE_OPENSSL

If your package really want opensll (libopenssl), then you must state
so:

    select BR2_PACKAGE_OPENSSL
    select BR2_PACKAGE_OPENSSL_FORCE_LIBOPENSSL

[--SNIP--]
> diff --git a/package/libspdm/libspdm.mk b/package/libspdm/libspdm.mk
> new file mode 100644
> index 0000000000..1f8012f599
> --- /dev/null
> +++ b/package/libspdm/libspdm.mk
> @@ -0,0 +1,75 @@
> +################################################################################
> +#
> +# libspdm
> +#
> +################################################################################
> +
> +LIBSPDM_VERSION = 3.0.0
> +LIBSPDM_SITE = $(call github,DMTF,libspdm,$(LIBSPDM_VERSION))
> +LIBSPDM_LICENSE = BSD-3-Clause
> +LIBSPDM_LICENSE_FILES = LICENSE.md
> +
> +LIBSPDM_INSTALL_STAGING = YES
> +LIBSPDM_INSTALL_TARGET = YES
> +
> +LIBSPDM_DEPENDENCIES += openssl
> +
> +ifeq ($(BR2_arcle)$(BR2_arceb),y)
> +LIBSPDM_TARGET_CPU_FAMILY = arc
> +else ifeq ($(BR2_arm)$(BR2_armeb),y)
> +LIBSPDM_TARGET_CPU_FAMILY = arm
> +else ifeq ($(BR2_aarch64)$(BR2_aarch64_be),y)
> +LIBSPDM_TARGET_CPU_FAMILY = aarch64
> +else ifeq ($(BR2_i386),y)
> +LIBSPDM_TARGET_CPU_FAMILY = ia32
> +else ifeq ($(BR2_riscv)$(BR2_RISCV_32),yy)
> +LIBSPDM_TARGET_CPU_FAMILY = riscv32
> +else ifeq ($(BR2_riscv)$(BR2_RISCV_64),yy)
> +LIBSPDM_TARGET_CPU_FAMILY = riscv64
> +else ifeq ($(BR2_x86_64),y)
> +LIBSPDM_TARGET_CPU_FAMILY = x64
> +else
> +LIBSPDM_TARGET_CPU_FAMILY = $(ARCH)
> +endif

I like that such conditions be written in Config.in, even if this is not
a hard requirement:

    config BR2_PACKAGE_LIBSPDM_CPU_FAMILLY
        string
        default "arc" if BR2_arcle || BR2_arceb
        default "arm" if BR2_arm || BR2_armeb
        ...
        default BR2_ARCH  # Yes, this should work...

and then in the .mk:

    LIBSPDM_TARGET_CPU_FAMILY = $(call qstrip,$(BR2_PACKAGE_LIBSPDM_CPU_FAMILLY))

[--SNIP--]
> +define LIBSPDM_INSTALL_STAGING_CMDS
> +	$(INSTALL) -D -m 0755 $(@D)/lib/* $(STAGING_DIR)/usr/lib/

You need to ensure that the destination direcotry exists before you copy
multiple files in one go there; what we usually do is:

    $(INSTALL) -m 075 -t $(STAGING_DIR)/usr/lib/ $(@D)/lib/*

or the usual mkdir-then-install dance.

> +	$(INSTALL) -d $(STAGING_DIR)/usr/include/library

The set of isntall directories:
    /usr/include/library/
    /usr/include/internal/
    /usr/include/hal/
    /usr/include/industry_standard/

are very generic and confusing. Should they not be in a common
sub-directory, e.g.:
    /usr/include/spdm/library/
    /usr/include/spdm/internal/
    /usr/include/spdm/hal/
    /usr/include/spdm/industry_standard/

Because I can at least see 'hal' conflict with a lot of other HALs (and
don't wait for Dave...)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot