How exactly is SPDLOG_FMT_EXTERNAL supposed to be enabled?

[mortie]

There are two ways to tell spdlog that it's supposed to use an external libfmt rather than its own bundled copy: You can compile spdlog with cmake -DSPDLOG_FMT_EXTERNAL, which adds -DSPDLOG_FMT_EXTERNAL to the CFlags section of spdlog.pc (and thus adds that flag to the command line), or you can uncomment #define SPDLOG_FMT_EXTERNAL in tweakme.h. Either version works by itself, but if you both uncomment the line in tweakme.h and add the flag to cmake, you get a macro redefinition warning.

Currently, the Homebrew package does both approaches (as reported in Homebrew/homebrew-core#88896). Clearly, it has to either stop changing tweakme.h, or stop providing -DSPDLOG_FMT_EXTERNAL. Which approach is the most "correct" when packaging spdlog as a library? Should changing tweakme.h be preferred or should the cmake build option be preferred?

[gabime]

Setting the CMake option is the preferred way.

[mortie]

Let me see if I understand this correctly:

• At spdlog build time, -DSPDLOG_FMT_EXTERNAL=ON is needed to make cmake correctly link against the external libfmt and avoid bundling its own libfmt.
• However, when building a dependent application, the macro SPDLOG_FMT_EXTERNAL also needs to be defined because spdlog's headers uses that macro.
• And when packaged as suggested (i.e setting the CMake option, leaving tweakme.h unchanged), the only mechanism for setting SPDLOG_FMT_EXTERNAL in dependent code is through the CFlags fields of the pkg-config file.

The concern among Homebrew's packagers (whom I don't speak on behalf of, I'm just some random Homebrew and spdlog user) is that some people may choose to use spdlog from Homebrew without using pkg-config, which is apparently a use-case they wish to support. For the time being, I think the approach Homebrew will take is to keep passing the option to cmake, but also change tweakme.h to contain:

#ifndef SPDLOG_FMT_EXTERNAL
#define SPDLOG_FMT_EXTERNAL
#endif

Does this seem like a reasonable (at least short-term) solution?

And for the future, would it be possible for spdlog to generate a tweakme.h file rather than to set the flags using pkg-config? From my experience, it's a really common pattern to generate a config.h file (which tweakme.h essentially is) at configure time and use that to set the necessary config macros. I can't remember having seen a project use a generated pkg-config file to set config options like this before.

[ainola]

Would it make sense for upstream to, at the very least, change the tweakme.h file to uncomment // #define SPDLOG_FMT_EXTERNAL so that programs don't fail to compile when using a system SPDLOG installation? If they want all the bundled stuff they could always use a submodule.

As it stands, a user installing via a package manager would have to manually change the tweakme.h. This is a bit odd, having to tell spdlog "yes, I want it to behave as it was compiled to do please".

[tt4g]

I think it would be useful to define SPDLOG_FMT_EXTERNAL in tweakme.h in the spdlog provided by the system.

[herbrechtsmeier]

The following line set the compile definition for CMake and indirect for pkg-config.

target_compile_definitions(spdlog PUBLIC SPDLOG_FMT_EXTERNAL)

The following line should ensure that the compile definition is set for a ${PROJECT_NAME} target:

target_link_libraries(${PROJECT_NAME} PRIVATE spdlog::spdlog)

[herbrechtsmeier]

Does the tweakme.h make sense in this case as the dependent application maybe need to add -lfmt to the linker flags.

[ainola]

Hi, @gabime

I'm unable to find any associated commits - was this completed or just closed?

[gabime]

@ainola, This is outside of the scope of spdlog. Homebrew can install a modified tweakme.h file.

[ainola]

Not sure why this is outside the scope... this is quite literally about spdlog's compilation behavior. :)

[gabime]

There is no plan to automatically generate modified tweakme.h file. That being said, a PR is always welcome.