-std=c++11 flag added on wrong macOS versions

[ryandesign]

python/setup.py checks the macOS version incorrectly, performing a floating point comparison instead of a version number comparison:

    v, _, _ = platform.mac_ver()
    if v:
      v = float('.'.join(v.split('.')[:2]))
      if v >= 10.12:
        extra_compile_args.append('-std=c++11')

So the -std=c++11 flag will be added for macOS versions with a floating point value greater than or equal to 10.12—e.g. 10.12, 10.13, 10.2, 10.3, 10.4, 10.5, 10.6, 10.7, 10.8, 10.9—but not for 10.10 or 10.11. Surely this isn't what was intended. The clang++ compiler in Xcode on macOS only started using libc++ (which supports C++11) as of 10.9, and newer versions of clang++ in newer versions of Xcode on newer versions of macOS support more of the standard. In 10.8 and earlier, the C++ compilers used libstdc++ 4.2.1 or earlier (which doesn't support C++11 at all).

This problem was introduced in the 3.2.x branch in 117f771 and merged to master as part of 7f3e237.

What was the intention? On which macOS versions did you mean to add the flag?

I suspect it is incorrect to add the flag on the basis of a macOS version. You should instead add the flag on the basis of compiler capability. (It is possible to use newer clang++ compilers and libc++ on older versions of macOS and thus give them C++11 capability.)

[TeBoring]

You are correct. Could you send us a PR for that?

[ryandesign]

I am not a Python programmer and don't know how to do a version comparison there. I have also never looked at protobuf code before and don't know what the fix should be. Should the flag be added on 10.12 or later, as it was written? Or should it be 10.9 or later? What will be done for the 3.6.x branch, where as I understand it C++11 will be unconditionally required?

[TeBoring]

Yeah, I think we should unconditionally add the flag now.

[ryandesign]

Please reopen. That commit only changed master. What about the 3.5.x branch which, if I understand correctly, is intended not to require C++11 yet?

[TeBoring]

Although we didn't enforce c++11 in 3.5.x, it can still work with c++11.

[TeBoring]

Could you update to our latest release?

[TeBoring]

Besides, we do you need our setup.py? Doesn't pypi install work?

[ryandesign]

To explain, I am a developer with the MacPorts package management system, where we have ports for protobuf and its python library. Investigating a build failure of that python library port in MacPorts is what led me to report this bug to you.

Although we didn't enforce c++11 in 3.5.x, it can still work with c++11.

Right. But you're adding the -std=c++11 flag in unintended circumstances, on systems where C++11 support is not available. Can that be fixed, either by correcting the logic of when to add the flag, or by removing it entirely since it is evidently not needed in 3.5.x?

Could you update to our latest release?

Now that you've released 3.6.0, sure, we'll try to update MacPorts to that version. But in the 3.6.0 release notes it says you'll try to keep the 3.5.x branch updated with critical fixes, so hopefully you can still fix this problem in the 3.5.x branch for the next 3.5.x release.

Besides, we do you need our setup.py? Doesn't pypi install work?

MacPorts runs setup.py when building python libraries. Having MacPorts run another package manager like pypi is problematic so we don't do that.

[TeBoring]

I'll fix it.

[TeBoring]

I think only commit to 3.5.x is needed, right? There is no need to push packages to pypi

[okomarov]

@TeBoring Hi, it seems that the recently added Mac wheel is still not installing the cpp extensions:

 ~ pip3 install protobuf

Collecting protobuf
  Using cached https://files.pythonhosted.org/packages/85/f8/d09e4bf21c4de65405ce053e90542e728c5b7cf296b9df36b0bf0488f534/protobuf-3.6.0-py2.py3-none-any.whl
Requirement already satisfied: six>=1.9 in /usr/local/lib/python3.7/site-packages (from protobuf) (1.11.0)
Requirement already satisfied: setuptools in /usr/local/lib/python3.7/site-packages (from protobuf) (40.0.0)
Installing collected packages: protobuf
Successfully installed protobuf-3.6.0

 ~ python -c "from google.protobuf.internal import api_implementation; print(api_implementation._default_implementation_type)"

python

and passing the --cpp_implementation flag results in an error:

 ~  pip3 install --install-option="--cpp_implementation" protobuf

/usr/local/lib/python3.7/site-packages/pip/_internal/commands/install.py:206: UserWarning: Disabling all use of wheels due to the use of --build-options / --global-options / --install-options.
  cmdoptions.check_install_build_global(options)
Collecting protobuf
  Using cached https://files.pythonhosted.org/packages/7f/51/7429b4009ccd54717d54b3a273d16df1a269845b39bcca3b4b7023a48078/protobuf-3.6.0.tar.gz
Requirement already satisfied: six>=1.9 in /usr/local/lib/python3.7/site-packages (from protobuf) (1.11.0)
Requirement already satisfied: setuptools in /usr/local/lib/python3.7/site-packages (from protobuf) (40.0.0)
Skipping bdist_wheel for protobuf, due to binaries being disabled for it.
Installing collected packages: protobuf
  Running setup.py install for protobuf ... error
    Complete output from command /usr/local/opt/python/bin/python3.7 -u -c "import setuptools, tokenize;__file__='/private/var/folders/00/gm0h8hz97mx9wz_9dy1nktkm0000gn/T/pip-install-f_t4wed5/protobuf/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /private/var/folders/00/gm0h8hz97mx9wz_9dy1nktkm0000gn/T/pip-record-8te7qlv9/install-record.txt --single-version-externally-managed --compile --cpp_implementation:
    running install
    running build
    running build_py
    creating build
    creating build/lib.macosx-10.13-x86_64-3.7
    creating build/lib.macosx-10.13-x86_64-3.7/google
    copying google/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google
    creating build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/descriptor.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/service_reflection.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/service.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/test_messages_proto3_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/json_format.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/text_format.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/text_encoding.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/proto_builder.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/wrappers_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/descriptor_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/symbol_database.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/reflection.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/field_mask_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/message_factory.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/any_test_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/message.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/empty_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/api_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/source_context_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/any_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/test_messages_proto2_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/duration_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/struct_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/descriptor_database.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/type_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/descriptor_pool.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    copying google/protobuf/timestamp_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf
    creating build/lib.macosx-10.13-x86_64-3.7/google/protobuf/util
    copying google/protobuf/util/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/util
    copying google/protobuf/util/json_format_proto3_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/util
    creating build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/type_checkers.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/well_known_types.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/wire_format.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/decoder.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/_parameterized.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/containers.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/testing_refleaks.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/api_implementation.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/encoder.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/python_message.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/message_listener.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    copying google/protobuf/internal/enum_type_wrapper.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/internal
    creating build/lib.macosx-10.13-x86_64-3.7/google/protobuf/pyext
    copying google/protobuf/pyext/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/pyext
    copying google/protobuf/pyext/python_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/pyext
    copying google/protobuf/pyext/cpp_message.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/pyext
    creating build/lib.macosx-10.13-x86_64-3.7/google/protobuf/compiler
    copying google/protobuf/compiler/plugin_pb2.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/compiler
    copying google/protobuf/compiler/__init__.py -> build/lib.macosx-10.13-x86_64-3.7/google/protobuf/compiler
    Skipping optional fixer: buffer
    Skipping optional fixer: idioms
    Skipping optional fixer: set_literal
    Skipping optional fixer: ws_comma
    running build_ext
    building 'google.protobuf.pyext._message' extension
    creating build/temp.macosx-10.13-x86_64-3.7
    creating build/temp.macosx-10.13-x86_64-3.7/google
    creating build/temp.macosx-10.13-x86_64-3.7/google/protobuf
    creating build/temp.macosx-10.13-x86_64-3.7/google/protobuf/pyext
    clang -Wno-unused-result -Wsign-compare -Wunreachable-code -fno-common -dynamic -DNDEBUG -g -fwrapv -O3 -Wall -I. -I../src -I/usr/local/include -I/usr/local/opt/openssl/include -I/usr/local/opt/sqlite/include -I/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/include/python3.7m -c google/protobuf/pyext/extension_dict.cc -o build/temp.macosx-10.13-x86_64-3.7/google/protobuf/pyext/extension_dict.o -Wno-write-strings -Wno-invalid-offsetof -Wno-sign-compare -std=c++11
    google/protobuf/pyext/extension_dict.cc:184:7: error: assigning to 'char *' from incompatible type 'const char *'
      if (PyString_AsStringAndSize(arg, &name, &name_size) < 0) {
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    google/protobuf/pyext/extension_dict.cc:56:22: note: expanded from macro 'PyString_AsStringAndSize'
           ((*(charpp) = PyUnicode_AsUTF8AndSize(ob, (sizep))) == NULL? -1: 0): \
                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1 error generated.
    error: command 'clang' failed with exit status 1

    ----------------------------------------
Command "/usr/local/opt/python/bin/python3.7 -u -c "import setuptools, tokenize;__file__='/private/var/folders/00/gm0h8hz97mx9wz_9dy1nktkm0000gn/T/pip-install-f_t4wed5/protobuf/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /private/var/folders/00/gm0h8hz97mx9wz_9dy1nktkm0000gn/T/pip-record-8te7qlv9/install-record.txt --single-version-externally-managed --compile --cpp_implementation" failed with error code 1 in /private/var/folders/00/gm0h8hz97mx9wz_9dy1nktkm0000gn/T/pip-install-f_t4wed5/protobuf/

---

So, the issue is coming from changes to the C API in Python 3.7 as described in #4086

Waiting for a fix release that includes the merged results.

[zhangskz]

This seems like it may be fixed already -- please reopen if it is not.