When MSGPACK_USE_BOOST=OFF installed headers still depend on Boost

[yurivict]

For example, the installed file include/msgpack/v1/adaptor/cpp11/chrono.hpp includes boost/numeric/conversion/cast.hpp without being conditional on MSGPACK_USE_BOOST.

Version: 4.1.2
FreeBSD 13.1

[redboltz]

Thank you for reporting the issue.

When you pass the flag -DMSGPACK_USE_BOOST=OFF to cmake then C++ preprocessor macro MSGPACK_NO_BOOST should be defined.

msgpack-c/CMakeLists.txt

Lines 68 to 82 in fb64ea0

	IF (MSGPACK_USE_BOOST)
	SET (Boost_USE_MULTITHREADED ON)
	IF (MSGPACK_USE_STATIC_BOOST)
	MESSAGE (STATUS "Staticly linking with Boost")
	SET (Boost_USE_STATIC_LIBS TRUE)
	ELSE ()
	MESSAGE (STATUS "Dynamically linking with Boost")
	SET (Boost_USE_STATIC_LIBS FALSE)
	ENDIF ()
	FIND_PACKAGE (Boost REQUIRED)
	ELSE ()
	TARGET_COMPILE_DEFINITIONS(msgpackc-cxx INTERFACE MSGPACK_NO_BOOST)
	ENDIF ()

And if MSGPACK_NO_BOOST is defined, chrono adaptor isn't included from msgpack.hpp.

msgpack-c/include/msgpack/type.hpp

Lines 40 to 42 in fb64ea0

	#if !defined(MSGPACK_NO_BOOST)
	#include "adaptor/cpp11/chrono.hpp"
	#endif // !defined(MSGPACK_NO_BOOST)

However, if you include chrono adaptor directly, you got the error.
It is the same way as the following adaptors.

msgpack-c/include/msgpack/type.hpp

Lines 72 to 78 in fb64ea0

	#if !defined(MSGPACK_NO_BOOST)
	#include "adaptor/boost/fusion.hpp"
	#include "adaptor/boost/msgpack_variant.hpp"
	#include "adaptor/boost/optional.hpp"
	#include "adaptor/boost/string_ref.hpp"
	#include "adaptor/boost/string_view.hpp"
	#endif // !defined(MSGPACK_NO_BOOST)

But chrono is NOT a part of boost libraries, it is in the standard library.
So individual MSGPACK_NO_BOOST macro check for chrono is reasonable.

I will fix it soon.

[redboltz]

Could you try #1038 ?

[yurivict]

Sorry, I wouldn't be able to test it.
I believe that the change is good as long as all boost header includes are ifdefed.

[redboltz]

No problem. I will release fixed version soon.

[flo-carrere]

@redboltz I come here from #1025 and #1005 because I realized this problem is probably caused by the same root cause :

• some boost headers are actually exposed by the public msgpack-c headers even when MSGPACK_USE_BOOT=OFF

From my understanding it feels like you should then hide this dependency from the msgpack-c final users rather than:

• Propagating -DMSGPACK_NO_BOOST (as you implemented in Fixed #1025. #1028)
  • This is actually a "work-around" that works only for CMake users because you have to link to a CMake target to get the flags
• Asking the users to add an explicit -DMSGPACK_NO_BOOST to their own target when linking to msgpack-c/cxx
  • This does essentially the same as above but means that the problem is simply forwarded above and not fixed at msgpack-c level.

If you agree, I may be able to propose a PR for this or open a new issue to describe a potential solution to overcome the aforementioned issues.

[redboltz]

@flo-carrere , I'm not sure what you mean. Could you elaborate?

MSGPACK_NO_BOOST is publish flag for users. See https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_configure#msgpack_no_boost--since-410
User can set the flag on their project. I think that this is a correct way.
Users don't need to use cmake.

I'm not an expert of cmake. But if users want to use cmake and use msgpack-c as subproject, then MSGPACK_USE_BOOST=OFF can be used.
msgpack-c just support cmake step by step. It is not designed to use cmake mandatory.

msgpack-c is header only library so users don't need to link anyhing. (I mean archive)
I guess that link feature is used some cmake specific feature but link nothing acutually.

Maybe I misunderstand something. If creating PR is easier than explain by text, please send it but I can't promise it would be merged. It could be used for discussion base.

[flo-carrere]

@redboltz I understand, I'll try to be more specific.

Firstly, I think all comes from #1001 where you isolated as much as you could the boost dependency from msgpack.
Difficult to sum up what is good/bad in the approach you followed, I'm kind of OK with what you did but I think it's either incomplete (#1005 #1025 #1037 are examples of errors) or maybe the approach is not the right one.

More specifically on your questions:

MSGPACK_NO_BOOST is publish flag for users. See https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_configure#msgpack_no_boost--since-410
User can set the flag on their project. I think that this is a correct way.

The whole point here, is I think it is not acceptable to force the user to add this flag to their project.
The actual only thing the user should do is to use the MSGPACK_USE_BOOST option which is OK.
And every hiding/encapsulation of either the "with_boost" or "without_boost" flavor of msgpack should be handled at msgpack level.

Why? Because it's a problem of encapsulation/coupling
The user should not care about your internal MSGPACK_NO_BOOST setting, it just asks for msgpack (a set of headers) with/without a coupling to boost headers. And this is your promise.

Having to define MSGPACK_NO_BOOST at the user level, means that msgpack fails to fulfill the promise. And its problems are actually progated above which is a bad thing.

If this was acceptable, imagine a chain of two libraries A and B, A depending on B, B depending on msgpack. It would force either B to encapsulate properly msgpack or propagate again the error to A. Forcing A and B to define MSGPACK_NO_BOOST in their respective configuration to overcome the issue of msgpack at first as exposed above.

Users don't need to use cmake

I agree (this is actually my case not using cmake) and by defining -DMSGPACK_NO_BOOST I am able to circumvent the issue.
My point here, is that by providing #1028 you kind of take advantage of CMake build system to propagate this compile flag above in a somehow "elegant/hidden" way (working for CMake users only as you need to have cmake targets to link).

The reality is that it's just a more elegant way of providing the flag, but does not solve the issue which is it should not be defined at all as exposed above. And for non-CMake users, it makes no difference, which is a sign of this not addressing the real problem.

Maybe I misunderstand something. If creating PR is easier than explain by text, please send it but I can't promise it would be merged. It could be used for discussion base.

Creating a PR is definitely not easy (I don't have a full understanding yet).
But I can try to point you in the right direction, at least my current views on what is missing.

1. I think at this moment you have two flavors of msgpack "with_boost" (including boost headers) and "without_boost" (not supposed to include boost headers, but still including some apparently).

It feels like to do this separation you kind of "forked boost" as part of #1001 and provided some sort of redirection headers techniques to select one version or another, defining sometimes "empty headers" to fill gaps and rely on a single inclusion hierarchy (it's probably too summed up here).

Selection is internally based on #ifdef MSGPACK_NO_BOOST -> which is OK for private headers (if there are) as the flag is defined internally.

What is problematic is when this selection is done on public headers -> meaning that if that public header is included on the user side it forces the user project to also resolve that selection -> leading to the problem.

2. My solution (initial naive thought) would be that you add a step to msgpack CMake to actually generate the public headers that actively relies on MSGPACK_NO_BOOST to do that selection once and for all -> The installed version of the public header would be then "selection free".

In example (problem pointed by #1025):

// from include/msgpack/sysdep.hpp L99/102
#if defined(MSGPACK_NO_BOOST)
#include <msgpack/predef/other/endian.h>
#else  // defined(MSGPACK_NO_BOOST)
#include <boost/predef/other/endian.h>

The final include/msgpack/sysdep.hpp would either have one include or the other (boost one).

To do this you either have to version the two flavors in some special place (may be a bit expensive but probably) or you could maybe rely on a templated version include/msgpack/sysdep.hpp.in that could be "patched" at build time to the correct output.

[redboltz]

I think I just understood what is the biggest difference of our concept.

The whole point here, is I think it is not acceptable to force the user to add this flag to their project.
The actual only thing the user should do is to use the MSGPACK_USE_BOOST option which is OK.
And every hiding/encapsulation of either the "with_boost" or "without_boost" flavor of msgpack should be handled at msgpack level.

I disagree. msgpack-c has configuration preprosessor macros.
See https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_configure
This includes MSGPACK_NO_BOOST. It changes the library behavior.
Other flags do the similar effects. If user defined MSGPACK_USE_LEGACY_NAME_AS_FLOAT then the type is changed. User might get some compile errors on their existing code.
It is similar to MSGPACK_NO_BOOST.
I don't think there is no essential difference between MSGPACK_NO_BOOST and MSGPACK_USE_LEGACY_NAME_AS_FLOAT.

msgpack-c user needs to care this configuration. This is a part of APIs(customization flags). The default value is provided but it is just default. User needs to understand what is the default value and can modify flags.

msgpack-c's responsibility is that referring the flags and changing behavior as the flags expected.
user's responsibiliy is setting the flags as user's expected behavior. Managing the flags setting is user's responsibility.

It is not only msgpack-c style. Many other C++ libraries e.g. Boost do the similar approach.
See https://www.boost.org/doc/libs/1_80_0/doc/html/boost_asio/using.html#boost_asio.using.macros

[redboltz]

Another important point.

you could maybe rely on a templated version include/msgpack/sysdep.hpp.in that could be "patched" at build time to the correct output.

This is opposite direction of C++ version of msgpack-c want to go.
msgpack-c can be used just cloned (or download) and add it to the include path.

https://github.com/msgpack/msgpack-c/tree/cpp_master#usage

g++ -I msgpack-c/include path_to_boost your_source_file.cpp

( I just found invalid -I option before path_to_boost your_source_file.cpp`, I will fix it later but it is not related to the issue)

This is very important. Configure should be by configurration macros as follows. No preprocessing by build system or package management system (e.g. cmake, automake, conan, vcpkg etc) execution must not enforced.

g++ -I msgpack-c/include -DMSGPACK_SOME_CONFIG1 -DMSGPACK_SOME_CONFIG2  path_to_boost your_source_file.cpp

[flo-carrere]

I disagree. msgpack-c has configuration preprosessor macros.
See https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_configure
This includes MSGPACK_NO_BOOST. It changes the library behavior.

Don't misunderstand me, I agree with as many configuration flags as needed.

I agree with MSGPACK_USE_BOOST for instance, it's a user choice and msgpack-c has then to provide the desired implementation according to.

If internally, MSGPACK_NO_BOOST or whatever other flags is then defined as a consequence of a user choice, I agree. It's msgpack-c business to handle things correctly.

What I disagree with is that the user is also forced to define some msgpack-c internal flags (like MSGPACK_NO_BOOST) to accomodate with msgpack-c "without boost" implementation. In that regard, I disagree with you stating that it is a configuration flag (despite it being in the list), to me it's just an internal/private flags and should remain as-is ideally.

Rapidly replying on other configuration flags (like USE_LEGACY_FLOAT....) but it's outside the scope of this question to me:

• I agree a configuration flag will have an influence on user code, because it will drive the API availability and so on. It's fine and expected ofc.
• Again I'm not against config flags.

This is opposite direction of C++ version of msgpack-c want to go.
msgpack-c can be used just cloned (or download) and add it to the include path.

OK this is a fair approach and in this case, there is no possibility to overcome the issue I'm pointing. I would have expected a little bit more encapsulation without compromising this too much as anyway msgpack-c is provided with a CMakelist.txt that handles configuration/preprocessing steps up to the installation step.

It would just mean the installed version (after preprocessing) would be straitgh-forward. Not forcing any flag definition coupling in user code.