> On June 18, 2018, 1:23 p.m., Andrew Schwartzmeyer wrote: > > 3rdparty/stout/Makefile.am > > Lines 96-99 (original), 96-97 (patched) > > <https://reviews.apache.org/r/67632/diff/1/?file=2042002#file2042002line96> > > > > We should probably remove this from CMake too, yeah? > > > > ``` > > 3rdparty/CMakeLists.txt > > 424: PICOJSON_USE_INT64 > > ``` > > Andrew Schwartzmeyer wrote: > That said, this part: > > > 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. > > Isn't true for CMake since it has a notion of interfaces. But still, if > we're defining it in the file itself, it's unneeded. > > Benjamin Bannier wrote: > mesos-split, https://reviews.apache.org/r/67634/ :| > > Benno Evers wrote: > As Benjamin said, the issue itself is already fixed in a subsequent > review due to `mesos-split`, so I'm dropping this. > > However, I think the `cmake` build actually has exactly the same problem, > no? Once stout is installed on the system, a program trying to use the stout > library has no way anymore to read the cmake interface defintion that was > defined in the mesos build system. (or rather, the problem will occur once > the cmake build gains an `install` target)
Gotcha. Though I don't think we'll ever be installing stout, as it's just headers + a lot of third party dependencies, it's still easier to define the flag in the source. Only problem is if _somebody else_ in Mesos starts to use PicoJSON but not through the stout header, but that's perhaps unlikely. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67632/#review204946 ----------------------------------------------------------- On June 18, 2018, 8:53 a.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67632/ > ----------------------------------------------------------- > > (Updated June 18, 2018, 8:53 a.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/2/ > > > Testing > ------- > > Installed stout on my system and compiled an external program using stout's > JSON processing functions. > > > Thanks, > > Benno Evers > >
