Set default CMAKE_BUILD_TYPE

We should be setting a default CMAKE_BUILD_TYPE (probably Release) so the user gets some sensible default. The cmake documentation also recommends not leaving this on the default empty string.

changed milestone to %v39r0

added bug build system labels

assigned to @clemenci

marked this issue as related to #217 (closed)

mentioned in issue #217 (closed)

mentioned in commit a70ff52f

mentioned in merge request !1626 (merged)

I started to look into this and it turns out a bit more complicated that expected.

A default CMAKE_BUILD_TYPE should be defined before the call to project(), but in LHCb we set CMAKE_BUILD_TYPE in the toolchain file we use to select the version of LCG and platform we want to build against, so if I set the default I cannot override it the way we use to do in LHCb.

I'll think a bit more about it, but at the moment the best I can think of is to add a check just after project() to fail the configuration if CMAKE_BUILD_TYPE is not set.

An alternative, of course, is to make build and tests work when CMAKE_BUILD_TYPE is not defined.

OK, the issue with the failing tests is slightly more convoluted than originally thought.

THistSvc contains a couple of debug messages that are compiled away in optimized builds via the C preprocessor variable NDEBUG, so optimized and debug builds have different printouts, which is handled my a mechanism of our test systems that allows to have different versions of the reference stdout for different platforms. This mechanism rely on the environment variable BINARY_TAG which is set consistently with the CMAKE_BUILD_TYPE, but only in LHCb builds.

In this scenario, from the point of view of the tests, an empty BINARY_TAG is equivalent to release builds, while an empty CMAKE_BUILD_TYPE is closer to a debug build (no optimizations, no NDEBUG, but no debug symbols).

I can modify the tests (or the configuration) such that an empty CMAKE_BUILD_TYPE makes the test choose the debug version of the reference file... or I can make THistSvc produce the same output in both optimized and debug builds (my preferred option)... or I can force the user to set CMAKE_BUILD_TYPE as suggested before, but this is not enough because the tests will work only when CMAKE_BUILD_TYPE=Release unless you set BINARY_TAG too.

@fwinkl what do you prefer? Remove NDEBUG from THistSvc or change the tests so that they work for all values of CMAKE_BUILD_TYPE even if BINARY_TAG is not set?

We probably want to keep the ability to have separate references for opt/dbg builds. So I would go with the solution of making the test choose the correct version even if CMAKE_BUILD_TYPE is not set.

mentioned in commit 10a4ca28

closed with merge request !1626 (merged)

mentioned in commit 03984d2e