Re: [Buildroot] [PATCH 1/1] package/cpp-httplib: new package

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Aleksandr Makarov <aleksandr.o.makarov@gmail.com>
Cc: buildroot@buildroot.org, Eric Le Bihan <eric.le.bihan.dev@free.fr>
Subject: Re: [Buildroot] [PATCH 1/1] package/cpp-httplib: new package
Date: Sat, 26 Oct 2024 22:52:57 +0200	[thread overview]
Message-ID: <20241026225257.37df6834@windsurf> (raw)
In-Reply-To: <20240926124131.930145-1-aleksandr.o.makarov@gmail.com>

Hello Aleksandr,

On Thu, 26 Sep 2024 15:41:31 +0300
Aleksandr Makarov <aleksandr.o.makarov@gmail.com> wrote:

> Signed-off-by: Aleksandr Makarov <aleksandr.o.makarov@gmail.com>

Thanks, I have applied your patch after doing a few tweaks.

> ---
>  package/Config.in                    |  1 +
>  package/cpp-httplib/Config.in        | 21 +++++++++++++
>  package/cpp-httplib/cpp-httplib.hash |  3 ++
>  package/cpp-httplib/cpp-httplib.mk   | 44 ++++++++++++++++++++++++++++
>  4 files changed, 69 insertions(+)

An entry in the DEVELOPERS should be added, so I did that.

> diff --git a/package/cpp-httplib/Config.in b/package/cpp-httplib/Config.in
> new file mode 100644
> index 0000000000..ee24adff81
> --- /dev/null
> +++ b/package/cpp-httplib/Config.in
> @@ -0,0 +1,21 @@
> +comment "cpp-httplib needs a toolchain w/ C++, wchar, threads"
> +	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_USE_WCHAR || !BR2_TOOLCHAIN_HAS_THREADS
> +
> +config BR2_PACKAGE_CPP_HTTPLIB
> +	bool "cpp-httplib"
> +	depends on BR2_INSTALL_LIBSTDCPP
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on BR2_USE_WCHAR
> +	help
> +	  A C++ header-only HTTP/HTTPS server and client library.
> +
> +	  https://github.com/yhirose/cpp-httplib
> +
> +if BR2_PACKAGE_CPP_HTTPLIB
> +config BR2_PACKAGE_CPP_HTTPLIB_COMPILE
> +	bool "compile"

"compile" was a bit short, so I changed to:

+       bool "compile as a shared library"
> diff --git a/package/cpp-httplib/cpp-httplib.mk b/package/cpp-httplib/cpp-httplib.mk
> new file mode 100644
> index 0000000000..9b66114851
> --- /dev/null
> +++ b/package/cpp-httplib/cpp-httplib.mk
> @@ -0,0 +1,44 @@
> +################################################################################
> +#
> +# cpp-httplib
> +#
> +################################################################################
> +
> +CPP_HTTPLIB_VERSION = 0.18.0
> +CPP_HTTPLIB_SITE = $(call github,yhirose,cpp-httplib,v$(CPP_HTTPLIB_VERSION))
> +CPP_HTTPLIB_LICENSE = MIT
> +CPP_HTTPLIB_LICENSE_FILES = LICENSE
> +CPP_HTTPLIB_CONF_OPTS = \
> +	-Dcpp-httplib_test=false
> +
> +

One two many lines.

> +ifeq ($(BR2_PACKAGE_CPP_HTTPLIB_COMPILE),y)
> +CPP_HTTPLIB_CONF_OPTS += -Dcpp-httplib_compile=true
> +CPP_HTTPLIB_DEPENDENCIES += host-python3
> +else
> +CPP_HTTPLIB_INSTALL_STAGING = YES

You tried to do the right thing, but this wasn't the correct thing. In
fact, what was needed is:

CPP_HTTPLIB_INSTALL_STAGING = YES

unconditionally.

And

CPP_HTTPLIB_INSTALL_TARGET = NO when BR2_PACKAGE_CPP_HTTPLIB_COMPILE is
disabled.

Indeed, when BR2_PACKAGE_CPP_HTTPLIB_COMPILE is disabled, it's a header
only library. The header is needed in staging but not in tgarget.

When BR2_PACKAGE_CPP_HTTPLIB_COMPILE is enabled, you have a shared
library, it's needed both in the target and staging.

> +$(eval $(meson-package))
> +

Final empty line not needed.

Once again: I fixed those issues when applying.

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