> On July 2, 2018, 11:14 a.m., Alexander Rojas wrote: > > 3rdparty/stout/include/stout/json.hpp > > Lines 25 (patched) > > <https://reviews.apache.org/r/67632/diff/3/?file=2045099#file2045099line25> > > > > This breaks compilation if someone already has define > > `PICOJSON_USE_INT64`. I think the cleanest way to go about it is: > > > > ```c++ > > #ifndef PICOJSON_USE_INT64 > > # define PICOJSON_USE_INT64 > > #endif > > ```
We cannot distinguish whether a user did not specify that flag because of us making the header self-contained here, or due to them wanting to parameterize picojson behavior. Compiling some picojson-dependent TUs with this flag and some without will lead to hard to diagnose issues down the line (e.g., linker errors, ODR violation). I am against adding to much magic here since that would make this even more intrasparent. If your project defines this flag to link against Mesos, you do not need to set it anymore with this change (and the "breakage" is merely your compiler giving you a heads-up and nothing fatal). Note that Mesos provides a pkg-config file (`mesos.pc`), so there is no need to tightly couple your project to whatever Mesos sets. If your project defines this flag because it depends on picojson itself, please make sure you are and remain compatible with what Mesos defines for picojson. It might make sense to make sure that the different parameterizations of picojson never are visible at the same time to prevent e.g., ODR violations. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67632/#review205630 ----------------------------------------------------------- On June 26, 2018, 1:48 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67632/ > ----------------------------------------------------------- > > (Updated June 26, 2018, 1:48 p.m.) > > > Review request for mesos. > > > Repository: mesos > > > Description > ------- > > The macro PICOJSON_USE_INT64 must always be defined > when attempting to use stout's JSON facilities, since > they unconditionally depend on these features. > > Previously, we relied on the Mesos build system to set > that macro from the command line, but since we don't > generate a separate pkg-config file for stout there is > no way to convey this information to other users of stout. > > Even for users trying to use stout as part of a libmesos > installation, it might be surprising that it is required to > read CPPFLAGS if only the header-only stout part is to be > used. > > > Diffs > ----- > > 3rdparty/stout/Makefile.am 5b922af6cd58dc03c44aacf88637e89f6e289702 > 3rdparty/stout/include/stout/json.hpp > 5e738cf6f72f32b852beb61a16787e48082dab14 > > > Diff: https://reviews.apache.org/r/67632/diff/3/ > > > Testing > ------- > > Installed stout on my system and compiled an external program using stout's > JSON processing functions. > > > Thanks, > > Benno Evers > >
