Re: [Buildroot] [PATCH v2 1/1] package/spirv-tools: fix build issue

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: Maciej Grela <enki@fsck.pl>
Cc: buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v2 1/1] package/spirv-tools: fix build issue
Date: Sun, 29 Oct 2023 00:13:10 +0200	[thread overview]
Message-ID: <20231028221310.GE687180@scaer> (raw)
In-Reply-To: <20231027091526.2224829-1-enki@fsck.pl>

Maciej, All,

On 2023-10-27 11:15 +0200, Maciej Grela spake thusly:
> spirv-tools requires a C++ toolchain with shared libs, add flag dependency and comment.

Fabrice had already sent a patch adding C++ requirement, and since it
also was more strict in requiring C++17, I applied his patch.

> Fixes: http://autobuild.buildroot.net/results/53b3cf124d6c299398e620120465eca396ddcdff
> Fixes: http://autobuild.buildroot.net/results/a1f615d2410bffb6fdacc8586761c9def05aafa9
> Fixes: http://autobuild.buildroot.net/results/294ade8c9aa3d650fa5ab6cc34701c4176bc197f
> Fixes: http://autobuild.buildroot.net/results/40e8f50eb7e82ca42f84fa5f4336cefb51204a28
> Fixes: http://autobuild.buildroot.net/results/3c3be2c454863709d0db896bafbe11cd9df092a3
> Fixes: http://autobuild.buildroot.net/results/8df8de1aeaf1317f6ba43f6752502da2eedcdea1
> Fixes: http://autobuild.buildroot.net/results/ea2c1f97e82efcd867c79f05ab02f5f099140379
> Fixes: http://autobuild.buildroot.net/results/fa4e39d1e78e885d20851201ed8a6fcf9d48d7d6
> Fixes: http://autobuild.buildroot.net/results/4ed3f13b3ce178f8fe87de7aee86acb9642e3434
> Fixes: http://autobuild.buildroot.net/results/e1b4853a7e8714fd2af8f8677681c9895034b3e8

AFAICS, none of those relate to shared libraries, but only to C++.

Can you point to a build failure that is explicitly about shared libs?

> Signed-off-by: Maciej Grela <enki@fsck.pl>
> ---
>  package/spirv-tools/Config.in | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/package/spirv-tools/Config.in b/package/spirv-tools/Config.in
> index e2f46253a9..f1ae11bd09 100644
> --- a/package/spirv-tools/Config.in
> +++ b/package/spirv-tools/Config.in
> @@ -1,6 +1,11 @@
> +comment "spirv-tools needs a toolchain w/ C++, dynamic library"
> +	depends on !BR2_INSTALL_LIBSTDCPP || ( !BR2_SHARED_LIBS && !BR2_SHARED_STATIC_LIBS )
> +
>  config BR2_PACKAGE_SPIRV_TOOLS
>  	bool "spirv-tools"
>  	select BR2_PACKAGE_SPIRV_HEADERS
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_SHARED_LIBS || BR2_SHARED_STATIC_LIBS

Also this is technically correct (and I would thinkg semantically the
best), we usally use the shorter dependnecy on !static:

    depends on !BR2_STATIC_LIBS

You also forgot to add a comment for when the condition is not met.

So, if sahred libs is really required, can you respin a patch with the
above fixed, please?

Regards,
Yann E. MORIN.

>  	help
>  	  The SPIR-V Tools project provides an API and commands for
>  	  processing SPIR-V modules.
> -- 
> 2.39.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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