cryptominisat fails to build with clang on macOS

[iMichka]

Hi. I am a maintainer of the homebrew package manger. We noticed that cryptominisat fails to build with newest clang on macOS. See https://github.com/Homebrew/homebrew-core/pull/91224#issuecomment-1015867222 for a full log.

2022-01-18T19:23:53.7944560Z /opt/homebrew/Library/Homebrew/shims/mac/super/clang -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -DLIBRARY_VERSION=\"5.8.0\" -I/opt/homebrew/opt/[email protected]/Frameworks/Python.framework/Versions/3.9/include/python3.9 -c /tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/pycryptosat/src/pycryptosat.cpp -o build/temp.macosx-12-arm64-3.9/tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/pycryptosat/src/pycryptosat.o -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX12.sdk -I/tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0 -I/tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/cmsat5-src
2022-01-18T19:23:53.7945990Z In file included from /tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/pycryptosat/src/pycryptosat.cpp:32:
2022-01-18T19:23:53.7946550Z In file included from /tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/cmsat5-src/cryptominisat5/cryptominisat.h:35:
2022-01-18T19:23:53.7947160Z /tmp/cryptominisat-20220118-51428-1paierq/cryptominisat-5.8.0/build/cmsat5-src/cryptominisat5/solvertypesmini.h:33:1: error: unknown type name 'constexpr'

Adding -std=gnu++11 should solve the issue. But we could not add it to the CFLAGS, as this will break other stuff elsewhere (CFLAGS or for C code, and this part of the code is c++). Ideally there should be a separate set of env flags for the C++ code. We had a look at the PY_C_CONFIG env variable, but that does not seem to help to inject -std=gnu++11.

python/setup.py.in does contain some reference to -std=gnu++11, but it's not being added. We hardcoded "cconf + ['-std=gnu++11'] +" in setup.py directly and this fixed the build. But a better fix is probably doable.

See https://github.com/Homebrew/homebrew-core/pull/91224#issuecomment-1022649668 and https://github.com/Homebrew/homebrew-core/pull/91224#issuecomment-1039618378 for reference.

[msoos]

Wow, first of all, thank you for packaging it! I'm super-grateful, I struggle to do that. And thank you for that feedback, that's very valuable!

The constexpr was created to fix some issues that caused headaches when using CryptoMiniSat as a library -- we polluted the namespace with lots of #define-s, which we no longer have to do, thankfully. I'm sorry it broke the Python module build :(

I find the Python build to be complicated to do, it assumes a number of things. In this case, it assumes that the flags to the compiler, which it thinks must be the same that was given to build Python itself :S I'll look into this in the coming days. Sorry again I made your work harder. I promise I'll do better!

Mate

[msoos]

Hi,

Let me bump this issue a bit :) First, I added set(CMAKE_CXX_STANDARD 17) so that should take care of it being compiled with C++17. Second, I added a PyPi package, here: https://pypi.org/project/pycryptosat/ that should install out-of-the-box. Do you think this makes sense? Does this help? Can you please check how this affects the packaging? Let me know if there is something I can do to help! Thanks in advance,

Mate

[AntoineHus]

Hello @msoos,

Putting a Pypi package would be a good idea.

I would be able to help for the testing.

Let me know.

Thanks.

[msoos]

Hey,

I actually put together a PyPi package some time ago, with all the bells and whistles -- it even auto-builds and auto-uploads to PyPi when there is a new version :) Check out the automation here: https://github.com/msoos/cryptominisat/actions/workflows/python-package-build.yml Cool, huh?

However... there are not too many (or any?) test cases in that python package build. Maybe you could add a few? I am not a pro at adding tests in Python, I think there are some well-known patterns to do them. Perhaps if you are a pro at it, you could have a go? It may be a lot faster for you? Just a thought :)

Let me know what you think and thanks in advance for the offer,

Mate

[msoos]

Hey @iMichka does the python package help? Did that help resolve your issue(s)? Please do let me know,

Mate

[iMichka]

I think on our side everything is good; we are using pip install instead of calling setup.py ourselves now. And the workaround has been removed by another maintainer, so it looks like everything builds fine now. We can close this issue.

[msoos]

Yaaaay! :partying_face: Glad to hear!

Thanks for getting back to me, on a Sunday of all days :smiley:

Mate