Bug 240148 - devel/spdlog: unbundle fmtlib or update to 6.0.0
Summary: devel/spdlog: unbundle fmtlib or update to 6.0.0
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Vanilla I. Shu
URL:
Keywords: needs-patch
Depends on:
Blocks: 240141
  Show dependency treegraph
 
Reported: 2019-08-27 14:34 UTC by Jan Beich
Modified: 2019-09-01 06:14 UTC (History)
2 users (show)

See Also:
vanilla: maintainer-feedback+


Attachments
unbundle (like Gentoo) (4.08 KB, patch)
2019-08-28 17:11 UTC, Jan Beich
no flags Details | Diff
unbundle (like Gentoo), v2 (3.85 KB, patch)
2019-08-29 02:18 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2019-08-27 14:34:00 UTC
spdlog is currently built against bundled fmt 5.3.0. If both fmt and spdlog are used at the same time but fmt version is different the following may happen:

$ cat a.cc
#include <spdlog/spdlog.h>
#include <fmt/chrono.h>
int main() {}

$ c++ a.cc $(pkg-config --cflags --libs fmt spdlog)
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:12:
/usr/local/include/fmt/locale.h:19:35: error: parameter type 'fmt::v5::internal::buffer'
      (aka 'basic_buffer<char>') is an abstract class
    const std::locale& loc, buffer<Char>& buf,
                                  ^
/usr/local/include/spdlog/fmt/bundled/core.h:238:16: note: unimplemented pure virtual method 'grow' in
      'basic_buffer'
  virtual void grow(std::size_t capacity) = 0;
               ^
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:12:
/usr/local/include/fmt/locale.h:19:35: error: expected ')'
    const std::locale& loc, buffer<Char>& buf,
                                  ^
/usr/local/include/fmt/locale.h:18:51: note: to match this '('
typename buffer_context<Char>::iterator vformat_to(
                                                  ^
/usr/local/include/fmt/locale.h:22:17: error: no template named 'buffer_range'
  using range = buffer_range<Char>;
                ^
/usr/local/include/fmt/locale.h:23:35: error: use of undeclared identifier 'range'
  return vformat_to<arg_formatter<range>>(buf, to_string_view(format_str), args,
                                  ^
/usr/local/include/fmt/locale.h:24:69: error: expected a type
                                          internal::locale_ref(loc));
                                                                    ^
/usr/local/include/fmt/locale.h:49:18: error: no template named 'make_args_checked' in namespace
      'fmt::v5::internal'; did you mean 'make_checked'?
      {internal::make_args_checked<Args...>(format_str, args...)});
       ~~~~~~~~~~^~~~~~~~~~~~~~~~~
                 make_checked
/usr/local/include/spdlog/fmt/bundled/format.h:389:11: note: 'make_checked' declared here
inline T *make_checked(T *p, std::size_t) { return p; }
          ^
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:12:
/usr/local/include/fmt/locale.h:53:27: error: no template named 'enable_if_t'; did you mean 'std::enable_if_t'?
          typename Char = enable_if_t<
                          ^~~~~~~~~~~
                          std::enable_if_t
/usr/include/c++/v1/type_traits:444:39: note: 'std::enable_if_t' declared here
template <bool _Bp, class _Tp = void> using enable_if_t = typename enable_if<_Bp, _Tp>::type;
                                      ^
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:12:
/usr/local/include/fmt/locale.h:58:17: error: no template named 'output_range' in namespace 'fmt::v5::internal';
      did you mean simply 'output_range'?
  using range = internal::output_range<OutputIt, Char>;
                ^~~~~~~~~~~~~~~~~~~~~~
                output_range
/usr/local/include/spdlog/fmt/bundled/format.h:333:7: note: 'output_range' declared here
class output_range {
      ^
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:12:
/usr/local/include/fmt/locale.h:64:25: error: missing 'typename' prior to dependent type name
      'internal::is_output_iterator<OutputIt>::value'
          FMT_ENABLE_IF(internal::is_output_iterator<OutputIt>::value&&
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                        typename
/usr/local/include/fmt/locale.h:65:53: error: parameter declarator cannot be qualified
                            internal::is_string<S>::value)>
                            ~~~~~~~~~~~~~~~~~~~~~~~~^
/usr/local/include/fmt/locale.h:65:59: error: expected template parameter
                            internal::is_string<S>::value)>
                                                          ^
In file included from a.cc:2:
In file included from /usr/local/include/fmt/chrono.h:25:
/usr/local/include/fmt/safe-duration-cast.h:24:25: error: expected parameter declarator
          FMT_ENABLE_IF(!std::is_same<From, To>::value &&
                        ^
/usr/local/include/fmt/safe-duration-cast.h:24:25: error: expected ')'
/usr/local/include/fmt/safe-duration-cast.h:24:24: note: to match this '('
          FMT_ENABLE_IF(!std::is_same<From, To>::value &&
                       ^
/usr/local/include/fmt/safe-duration-cast.h:26:64: error: expected template parameter
                            std::numeric_limits<To>::is_signed)>
                                                               ^
/usr/local/include/fmt/safe-duration-cast.h:53:25: error: expected parameter declarator
          FMT_ENABLE_IF(!std::is_same<From, To>::value &&
                        ^
/usr/local/include/fmt/safe-duration-cast.h:53:25: error: expected ')'
/usr/local/include/fmt/safe-duration-cast.h:53:24: note: to match this '('
          FMT_ENABLE_IF(!std::is_same<From, To>::value &&
                       ^
/usr/local/include/fmt/safe-duration-cast.h:55:64: error: expected template parameter
                            std::numeric_limits<To>::is_signed)>
                                                               ^
/usr/local/include/fmt/safe-duration-cast.h:56:18: error: redefinition of 'lossless_integral_conversion'
FMT_CONSTEXPR To lossless_integral_conversion(const From from, int& ec) {
                 ^
/usr/local/include/fmt/safe-duration-cast.h:27:18: note: previous definition is here
FMT_CONSTEXPR To lossless_integral_conversion(const From from, int& ec) {
                 ^
/usr/local/include/fmt/safe-duration-cast.h:102:11: error: unknown type name 'FMT_ENABLE_IF'
          FMT_ENABLE_IF(std::is_same<From, To>::value)>
          ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
Comment 1 Jan Beich freebsd_committer 2019-08-28 17:11:51 UTC
Created attachment 206981 [details]
unbundle (like Gentoo)

FMT_HEADER_ONLY fix is suboptimal. <spdlog/fmt/fmt.h> and <spdlog/fmt/ostr.h> appear to be wrappers to cheat on libfmt dependency.
Comment 2 Jan Beich freebsd_committer 2019-08-29 02:18:50 UTC
Created attachment 206993 [details]
unbundle (like Gentoo), v2

- Avoid having to add -DFMT_HEADER_ONLY in consumers
- Don't break build with libfmt 5.3.0 (from upstream)
Comment 3 Tobias Kortkamp freebsd_committer 2019-08-30 07:06:09 UTC
Comment on attachment 206993 [details]
unbundle (like Gentoo), v2

I'm still not sure how I feel about preserving the header-only API
with system fmtlib.  I think it might be nicer to declare the libfmt
dependency in consumers explicitly instead.  However that rabbit
hole seems too deep to be worth it, so I'm ok with the patch as is.

Jan, have you already tested that all spdlog consumers also build
fine in combination with fmtlib 6.0.0 or is that still open?
Comment 4 Jan Beich freebsd_committer 2019-08-30 11:50:04 UTC
(In reply to Tobias Kortkamp from comment #3)
> Jan, have you already tested that all spdlog consumers also build
> fine in combination with fmtlib 6.0.0 or is that still open?

I've only tested spdlog against fmtlib 5.3.0 and 6.0.0 and consumers of both against fmtlib 6.0.0. The approach taken was exactly because dropping FMT_HEADER_ONLY required too much work e.g., how a consumer is supposed to figure out whether -lfmt is needed which can be upstreamed.
Comment 5 Jan Beich freebsd_committer 2019-08-30 12:20:39 UTC
(In reply to Jan Beich from comment #4)
> ... how a consumer is supposed to figure out whether -lfmt is needed which can be upstreamed.

For one, -lfmt can be part of `pkg-config --libs spdlog` (via Requires) but at least databases/tiledb and net-im/nheko use custom CMake module instead of pkg-config while sysutils/lizardfs doesn't use either relying on -I/usr/local/include from elsewhere.
Comment 6 commit-hook freebsd_committer 2019-09-01 06:10:43 UTC
A commit references this bug:

Author: tobik
Date: Sun Sep  1 06:09:52 UTC 2019
New revision: 510679
URL: https://svnweb.freebsd.org/changeset/ports/510679

Log:
  devel/spdlog: Use system fmt

  This is in preparation for updating devel/libfmt to 6.0.0.  Spdlog
  bundles fmt 5.3.0 which is incompatible with 6.0.0.  This would
  lead to problems in consumers that use both.

  PR:		240148
  Submitted by:	jbeich
  Approved by:	vanilla (maintainer)

Changes:
  head/devel/spdlog/Makefile
  head/devel/spdlog/distinfo
  head/devel/spdlog/files/
  head/devel/spdlog/files/patch-system-fmt
  head/devel/spdlog/pkg-plist
Comment 7 Tobias Kortkamp freebsd_committer 2019-09-01 06:14:47 UTC
Committed. Thanks!