spdlog icon indicating copy to clipboard operation
spdlog copied to clipboard

Auto generate tweakme.h

Open xvitaly opened this issue 1 year ago • 17 comments

It looks like the include/spdlog/tweakme.h file should be automatically generated by CMake depending on the build configuration.

Various Linux distributions need to manually patch it in downstream to resolve issues with packages not using pkg-config or CMake.

xvitaly avatar Apr 26 '24 12:04 xvitaly

It looks like the include/spdlog/tweakme.h file should be automatically generated by CMake depending on the build configuration.

CMake shouldn't generate tweakme.h, what makes you think it does?

tt4g avatar Apr 26 '24 14:04 tt4g

CMake shouldn't generate tweakme.h, what makes you think it does?

I think it should be auto-generated for feature parity with pkg-config and CMake configurations.

xvitaly avatar Apr 26 '24 17:04 xvitaly

Sorry. I am not a native speaker so I misunderstood the meaning.

spdlog was initially a header-only library. The option to build as a shared or static library was added later. I believe that when it was header-only, there was no automatic tweakme.h generation because there was no significant difference between downloading and installing. It is technically possible to generate tweakme.h at build time. However, this may cause inconvenience for some users who still use spdlog as a header-only library and they have multiple projects using spdlog (even though spdlog is installed as a library, users can use it as a header-only library with a CMake target named spdlog::spdlog_header_only).

The final decision must be made by the maintainer @gabime.

tt4g avatar Apr 26 '24 21:04 tt4g

However, this may cause inconvenience for some users who still use spdlog as a header-only library and they have multiple projects using spdlog (even though spdlog is installed as a library, users can use it as a header-only library with a CMake target named spdlog::spdlog_header_only).

Auto-generation can be enabled only if the SPDLOG_BUILD_SHARED flag is set.

xvitaly avatar Apr 26 '24 21:04 xvitaly

spdlog::spdlog_header_only target is added with or without the SPDLOG_BUILD_SHARED flag. When installed by CMake, spdlog can be either header-only and static/shared library.

tt4g avatar Apr 26 '24 21:04 tt4g

When installed by CMake, spdlog can be either header-only and static/shared library.

Yes, but it has major issues with packages not using pkg-config or CMake. For more information please check this ticket.

xvitaly avatar Apr 26 '24 21:04 xvitaly

I’ve been working on something like this in the v2.x branch. cmake would generate a config.h file. Not sure yet if this the right approach though..

gabime avatar Apr 26 '24 21:04 gabime

If this is considered a major change, it would make sense to automatically generate a configuration file from v2. I personally think it is better to keep the style of macro definition by the developer as before, as it may confuse some users to automatically generate tweakme.h in v1.

tt4g avatar Apr 26 '24 21:04 tt4g

cmake would generate a config.h file. Not sure yet if this the right approach though..

Looks good. Is it possible to backport this to 1.x?

xvitaly avatar Apr 26 '24 21:04 xvitaly

Porting it to v1.x would break current usage of tweakme.h or add confusion if generate a new file in addition to tweakme.h.

gabime avatar Apr 26 '24 21:04 gabime

Porting it to v1.x would break current usage of tweakme.h or add confusion if generate a new file in addition to tweakme.h.

Even if it will be optional and disabled by default? SPDLOG_GENERATE_CONFIG for example. This feature can be very useful for Linux package maintainers.

If enabled, the config.h file will be generated and installed instead of tweakme.h.

xvitaly avatar Apr 26 '24 22:04 xvitaly

How would the new config.h be included by the code? Also note that when consuming spdlog using cmake find_ pacakge, cmake already adds the right defines(e.g. target_compile_definitions(spdlog PUBLIC SPDLOG_SHARED_LIB) ,so what would happen if both methods used. Still seems confusing.

gabime avatar Apr 26 '24 22:04 gabime

If a macro was defined when the library was built, CMake will define the same macro for the application to be linked. If the same macro is also defined in config.h, the macro definitions will be duplicated, but as long as the definitions are the same, this should not be a problem.

tt4g avatar Apr 27 '24 03:04 tt4g

How would the new config.h be included by the code?

Something like that:

#if __has_include(<spdlog/config.h>)
#include <spdlog/config.h>
#else
#include <spdlog/tweakme.h>
#endif

Also note that when consuming spdlog using cmake find_ pacakge, cmake already adds the right defines(e.g. target_compile_definitions(spdlog PUBLIC SPDLOG_SHARED_LIB) ,so what would happen if both methods used. Still seems confusing.

Yes, but we're talking about a situation where the packages don't use pkg-config or CMake.

xvitaly avatar Apr 27 '24 09:04 xvitaly

__has_include is c++17 while spdlog v1.x is c++11

gabime avatar Apr 27 '24 09:04 gabime

__has_include is c++17 while spdlog v1.x is c++11

Another option is to install the generated config.h file under the name tweakme.h if the SPDLOG_GENERATE_CONFIG flag is enabled.

xvitaly avatar Apr 27 '24 09:04 xvitaly

Perhaps a SPDLOG_GENERATE_TWEAKME_Hcmake option (default=OFF) is indeed good way to solve this for 1.x.

PR is welcme.

gabime avatar Apr 27 '24 12:04 gabime