Re: [Buildroot] [PATCH v2 1/1] package/spdm-utils: new package

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

Hello Wilfred,

I wanted to merge this package, but got some build issues. See below.

On Tue,  2 Apr 2024 13:51:41 +1000
Wilfred Mallawa via buildroot <buildroot@buildroot.org> wrote:

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> ---
> Changes in V2:                                                   
>         - Added required python3 dependencies
>         - Now depends on `udev` instead of selecting `eudev`
>         - Now depends on `libspdm`
>         - Hook define moved below the hook, stylistically similar to
>             other packages
>         - Removed cargo package re-vendoring as we aren't patching
>         - Changed certificate/manifest installation path to be system wide.
> 
>  package/Config.in                  |  1 +
>  package/spdm-utils/Config.in       | 24 ++++++++++++++++++++
>  package/spdm-utils/spdm-utils.hash |  2 ++
>  package/spdm-utils/spdm-utils.mk   | 36 ++++++++++++++++++++++++++++++
>  4 files changed, 63 insertions(+)
>  create mode 100644 package/spdm-utils/Config.in
>  create mode 100644 package/spdm-utils/spdm-utils.hash
>  create mode 100644 package/spdm-utils/spdm-utils.mk

In order to validate the dependencies, I went back to a simpler
package, like this:

config BR2_PACKAGE_SPDM_UTILS
	bool "spdm-utils"
	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
	depends on BR2_PACKAGE_HAS_UDEV
	select BR2_PACKAGE_HOST_RUSTC
	help
	  SPDM-Utils is a Linux application designed to support,
	  test and develop SPDM requesters and responders.
	  SPDM-Utils uses libspdm as the backend to perform SPDM
	  communication. SPDM-Utils currently supports the
	  PCIe Data Object Exchange (DOE) Capability.

	  https://github.com/westerndigitalcorporation/spdm-utils

comment "spdm-utils needs udev /dev management"
	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
        depends on !BR2_PACKAGE_HAS_UDEV

and in the .mk file, reduced the DEPENDENCIES variable to:

SPDM_UTILS_DEPENDENCIES = udev

And with this, I get:

  Unable to find libclang: "couldn't find any valid shared libraries matching: ['libclang.so', 'libclang-*.so', 'libclang.so.*', 'libclang-*.so.*'], set the `LIBCLANG_PATH` environment variable to a path where one of these files can be found (invalid: [])"

However, none of your dependencies where related to libclang, so I
believe something is probably missing in terms of dependencies in your
submission. Could you double check?

Also, see below some comments.


> diff --git a/package/spdm-utils/Config.in b/package/spdm-utils/Config.in
> new file mode 100644
> index 0000000000..b96365451f
> --- /dev/null
> +++ b/package/spdm-utils/Config.in
> @@ -0,0 +1,24 @@
> +config BR2_PACKAGE_SPDM_UTILS
> +	bool "spdm-utils"
> +	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # python3
> +	depends on BR2_USE_WCHAR # eudev

Not needed, you simply need BR2_PACKAGE_HAS_UDEV.

> +	depends on !BR2_STATIC_LIBS

Please drop, you have this line duplicated below.

> +	depends on BR2_USE_MMU # eudev

We don't care about eudev dependencies, BR2_PACKAGE_HAS_UDEV is enough.
However, BR2_USE_MMU is needed for python3, so add # python3 at the end
of this line (if python3 is really needed, of course)

> +	depends on !BR2_STATIC_LIBS  # python3

> +	depends on BR2_USE_MMU  # eudev, python3
> +	depends on BR2_PACKAGE_HAS_UDEV
> +	depends on BR2_PACKAGE_LIBSPDM

Please select this library, but then you need:

	depends on BR2_PACKAGE_LIBSPDM_ARCH_SUPPORTS

> +	select BR2_PACKAGE_HOST_RUSTC
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_PCIUTILS
> +	select BR2_PACKAGE_EUDEV

This select is not needed.

> +	select BR2_PACKAGE_PYTHON3
> +	help
> +	  SPDM-Utils is a Linux application designed to support,
> +	  test and develop SPDM requesters and responders.
> +	  SPDM-Utils uses libspdm as the backend to perform SPDM
> +	  communication. SPDM-Utils currently supports the
> +	  PCIe Data Object Exchange (DOE) Capability.
> +
> +	  https://github.com/westerndigitalcorporation/spdm-utils

Please add Config.in comments about the dependencies. So probably:

comment "spdm-utils needs a toolchain w/ threads, dynamic library"
	depends on BR2_USE_MMU
	depends on BR2_PACKAGE_LIBSPDM_ARCH_SUPPORTS
	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS

comment "spdm-utils needs udev /dev management"
	depends on BR2_USE_MMU
	depends on BR2_PACKAGE_LIBSPDM_ARCH_SUPPORTS
	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
        depends on !BR2_PACKAGE_HAS_UDEV


> diff --git a/package/spdm-utils/spdm-utils.hash b/package/spdm-utils/spdm-utils.hash
> new file mode 100644
> index 0000000000..aaa243315e
> --- /dev/null
> +++ b/package/spdm-utils/spdm-utils.hash
> @@ -0,0 +1,2 @@
> +# Locally calculated
> +sha256  3f06f087220b126262a2becf68c9e06a59d8d613816f82a168c81093de087d1a  spdm-utils-0.3.0.tar.gz

This needs to be fixed with recent changes in the Cargo infrastructure.
Do a build, you'll notice.

> diff --git a/package/spdm-utils/spdm-utils.mk b/package/spdm-utils/spdm-utils.mk
> new file mode 100644
> index 0000000000..4176f61871
> --- /dev/null
> +++ b/package/spdm-utils/spdm-utils.mk
> @@ -0,0 +1,36 @@
> +################################################################################
> +#
> +# spdm-utils
> +#
> +################################################################################
> +
> +SPDM_UTILS_VERSION = 0.3.0
> +SPDM_UTILS_SITE = $(call github,westerndigitalcorporation,spdm-utils,v$(SPDM_UTILS_VERSION))
> +SPDM_UTILS_LICENSE = Apache-2.0 or MIT
> +SPDM_UTILS_DEPENDENCIES += pciutils libspdm openssl udev

Just =, not +=. What about python3? Not needed as a dependency?

> +
> +# Note that we also copy `setup_certs.sh` and `generate_certs.sh`.
> +# `setup_certs.sh` shall be used by a responder to regenerate it's mutable
> +# certificate chain. `generate_certs.sh` can be used to generate a new
> +# certificate chain, which maybe useful in testing and development.
> +define SPDM_UTILS_INSTALL_CERTS
> +	$(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/local/spdm_certs
> +	cp -r $(@D)/certs/generate_certs.sh $(TARGET_DIR)/usr/local/spdm_certs
> +	cp -r $(@D)/certs/openssl.cnf $(TARGET_DIR)/usr/local/spdm_certs
> +	cp -r $(@D)/certs/setup_certs.sh $(TARGET_DIR)/usr/local/spdm_certs
> +
> +	$(INSTALL) -d -m 0755 $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/immutable.der $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/device.cert.der $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/device.der $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/device.key $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/param.pem $(TARGET_DIR)/usr/local/spdm_certs/slot0
> +	cp $(@D)/certs/slot0/bundle_responder.certchain.der $(TARGET_DIR)/usr/local/spdm_certs/slot0

Meh, a bit verbose. Maybe:

SPDM_UTILS_CERT_SCRIPTS = \
	generate_certs.sh \
	setup_certs.sh

SPDM_UTILS_CERT_DATA_FILES = \
	openssl.cnf \
	slot0/immutable.der \
	slot0/device.cert.der \
	slot0/device.der \
	slot0/device.key \
	slot0/param.pem \
	slot0/bundle_responder.certchain.der

and then in SPDM_UTILS_INSTALL_CERTS:

	$(foreach f,$(SPDM_UTILS_CERT_DATA_FILES),
		$(INSTALL) -D -m 0644 $(f) $(TARGET_DIR)/usr/local/spdm_certs/$(f)
	)

	$(foreach f,$(SPDM_UTILS_CERT_SCRIPTS),
		$(INSTALL) -D -m 0755 $(f) $(TARGET_DIR)/usr/local/spdm_certs/$(f)
	)

Also, why do we install those files in /usr/local/ ?
/usr/local/spdm_certs and /usr/local/spdm_manifest seem like very
unconventional paths. I would rather expect those in
/usr/share/spdm/certs/ and /usr/share/spdm/manifest/ or something like
this.

Could you have a look at those different suggestions, and come back
with a v3 of your submission?

Thanks a lot!

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