Re: [Buildroot] [PATCH v2 5/9] package/spirv-tools: new package

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

On Tue, 25 Jul 2023 19:30:57 +0200
Sebastian Weyer <sebastian.weyer@smile.fr> wrote:

> From: Romain Naour <romain.naour@smile.fr>
> 
> This package is required by mesa3d for building rusticl:
> 
> https://docs.mesa3d.org/rusticl.html
> 
> Signed-off-by: Romain Naour <romain.naour@smile.fr>
> Signed-off-by: Sebastian Weyer <sebastian.weyer@smile.fr>
> ---
>  package/Config.in                    |  1 +
>  package/spirv-tools/Config.in        | 12 ++++++++++++
>  package/spirv-tools/spirv-tools.hash |  2 ++
>  package/spirv-tools/spirv-tools.mk   | 22 ++++++++++++++++++++++
>  4 files changed, 37 insertions(+)

Entry in DEVELOPERS file was missing.

> diff --git a/package/spirv-tools/Config.in b/package/spirv-tools/Config.in
> new file mode 100644
> index 0000000000..21b3852779
> --- /dev/null
> +++ b/package/spirv-tools/Config.in
> @@ -0,0 +1,12 @@
> +config BR2_PACKAGE_SPIRV_TOOLS
> +	bool "spirv-tools"
> +	depends on BR2_USE_WCHAR # python3
> +	depends on BR2_USE_MMU # python3
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # python3
> +	depends on !BR2_STATIC_LIBS # python3
> +	select BR2_PACKAGE_PYTHON3

Not sure why you thought python3 was needed on the target. Reading
things a bit about spirv-tools, it doesn't seem like Python is needed,
so I dropped this.

However, it needs spirv-headers, and you had forgotten "select
BR2_PACKAGE_SPIRV_HEADERS" here.


> diff --git a/package/spirv-tools/spirv-tools.hash b/package/spirv-tools/spirv-tools.hash
> new file mode 100644
> index 0000000000..6861f543ea
> --- /dev/null
> +++ b/package/spirv-tools/spirv-tools.hash
> @@ -0,0 +1,2 @@

Missing "Locally calculated" comment here.

> +sha256  645c4585c207cbdfda81a09f8ab180773c18df50fad9644d2d9a53b4464c692b  spirv-tools-1.3.246.1.tar.gz
> +sha256  cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30  LICENSE
> diff --git a/package/spirv-tools/spirv-tools.mk b/package/spirv-tools/spirv-tools.mk
> new file mode 100644
> index 0000000000..758c01343c
> --- /dev/null
> +++ b/package/spirv-tools/spirv-tools.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# spirv-tools
> +#
> +################################################################################
> +
> +SPIRV_TOOLS_VERSION = 1.3.246.1

I bumped to a newer version to match spirv-headers, as otherwise
spirv-tools would fail to build. I also added a comment in both
spirv-headers and spirv-tools to hint that they should be kept in sync
in terms of version.

> +SPIRV_TOOLS_SITE = $(call github,KhronosGroup,SPIRV-Tools,sdk-$(SPIRV_TOOLS_VERSION))
> +SPIRV_TOOLS_LICENSE = Apache-2.0
> +SPIRV_TOOLS_LICENSE_FILES = LICENSE
> +
> +SPIRV_TOOLS_DEPENDENCIES = python3 spirv-headers

Dropped python3 here.

> +
> +SPIRV_TOOLS_INSTALL_STAGING = YES
> +
> +SPIRV_TOOLS_CONF_OPTS = \
> +    -DSPIRV-Headers_SOURCE_DIR=${STAGING_DIR}/usr \
> +    -DSPIRV_TOOLS_BUILD_STATIC=OFF \
> +    -DBUILD_SHARED_LIBS=ON \
> +    -DSPIRV_SKIP_TESTS=ON \

Indentation here was wrong (spaces used instead of tab), and
${STAGING_DIR} should have been $(STAGING_DIR). Both of these issues
were reported by "make check-package".

Also BUILD_SHARED_LIBS=ON is already passed by the cmake-package
infrastructure as required.

I've added -DSPIRV_WERROR=OFF because it exists, and -Werror is
annoying in the context of Buildroot.

Applied with those various changes. Thanks!

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