Break DateTime <-> Serialization circular dependency

[Kojoley]

Breaks DateTime -> Serialization -> Spirit -> Thread -> DateTime chain.

The Serialization documentation seems to be explicitly allow this: https://www.boost.org/doc/libs/1_72_0/libs/serialization/doc/traits.html#version https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/serialization.html#splitting

[codecov[bot]]

Codecov Report

Merging #154 into develop will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #154      +/-   ##
===========================================
- Coverage    52.72%   52.71%   -0.02%     
===========================================
  Files           76       76              
  Lines         4491     4490       -1     
  Branches      2243     2243              
===========================================
- Hits          2368     2367       -1     
  Misses         391      391              
  Partials      1732     1732              

Impacted Files	Coverage Δ
...clude/boost/date_time/gregorian/greg_serialize.hpp	43.06% <100.00%> (-0.42%)	:arrow_down:
...lude/boost/date_time/posix_time/time_serialize.hpp	28.98% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3d55c15...d1bfd6a. Read the comment docs.

[JeffGarland]

So I guess one question about this is how much user code is broken by this changes -- is it none since users already had to include the particular archive type to use serialization?

Thinking about this from a design point of view I like breaking the header dependency, although practically speaking this doesn't extend for the whole library since tests still have serialization dependence - would be nice to conditionally detect lack of serialization and not run those. The downside here, btw is if serialization ever changes then this code will just break. But seems unlikely given this interface has been stable for a long time.

Broader Context: The bigger issue is making date_time less dependent the rest of boost for c++11 and beyond. lexical_cast is a big one that should be removed in favor of more modern std:: conversion functions. One wonders if the best way forward is to drop support for 98 to do that. Also fyi - on my personal todo list is to add support cereal (a non-boost serialization library). That code is in production and just needs to be put back into the library.

[Kojoley]

So I guess one question about this is how much user code is broken by this changes -- is it none since users already had to include the particular archive type to use serialization?

It should not break any user code because actual serialization routines are included on the user side. I say 'should' because removing some headers always can break some code that unintentionally relies on transient includes, but it is orthogonal problem.

Thinking about this from a design point of view I like breaking the header dependency, although practically speaking this doesn't extend for the whole library since tests still have serialization dependence - would be nice to conditionally detect lack of serialization and not run those.

My main goal is to have simpler dependency graph for Boost users; as you may expect they are not particularly interested in the test code and its dependencies :-) One can write a B2 check to detect missing headers and use it as a condition to turn some tests off, see https://boostorg.github.io/build/manual/develop/index.html#bbv2.reference.rules.check-target-builds You probably may be interested in https://github.com/boostorg/optional/issues/66 thread.

[JeffGarland]

B2 check...

Thanks for the pointer.

My main goal is to have simpler dependency graph for Boost users; as you may expect they are not particularly interested in the test code and its dependencies :-)

Right, but they might be if you could extract date_time from boost completely -- and yes, that's a big ask given the state of things -- and I'm not sure it's even a goal. But here's, the thing -- if you're using date_time and serialization together then you already have to have the includes. If you're not using serialization it already doesn't matter -- this header is not included in the global includes -- it has to be specifically included.

[Kojoley]

If you're not using serialization it already doesn't matter -- this header is not included in the global includes -- it has to be specifically included.

The thing is that dependency trackers (like https://pdimov.github.io/boostdep-report/develop/date_time.html) and dependency resolver tools (like https://www.boost.org/doc/tools/bcp/) are pessimistic about that and consider it as a hard dependency.

[JeffGarland]

The thing is that dependency trackers...are pessimistic about that and consider it as a hard dependency.

Well sure. To be clear, I'm not arguing against doing this -- we will. And actually now that I'm looking at Peter's report again this is more significant than first glance bc serialization drags in spirit and filesystem -- which is just nonsense. lexical_cast is maddening bc it drags in 'math' which led to upstream warnings since math is deprecating c++98 (John M did fix it though).

[pdimov]

Once math drops C++03 lexical_cast will have no excuse to forgo the use of the standard isinf functions in favor of math's ones. :-)

And many thanks to Nikita for this PR, these cycles are maddening. Serialization support in a library really shouldn't require Spirit. One is almost driven to rewrite Serialization's XML parser in a PR, with the full knowledge that Robert will then refuse to merge it.