Re: [Buildroot] [PATCH v4 1/2] package/dlib: new package

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

Hello Roy,

On Mon,  9 Sep 2024 14:11:12 +0200
Roy Kollen Svendsen <roykollensvendsen@gmail.com> wrote:

> Signed-off-by: Roy Kollen Svendsen <roykollensvendsen@gmail.com>

Thanks for the patch!

> Should I enforce using system libraries where alternative internal libraries exist?

Yes, we always want to use system libraries instead of bundled ones.

> Do I add too much license information here? (I assume I only need BSL-1.0)

I think the license information you provided is good as it is.

> diff --git a/package/dlib/Config.in b/package/dlib/Config.in
> new file mode 100644
> index 0000000000..77ff5086ae
> --- /dev/null
> +++ b/package/dlib/Config.in
> @@ -0,0 +1,60 @@
> +config BR2_PACKAGE_DLIB
> +	bool "dlib"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	help
> +	  Dlib is a modern C++ toolkit containing machine learning
> +	  algorithms and tools for creating complex software in C++ to
> +	  solve real world problems.
> +
> +	  http://dlib.net
> +
> +if BR2_PACKAGE_DLIB
> +
> +config BR2_PACKAGE_DLIB_GIF_SUPPORT
> +	bool "GIF support"
> +	select BR2_PACKAGE_GIFLIB

I think you should drop all sub-options, and instead rely on automatic
dependencies, i.e:

ifeq ($(BR2_PACKAGE_GIFLIB),y)
... enable GIF support
else
... disable GIF support
endif

and ditto for all other options.

Last but not least, the addition of the entry in the DEVELOPERS file
should be done as part of the patch adding the package.

Could you adjust those small details and sent a new iteration, which I
believe we will be able to merge?

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