Re: [Buildroot] [PATCH 2/2] package/imx-cst: add new package

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

Hello Thomas,

On Tue, 19 Apr 2022 11:21:51 +0200
Thomas Perrot <thomas.perrot@bootlin.com> wrote:

>  N:	Thomas Perrot <thomas.perrot@bootlin.com>
>  F:	package/byacc
> +F:	package/imx-cst

Trailing slash needed.

> diff --git a/package/imx-cst/Config.in.host b/package/imx-cst/Config.in.host
> new file mode 100644
> index 000000000000..84cb825e59cf
> --- /dev/null
> +++ b/package/imx-cst/Config.in.host
> @@ -0,0 +1,8 @@
> +config BR2_PACKAGE_HOST_IMX_CST
> +	bool "host imx-cst"
> +	help
> +	  I.MX code signing tool provides software code signing
> +	  support designed that integrate the HABv4 and AHAB
> +	  library.
> +
> +	  https://www.nxp.com/webapp/sps/download/license.jsp?colCode=IMX_CST_TOOL

When I go to this page, I need to agree to a "SOFTWARE LICENSE
AGREEMENT i.MX CODE SIGNING TOOL", but then your .mk is pointing to a
non-NXP Git repository, and you state the license is BSD-3-Clause.

> +# debian/3.3.1+dfsg-2
> +IMX_CST_SITE = https://gitlab.apertis.org/pkg/imx-code-signing-tool.git

What is this location? Just someone who put the NXP code online? Or a
separate implementation?

> +IMX_CST_SITE_METHOD = git
> +IMX_CST_VERSION = e2c687a856e6670e753147aacef42d0a3c07891a
> +IMX_CST_LICENSE = BSD-3-Clause
> +IMX_CST_LICENSE_FILES = LICENSE.bsd3

There are two other license files in this Git repo, the hidapi license
and the OpenSSL license. You are confident they are not relevant? Would
be worth explaining that.

> +HOST_IMX_CST_DEPENDENCIES = host-byacc host-flex host-openssl
> +
> +define HOST_IMX_CST_BUILD_CMDS
> +	$(HOST_MAKE_ENV) $(MAKE) $(HOST_CONFIGURE_OPTS) \
> +		OSTYPE=linux64 \

Would be useful to explain why linux64 is fine even on 32-bit host
platforms. Actually, when I look at
https://gitlab.apertis.org/pkg/imx-code-signing-tool/-/blob/apertis/v2023dev2/code/cst/code/obj.linux64/Makefile
I believe that using OSTYPE=linux64 on 32-bit host machines will not
work, as it adds -m64 to the compiler flags.

> +		ENCRYPTION=yes \
> +		COPTIONS="$(HOST_CFLAGS) $(HOST_CPPFLAGS)" \
> +		LDFLAGS="$(HOST_LDFLAGS) -lcrypto" \
> +		PWD=$(@D)/code/cst \
> +		CC="$(CC) -I../../code/common/hdr \
> +			-I../../code/back_end/hdr \
> +			-I../../code/srktool/hdr \
> +			-I../../code/front_end/hdr \
> +			-I../../code/convlb/hdr" \

This looks strange. Flags are normally passed using CFLAGS, and $(CC)
in Buildroot doesn't exist, it's $(HOSTCC) that should be used.

Also, I think the list of header paths is properly set with:

CINCLUDES := $(SUBSYS:%=-I$(CST_CODE_PATH)/%/hdr)

but it gets overridden by the fact that you override CFLAGS.

I believe there's a better way, let me have a look.

> +		LD=$(CC) \

$(CC) doesn't exist, it should be $(HOSTCC).

> +		-C $(@D)/code/cst \
> +		build

Best regards,

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