-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
Conflicting PR. Removed from build OME-FILES-CPP-DEV-merge-push-superbuild#729. See the console output for more details.
|
Conflicting PR. Removed from build OME-FILES-CPP-DEV-merge-push-superbuild#730. See the console output for more details.
|
Conflicting PR. Removed from build OME-FILES-CPP-DEV-merge-push-superbuild#731. See the console output for more details.
|
In general, 👍 for keeping up with the upstream boosts releases. Can you comment on the rationale for bumping the minimal Cmake requirements as this I cannot see it either in the commit or in the PR description. To the extent possible, any discussion on minimal requirements should probably be widened to include the ongoing investigation of the C7 platform support since the default version of Cmake is already a blocker. In the case of this PR, can we make |
The rationale is in the Trello card. The FindBoost script is using Boost 3.3 and 3.4 features. For the last few Boost updates, I've manually kept the version requirement at 3.2 by rewriting various parts of the script. However, as the use of new features has increased over the last few releases, this has become increasingly burdensome and error-prone, and is not something we can continue to do indefinitely. The version bump has been kept as small as possible, at 2 minor versions. Regarding CentOS 7, it only provides CMake 2.8.x and we have always required CMake 3.x on this platform. The version installed on our CI infrastructure is already meeting the updated requirement (3.7.1). Regarding fallbacks, that would be difficult. The features we are concerned with are fairly basic things such as string append operations and searching in lists which are present in many places throughout the script. |
CMake script changes all seem fine, nothing complex going on there. @rleigh-codelibre has identified a breaking bug in the new boost 1.65 libraries occurring on our CI node brill (FreeBSD) and submitted an issue with the Boost team. |
https://gitlab.kitware.com/cmake/cmake/merge_requests/1172 merged upstream |
Regarding the build failure, we could drop the boost update and leave this PR updating FindBoost only. Or I can add a patch to fix the issue. |
Note I've dropped the upgrade of boost itself to leave this restricted to the FindBoost changes only, as a result of the compile failure on brill. We can update this separately once we have a working patch (I tried the existing patch, but it broke cowfish), so some further work required. It has served its purpose in demonstrating that the FindBoost changes work on all the other platforms however. |
All CI builds passing now! Looks like it has been sorted. |
As mentioned elsewhere, my primary concern was the growing increase of the gap between our minimal CMake requirement and the version shipped in CentOS7 which is arguably our most conservative platform. This is until @joshmoore pointed me to the
|
Routine update of the Boost support for the new Boost 1.65 release. Trello card
All other components copy the last two changes.
Testing: Check builds remain green.