Is the boost dependency check in CMakeLists.txt correct?

[Grimler91]

Hi, I am trying to build openscad but run into an error during configuring:

-- Android: Targeting API '24' with architecture 'arm64', ABI 'arm64-v8a', and processor 'aarch64'
-- The C compiler identification is Clang 8.0.7
-- The CXX compiler identification is Clang 8.0.7
-- Check for working C compiler: /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/clang
-- Check for working C compiler: /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/clang -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/clang++
-- Check for working CXX compiler: /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/clang++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/aarch64-linux-android-pkg-config (found version "1.6.3") 
-- Using header-only CGAL
-- Targetting Ninja
-- Using /home/grimler/.termux-build/_cache/android-r20-api-24-v3/bin/clang++ compiler.
-- Found GMP: /data/data/com.termux/files/usr/lib/libgmp.so  
-- Found MPFR: /data/data/com.termux/files/usr/lib/libmpfr.so  
-- Found Boost: /data/data/com.termux/files/usr/lib/cmake/Boost-1.72.0/BoostConfig.cmake (found suitable version "1.72.0", minimum required is "1.48")  
-- Boost include dirs: /data/data/com.termux/files/usr/include
-- Boost libraries:    
-- CGAL: 5.00
-- CGAL: Using target CGAL::CGAL
-- Eigen3: 3.3.7
-- Boost: 1.72.0
CMake Error at CMakeLists.txt:206 (message):
  No dependent Boost libraries found.  Your CMake (3.16.2) version might be
  too old for the version of boost you are using (1.72.0).  In that case, you
  should have received warnings above of the type: 'Imported targets not
  available for Boost version 1.72.0'


-- Configuring incomplete, errors occurred!
See also "/home/grimler/.termux-build/openscad/build/CMakeFiles/CMakeOutput.log".

This comes from line 205 of the CMakeLists.txt, where we check, I think, if the number of required boost libraries equals the number of boost libraries that we have. (The same check is also done on line 148 for MSVC)

It checks if(Boost_LIBRARIES_LENGTH EQUAL BOOST_DIRECTLY_REQUIRED_LIBRARIES_LENGTH), which in my case expands to 5 EQUALS 5.
Shouldn't this rather be if(NOT Boost_LIBRARIES_LENGTH EQUAL BOOST_DIRECTLY_REQUIRED_LIBRARIES_LENGTH)?
The arrays, BOOST_DIRECTLY_REQUIRED_LIBRARIES and Boost_LIBRARIES, checked expand to filesystem;system;thread;regex;program_options and Boost::filesystem;Boost::system;Boost::thread;Boost::regex;Boost::program_options, so it looks like all dependencies are there, if I comment this check away openscad seems to build fine.

Maybe I am misunderstanding the intent behind the check..

[t-paul]

Note that cmake is not the official build system for the application just yet.

[kintel]

That boost check looks a bit weird. @thehans Looks like smth. you added to work around MSVC issues and possibly some CGAL issues. Perhaps that can be cleaned up now that CGAL is header-only?

[thehans]

I agree it does look weird, but as far as I remember, I don't think I originally added that check, just carried it forward through various large changes.

Its not specific to MSVC; it does the same check in both the if (MSVC) and the else() section.
I also haven't tried MSVC builds in a while, but the idea is for the boost libraries which OpenSCAD directly requires, those have their own dependencies which should get picked up by the find_package line.

Perhaps a more clear representation of the logic would be NOT (Boost_LIBRARIES_LENGTH GREATER BOOST_DIRECTLY_REQUIRED_LIBRARIES_LENGTH), instead of EQUALS.

Here is an example of what I see when I print out BOOST_DIRECTLY_REQUIRED_LIBRARIES and Boost_LIBRARIES variables in a message on my Linux machine (after some manual formatting to separate lines):

Directly required:

1. filesystem
2. system
3. thread
4. regex
5. program_options

Boost Libraries:

1. /usr/lib/x86_64-linux-gnu/libboost_filesystem.so
2. /usr/lib/x86_64-linux-gnu/libboost_system.so
3. /usr/lib/x86_64-linux-gnu/libboost_thread.so
4. /usr/lib/x86_64-linux-gnu/libboost_regex.so
5. /usr/lib/x86_64-linux-gnu/libboost_program_options.so
6. /usr/lib/x86_64-linux-gnu/libboost_chrono.so
7. /usr/lib/x86_64-linux-gnu/libboost_date_time.so
8. /usr/lib/x86_64-linux-gnu/libboost_atomic.so
9. /usr/lib/x86_64-linux-gnu/libpthread.so

Also you might be able to see some extra info by removing the QUIET arg from the corresponding find_package line. I think that's another sort of "cargo cult" style I carried forward that probably would be better to not silence in cases like this.

[thehans]

BTW, here is the original commit by @hoijui 1a1b68d

It only looks like I added that since I had to copy it into both of those large sections of mostly-same cmake code due to some differences in library representation on MSVC.

[Grimler91]

Note that cmake is not the official build system for the application just yet.

Thanks, I have somehow not realized this.
Nevertheless, I have managed to build openscad before for this platform with cmake, something with the build setup (boost version?) has probably changed since then. I had to apply a pretty weird patch where I "manually" linked against boost though.

Anyways, I'll try to investigate if libboost_chrono.so, libboost_date_time.so and libboost_atomic.so are truly necessary and truly missing (libpthread.so doesn't exist on this platform). If that is the case then the check is okay I guess (but could maybe be written more intuitive)

[t-paul]

If you have suggestions how to improve/modernize the cmake build, that would be much appreciated. I think we need to setup a CI build for that as soon as possible so it does not keep breaking (and @thehans fixing it). The goal is certainly to move to cmake eventually, I guess we should be ok to move on as all supported platforms hopefully now support a reasonable version of cmake.

Sounds like a nice first use case for the github CI :-). I'll try to find some time to get that setup.

[thehans]

I recently noticed that boost thread doesn't appear to be used as far as I can tell.
I tried removing it from BOOST_DIRECTLY_REQUIRED_LIBRARIES, which caused this additional dependency check to fail for me.
So I guess those extra dependent libraries were all due to boost thread.

If we truly don't need that on any platform anymore (there is std::thread in c++11 after all, and even so we're only using QThread as far as I can tell), then its probably best to get rid of this weird hack once and for all.
In that case there are also a few special checks for boost thread in various qmake files which should be taken care of too.

[t-paul]

@thehans I've merged your change fa2917c, so I think we can close this?

[thehans]

Thanks, agreed.