[Buildroot] [PATCH 1/1] package/spdlog: add menuconfig option to use bundled fmt

[Buildroot] [PATCH 1/1] package/spdlog: add menuconfig option to use bundled fmt

[Joachim Wiberg]

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/spdlog/Config.in | 10 ++++++++++
 package/spdlog/spdlog.mk |  9 +++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/package/spdlog/Config.in b/package/spdlog/Config.in
index 877c1b6ab7..170e17a4f1 100644
--- a/package/spdlog/Config.in
+++ b/package/spdlog/Config.in
@@ -9,6 +9,16 @@ config BR2_PACKAGE_SPDLOG
 
 	  https://github.com/gabime/spdlog
 
+if BR2_PACKAGE_SPDLOG
+
+config BR2_PACKAGE_SPDLOG_FMT_EXTERNAL
+	bool "Use external fmt library"
+	default y
+	help
+	  Use external or bundled fmt library, default external.
+
+endif
+
 comment "spdlog needs a toolchain w/ C++, threads, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS \
 		|| !BR2_USE_WCHAR
diff --git a/package/spdlog/spdlog.mk b/package/spdlog/spdlog.mk
index 6b24357697..c2832c94f3 100644
--- a/package/spdlog/spdlog.mk
+++ b/package/spdlog/spdlog.mk
@@ -13,8 +13,13 @@ SPDLOG_DEPENDENCIES = fmt
 SPDLOG_CONF_OPTS += \
 	-DSPDLOG_BUILD_TESTS=OFF \
 	-DSPDLOG_BUILD_EXAMPLE=OFF \
-	-DSPDLOG_BUILD_BENCH=OFF \
-	-DSPDLOG_FMT_EXTERNAL=ON
+	-DSPDLOG_BUILD_BENCH=OFF
+
+ifeq ($(BR2_PACKAGE_SPDLOG_FMT_EXTERNAL),y)
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=ON
+else
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=OFF
+endif
 
 ifeq ($(BR2_STATIC_LIBS),y)
 SPDLOG_CONF_OPTS += -DSPDLOG_BUILD_SHARED=OFF
-- 
2.43.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/spdlog: add menuconfig option to use bundled fmt

[Baruch Siach via buildroot]

Hi Joachim,

On Sun, Aug 04 2024, Joachim Wiberg wrote:
> Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
> ---
>  package/spdlog/Config.in | 10 ++++++++++
>  package/spdlog/spdlog.mk |  9 +++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/package/spdlog/Config.in b/package/spdlog/Config.in
> index 877c1b6ab7..170e17a4f1 100644
> --- a/package/spdlog/Config.in
> +++ b/package/spdlog/Config.in
> @@ -9,6 +9,16 @@ config BR2_PACKAGE_SPDLOG
>  
>  	  https://github.com/gabime/spdlog
>  
> +if BR2_PACKAGE_SPDLOG
> +
> +config BR2_PACKAGE_SPDLOG_FMT_EXTERNAL
> +	bool "Use external fmt library"
> +	default y
> +	help
> +	  Use external or bundled fmt library, default external.

What is the advantage of bundled library? The commit log should explain
that.

Also, you should probably move 'select BR2_PACKAGE_FMT' here. We don't
have to build fmt if spdlog doesn't use it.

> +
> +endif
> +
>  comment "spdlog needs a toolchain w/ C++, threads, wchar"
>  	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS \
>  		|| !BR2_USE_WCHAR
> diff --git a/package/spdlog/spdlog.mk b/package/spdlog/spdlog.mk
> index 6b24357697..c2832c94f3 100644
> --- a/package/spdlog/spdlog.mk
> +++ b/package/spdlog/spdlog.mk
> @@ -13,8 +13,13 @@ SPDLOG_DEPENDENCIES = fmt
>  SPDLOG_CONF_OPTS += \
>  	-DSPDLOG_BUILD_TESTS=OFF \
>  	-DSPDLOG_BUILD_EXAMPLE=OFF \
> -	-DSPDLOG_BUILD_BENCH=OFF \
> -	-DSPDLOG_FMT_EXTERNAL=ON
> +	-DSPDLOG_BUILD_BENCH=OFF
> +
> +ifeq ($(BR2_PACKAGE_SPDLOG_FMT_EXTERNAL),y)
> +SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=ON

fmt dependency should also move here.

> +else
> +SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=OFF
> +endif
>  
>  ifeq ($(BR2_STATIC_LIBS),y)
>  SPDLOG_CONF_OPTS += -DSPDLOG_BUILD_SHARED=OFF

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/spdlog: add menuconfig option to use bundled fmt

[Joachim Wiberg]

Hi Baruch!

On Sun, 2024-08-04 at 09:20 +0300, Baruch Siach wrote:
> Hi Joachim,
> On Sun, Aug 04 2024, Joachim Wiberg wrote:
> > +config BR2_PACKAGE_SPDLOG_FMT_EXTERNAL
> > +	bool "Use external fmt library"
> > +	default y
> > +	help
> > +	  Use external or bundled fmt library, default external.
> What is the advantage of bundled library? The commit log should
> explain that.

We're trying to package Rousette (RESTCONF for sysrepo), it requires
the bundled fmt instead.   I'll update the patch, thanks!

> Also, you should probably move 'select BR2_PACKAGE_FMT' here. We
> don't have to build fmt if spdlog doesn't use it.

Ah, of course, thanks!

> > +ifeq ($(BR2_PACKAGE_SPDLOG_FMT_EXTERNAL),y)
> > +SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=ON
> 
> fmt dependency should also move here.

Got it!

Best regards
 /Joachim

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 1/1] package/spdlog: add menuconfig option to select formatter

[Joachim Wiberg]

Add an option menu to select the spdlog formatter: external fmt library,
external using headers-only, C++20 std:format, or the bundled (version
matched) fmt library, which each release of spdlog is tested against.

The formatters are mutually exclusive (see the CMakeLists.txt[1] file),
selecting the 'internal' option disables all the other options in the
.mk file, thus "enabling" the bundled formatter.

For backwards compatibility, the Buildroot default remains using the
external fmt library.

[1]: https://github.com/gabime/spdlog/blob/5ebfc92/CMakeLists.txt#L95-L105

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
 package/spdlog/Config.in | 31 ++++++++++++++++++++++++++++++-
 package/spdlog/spdlog.mk | 24 +++++++++++++++++++++---
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/package/spdlog/Config.in b/package/spdlog/Config.in
index 877c1b6ab7..ee31fa7e7c 100644
--- a/package/spdlog/Config.in
+++ b/package/spdlog/Config.in
@@ -3,12 +3,41 @@ config BR2_PACKAGE_SPDLOG
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_USE_WCHAR # fmt
-	select BR2_PACKAGE_FMT
 	help
 	  Very fast, header-only/compiled, C++ logging library.
 
 	  https://github.com/gabime/spdlog
 
+if BR2_PACKAGE_SPDLOG
+
+choice
+        prompt "Select text formatting library"
+        default BR2_PACKAGE_SPDLOG_FMT_EXTERNAL
+        help
+	  Select one of:
+	    - The external fmt library (default)
+	    - External fmt library in header-only mode
+	    - Bundled (version matched) fmt library
+	    - C++20 std::format
+
+config BR2_PACKAGE_SPDLOG_FMT_EXTERNAL
+        bool "external"
+	select BR2_PACKAGE_FMT
+
+config BR2_PACKAGE_SPDLOG_FMT_EXTERNAL_HO
+        bool "external (header-only)"
+	select BR2_PACKAGE_FMT
+
+config BR2_PACKAGE_SPDLOG_FMT_INTERNAL
+        bool "internal"
+
+config BR2_PACKAGE_SPDLOG_FMT_STD
+        bool "std:format"
+
+endchoice
+
+endif
+
 comment "spdlog needs a toolchain w/ C++, threads, wchar"
 	depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS \
 		|| !BR2_USE_WCHAR
diff --git a/package/spdlog/spdlog.mk b/package/spdlog/spdlog.mk
index 6b24357697..71ad64c923 100644
--- a/package/spdlog/spdlog.mk
+++ b/package/spdlog/spdlog.mk
@@ -9,12 +9,30 @@ SPDLOG_SITE = $(call github,gabime,spdlog,v$(SPDLOG_VERSION))
 SPDLOG_LICENSE = MIT
 SPDLOG_LICENSE_FILES = LICENSE
 SPDLOG_INSTALL_STAGING = YES
-SPDLOG_DEPENDENCIES = fmt
 SPDLOG_CONF_OPTS += \
 	-DSPDLOG_BUILD_TESTS=OFF \
 	-DSPDLOG_BUILD_EXAMPLE=OFF \
-	-DSPDLOG_BUILD_BENCH=OFF \
-	-DSPDLOG_FMT_EXTERNAL=ON
+	-DSPDLOG_BUILD_BENCH=OFF
+
+ifeq ($(BR2_PACKAGE_SPDLOG_FMT_EXTERNAL),y)
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=ON
+SPDLOG_DEPENDENCIES = fmt
+else
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_SPDLOG_FMT_EXTERNAL_HO),y)
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL_HO=ON
+SPDLOG_DEPENDENCIES = fmt
+else
+SPDLOG_CONF_OPTS += -DSPDLOG_FMT_EXTERNAL_HO=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_SPDLOG_FMT_STD),y)
+SPDLOG_CONF_OPTS += -DSPDLOG_USE_STD_FORMAT=ON
+else
+SPDLOG_CONF_OPTS += -DSPDLOG_USE_STD_FORMAT=OFF
+endif
 
 ifeq ($(BR2_STATIC_LIBS),y)
 SPDLOG_CONF_OPTS += -DSPDLOG_BUILD_SHARED=OFF
-- 
2.43.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2 1/1] package/spdlog: add menuconfig option to select formatter

[Thomas Petazzoni via buildroot]

Hello Joachim,

On Sun,  4 Aug 2024 11:46:47 +0200
Joachim Wiberg <troglobit@gmail.com> wrote:

> Add an option menu to select the spdlog formatter: external fmt library,
> external using headers-only, C++20 std:format, or the bundled (version
> matched) fmt library, which each release of spdlog is tested against.
> 
> The formatters are mutually exclusive (see the CMakeLists.txt[1] file),
> selecting the 'internal' option disables all the other options in the
> .mk file, thus "enabling" the bundled formatter.
> 
> For backwards compatibility, the Buildroot default remains using the
> external fmt library.
> 
> [1]: https://github.com/gabime/spdlog/blob/5ebfc92/CMakeLists.txt#L95-L105

Could you provide some details on why we would want to support
different formatters?

In your reply to Baruch you said you wanted to package Rousette, which
requires the bundled fmt. How is the bundled fmt different from the
external one?

Also using a choice...endchoice here will not make it easy to package
Rousette, because you can't "select" an option of a choice...endchoice,
so the Rousette package will not be able to select
BR2_PACKAGE_SPDLOG_FMT_INTERNAL.

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

Re: [Buildroot] [PATCH v2 1/1] package/spdlog: add menuconfig option to select formatter

[Joachim Wiberg]

Hi Thomas!

On Sun, 2024-08-04 at 22:22 +0200, Thomas Petazzoni wrote:
> On Sun,  4 Aug 2024 Joachim Wiberg <troglobit@gmail.com> wrote:
> > Add an option menu to select the spdlog formatter: external fmt
> > library, external using headers-only, C++20 std:format, or the
> > bundled (version matched) fmt library, which each release of spdlog
> > is tested against.
> > 
> > The formatters are mutually exclusive (see the CMakeLists.txt[1]
> > file), selecting the 'internal' option disables all the other
> > options in the .mk file, thus "enabling" the bundled formatter.
> Could you provide some details on why we would want to support
> different formatters?

I read up a bit on this.  Turns out spdlog started out with the bundled
(tested) fmt and later added an option to use an external one. From
what I could see, there have been several regressions caused by this,
changes in upstream fmt that did not work with spdlog.  In C++20 they
added std::logger support, meaning spdlog can now be used completely
without fmt, which seems to be the future direction of the project.

> In your reply to Baruch you said you wanted to package Rousette,
> which requires the bundled fmt. How is the bundled fmt different from
> the external one?

This was very unclear.  I had to go back to the original developer who
did our integration and check with them.  Turns out it was nothing more
than some linker issue, not properly pulling in the fmt dependency.  So
we can ignore/drop this patch since at least we do not need it anymore.

Sorry for taking up everyone's time!

> Also using a choice...endchoice here will not make it easy to package
> Rousette, because you can't "select" an option of a
> choice...endchoice, so the Rousette package will not be able to
> select BR2_PACKAGE_SPDLOG_FMT_INTERNAL.

Huh, did not know that.  OK, if something similar comes up later we can
revisit the construct and possible alternatives.

Best regards
 /Joachim


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot