multiscale / muscle3

The third major version of the MUltiScale Coupling Library and Environment
Apache License 2.0
25 stars 13 forks source link

Inconsistent use of googletest in libmuscle and msgpack #199

Closed peter-t-fox closed 11 months ago

peter-t-fox commented 1 year ago

I am trying to build MUSCLE3 and have run into a problem with its use of googletest and msgpack.

I am building on a cluster where there is an old version of googletest installed under /lib64. When I build libmuscle, it does not find the googletest installation because it is too old and does not provide a pkg-config configuration. And so I get this:

Checking for googletest >= 1.8.1...
- googletest_ROOT set to 
- googletest not found on the system.
- Will build googletest automatically.

This is fine, as I don't want to build the tests. Moreover, since I am not building the libmuscle tests target, googletest is never downloaded anyway. So far so good.

However, when the build of libmuscle triggers the build of the msgpack dependency, the build of msgpack finds the old version of googletest (via the CMake FindGTest facility). The build of msgpack then fails because googletest is too old, so some of the symbols it needs are out of date. This kills the entire libmuscle build.

I can work around this problem by setting -DMSGPACK_BUILD_TESTS=OFF in the CMake command in the msgpack target here:

msgpack: msgpack-$(msgpack_VERSION)
    cd msgpack-$(msgpack_VERSION) && mkdir -p build && cd build && \
        export PKG_CONFIG_PATH=$(PKG_CONFIG_PATH):$(PKG_CONFIG_EXTRA_DIRS) && \
        cmake -DMSGPACK_BUILD_TESTS=OFF -DCMAKE_INSTALL_PREFIX=$(CURDIR)/msgpack -DMSGPACK_CXX11=ON -DMSGPACK_BUILD_EXAMPLES=OFF .. && \
        $(MAKE) -j $(NCORES) && make install

However, this obviously requires me to manually edit the MUSCLE3 build sources and maintain that edit in the future, which is not ideal. The alternative is that I separately download and build msgpack on my own system, so that I can control whether or not its builds its unit tests. This is probably going to have to be my approach.

So I have the question: should MUSCLE3 build the msgpack unit tests at all?

If yes, maybe it would make sense to arrange the build so that if googletest is not found by MUSCLE3, it is downloaded (even for a non-tests build) and that downloaded version of googletest is also used by msgpack, so that everything is consistent?

If no (because I guess most/all people using MUSCLE3 are not interested in the msgpack unit tests), maybe it would make sense to disable the unit tests as I've done above, and so avoid this kind of problem entirely.

(Arguably this is really a msgpack problem, as it should maybe download the version of googletest that it needs, rather than relying on the system to provide it.)

LourensVeen commented 1 year ago

Hi Peter,

Well, that's not a good user experience for sure, thank you for reporting it!

Yes, it is kind of a msgpack problem, but I've decided that I'm responsible for my dependencies, so it is also a MUSCLE3 problem. I think the best solution is to just skip building the msgpack tests, since we're trying to use it, not test it.

I'll add this fix in the next release, thank you!

LourensVeen commented 1 year ago

The fix for this was included in 0.7.0, so it should work out of the box now. If that fixes it, could you close this issue?

LourensVeen commented 1 year ago

@peter-t-fox, since you probably didn't get notified of this one either. There's no hurry, whenever is convenient for you is fine.

peter-t-fox commented 11 months ago

@LourensVeen Apologies for the delay, I don't seem to get any notifications at all. We are now using MUSCLE3 v0.7.0 and the build works perfectly without any hacking of the msgpack build. Thanks for resolving this!