wbolster / plyvel

Plyvel, a fast and feature-rich Python interface to LevelDB
https://plyvel.readthedocs.io/
Other
529 stars 75 forks source link

Use -x c++ to enable C++ mode #91

Closed ryandesign closed 5 years ago

ryandesign commented 5 years ago

Use the generic flags -x c++ to enable C++ mode, instead of the Clang-specific -stdlib=libc++ and Mac-specific -mmacosx-version-min=10.7.

Fixes build failure on Mac OS X 10.6.

This reverts part of #67 and fixes it a different way.

The reason why any fix was necessary in the first place is that apparently distutils compiles all code, even C++ code, using the C compiler. This seems to be a longstanding distutils bug. The C compiler of course doesn't expect C++ code, which appears to be what caused the missing symbols mentioned in #66. (It appears that only some systems are affected. Reverting the change from #67, I see the missing symbol errors on OS X 10.8, but not on macOS 10.13. It may depend on the clang version or on which C++ standard library is used. #66 doesn't mention what version of macOS was being used there.)

The solution implemented in #67 was to enable C++ mode in the C compiler by using the flag -stdlib=libc++. And libc++ was first shipped in Mac OS X 10.7, so the flag -mmacosx-version-min=10.7 was added.

This causes build failure on Mac OS X 10.6 and earlier, because those OS versions don't use clang by default, and only clang understands -stdlib:

cc1plus: error: unrecognized command line option "-stdlib=libc++"
error: command '/usr/bin/gcc-4.2' failed with exit status 1

The plyvel code isn't doing anything that's specific to libc++ or Mac OS X 10.7 or later, and it compiles fine using libstdc++, which is the default C++ standard library on OS X 10.8 and earlier. The revised solution implemented in this PR is to enable C++ mode in the C compiler using the standard -x c++ flag instead. I have verified this compiles fine on Mac OS X 10.6 and that a test file containing import plyvel runs without showing any missing symbol errors.

wbolster commented 5 years ago

sorry for the late response @ryandesign. i totally forgot about this.

would this also fix #95?

wbolster commented 5 years ago

thanks all, see #97