[setuptools] [distutils] Python builds failed on master

[lidizheng]

https://fusion.corp.google.com/projectanalysis/summary/KOKORO/prod%3Agrpc%2Fcore%2Fmaster%2Flinux%2Fgrpc_basictests_python

https://source.cloud.google.com/results/invocations/baba7e4f-6421-4b0c-8cb7-b106b252191d/log

https://fusion.corp.google.com/projectanalysis/summary/KOKORO/prod%3Agrpc%2Fcore%2Fmaster%2Fwindows%2Fgrpc_basictests_python

https://source.cloud.google.com/results/invocations/9cd6212f-643a-4c47-9ef6-76b482a60932/targets/github%2Fgrpc%2Ftoplevel_run_tests_invocations%2Frun_tests_python_windows_opt_native/tests

It started failing yesterday, when setuptools releases 50.0.0. It's highly likely that their latest build broke us.

CC @jtattermusch @gnossen

[gnossen]

Interesting. Whatever changed in setuptools, it's causing the __wrap_memcpy hack to fail:

    /usr/include/unistd.h:366:16: note: code may be misoptimized unless -fno-strict-aliasing is used
    /tmp/ccdqoMwn.ltrans1.ltrans.o: In function `grpc_core::(anonymous namespace)::XdsRoutingLb::RoutePicker::Pick(grpc_core::LoadBalancingPolicy::PickArgs) [clone .lto_priv.4464]':
    <artificial>:(.text+0x389e): undefined reference to `__wrap_memcpy'
    <artificial>:(.text+0x38c8): undefined reference to `__wrap_memcpy'
    <artificial>:(.text+0x39a5): undefined reference to `__wrap_memcpy'
    /tmp/ccdqoMwn.ltrans3.ltrans.o: In function `http_client_start_transport_stream_op_batch(grpc_call_element*, grpc_transport_stream_op_batch*)':
    <artificial>:(.text+0x17034): undefined reference to `__wrap_memcpy'
    <artificial>:(.text+0x170bd): undefined reference to `__wrap_memcpy'
    /tmp/ccdqoMwn.ltrans3.ltrans.o:<artificial>:(.text+0x1742a): more undefined references to `__wrap_memcpy' follow
    collect2: error: ld returned 1 exit status

[gnossen]

Potentially related: pypa/setuptools#2232

[lidizheng]

The discussion seems mainly about the install location. The compilation failure you mentioned is a behavior change in compiler. So, I guess there are still some undiscovered issue.

[gnossen]

It's about whether we're pulling in setuptools' vendored distutils or Debian's vendored (and patched) distutils. Prior to version 50, we were getting Debian's, whereas now we're getting setuptool's packaged version. Since distutils controls the exact compiler invocation, I think it's likely that difference is significant.

I don't have full compiler invocations handy from before setuptools 50, but the object that fails to link appears to be an intermediary target used in link-time optimization. I wasn't aware that we were using LTO before, so it's possible that it's a feature of the packaged distutils version that we didn't have before.

[lidizheng]

Mentioned in https://github.com/pypa/setuptools/issues/2350 that setting environment variable SETUPTOOLS_USE_DISTUTILS=stdlib fixes this problem. TIL that Debian and Ubuntu has patched distutils. Let's see how setuptools owners decided to fix this issue.

[lidizheng]

Reopen since the we still need to plan for how to adapt to the new distutils changes introduced by setuptools 50.0.0+.

[gnossen]

I was expecting that maybe -Wl,-wrap,memcpy didn't get propagated from setup.py down to the compiler invocation, but it's in there:

x86_64-linux-gnu-g++ -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions \
  -Wl,-z,relro -I/var/local/git/grpc/include -std=gnu99 -fno-wrapv \
  python_build/temp.linux-x86_64-3.5/src/core/ext/filters/census/grpc_context.o ... \
  -L/usr/lib -lrt -lm -o python_build/lib.linux-x86_64-3.5/grpc/_cython/cygrpc.cpython-35m-x86_64-linux-gnu.so \
  -lpthread -Wl,-wrap,memcpy -static-libgcc

So far, the only difference I've found is the use of LTO.

[gnossen]

Comparison of compilation of single translation unit:

My machine:

x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare \
  -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat \
  -Werror=format-security -g -fwrapv -O2 -fPIC -D_WIN32_WINNT=1536 \
  -DGPR_BACKWARDS_COMPATIBILITY_MODE=1 -DHAVE_CONFIG_H=1 \
  -DGRPC_ENABLE_FORK_SUPPORT=1 \
  -DPyMODINIT_FUNC=extern "C" __attribute__((visibility ("default"))) PyObject* \
  -DGRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK=1 -Isrc/python/grpcio \
  -Iinclude -I. -Ithird_party/abseil-cpp -Ithird_party/address_sorting/include -Ithird_party/cares \
  ... \
  -I/usr/local/google/home/rbellevi/Dev/grpc/venv3.7/include/python3.7m -c src/core/ext/filters/census/grpc_context.cc \
  -o python_build/temp.linux-x86_64-3.7/src/core/ext/filters/census/grpc_context.o -std=c++11 \
  -std=gnu99 -fvisibility=hidden -fno-wrapv -fno-exceptions -pthread

CI:

ccache gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv \
  -O2 -Wall -Wstrict-prototypes -g -fdebug-prefix-map=/build/python3.5-3.5.3=. \
  -fstack-protector-strong -Wformat -Werror=format-security -g \
  -flto -fuse-linker-plugin -ffat-lto-objects -I/var/local/git/grpc/include -std=gnu99 \
  -fno-wrapv -fPIC -D_WIN32_WINNT=1536 -DGPR_BACKWARDS_COMPATIBILITY_MODE=1 \
  -DHAVE_CONFIG_H=1 -DGRPC_ENABLE_FORK_SUPPORT=1 \
  -DPyMODINIT_FUNC=extern "C" __attribute__((visibility ("default"))) PyObject* \
  -DGRPC_POSIX_FORK_ALLOW_PTHREAD_ATFORK=1 -Isrc/python/grpcio -Iinclude -I. \
  -Ithird_party/abseil-cpp -Ithird_party/address_sorting/include -Ithird_party/cares \
  ... \
  -I/var/local/git/grpc/py35_native/include/python3.5m -c src/core/ext/filters/census/grpc_context.cc \
  -o python_build/temp.linux-x86_64-3.5/src/core/ext/filters/census/grpc_context.o \
  -std=c++11 -std=gnu99 -fvisibility=hidden -fno-wrapv -fno-exceptions -pthread

[veblush]

It's worth mentioning the PR removing wrap-memory hack. This would be helpful to stay with setuptool 50 in future. Note that it sill has another problem with Windows Python, TypeError: _commandfile_spawn() got an unexpected keyword argument 'env'. (test)

[lidizheng]

I saw the Windows issue. It could be introduced by the distutils behavior change. Pinning to older version solved it.

[gnossen]

Using export SETUPTOOLS_USE_DISTUTILS=stdlib fixes the issue on all platforms, meaning that pypa/setuptools#2232 is indeed the root cause. Unfortunately, this doesn't do a whole lot for us. We could potentially just add os.environ["SETUPTOOLS_USE_DISTUTILS"] = "stdlib" to setup.py before we import setuptools. That would solve the case where people invoke us as a subprocess. But if someone uses us as a library, then the import statement might be a no-op and we would end up using the system library anyway. But I'm not sure if that's a case that's worth taking into consideration. Does anyone ever try to import our setup.py file?

It's also worth noting that the maintainer of setuptools called this environment variable "temporary." We may need to track down the underlying issues, but it seems like this is a problem for distro maintainers and the setuptools maintainers, not for us.

[lidizheng]

Good finding! I would doubt anyone trying to import our setup.py. In the most extreme definition, the compilation of our library is part of our API contract. But we already doing a bunch of hacky things, like monkey patching distutils, patching multiple compilers. At this moment, I prefer to use the pin-back solution, since the environment variable is marked as "temporary".

---

Let's wait for how setuptools owners reply to https://github.com/pypa/setuptools/issues/2350 and pypa/setuptools#2232

[gnossen]

I don't think pinning back setuptools is sufficient. Right now, if someone on Debian or Ubuntu does pip install grpcio, it won't work.

Just tested the debian:latest Docker image with setuptools 50.0.0. It worked with no modifications. A Debian user would have to explicitly update their setuptools to hit this problem, so I agree that pinning back in CI should be sufficient for now. If we receive any user issues about this, we can just recommend the SETUPTOOLS_USE_DISTUTILS environment variable.

[lidizheng]

This plan sounds good. Closing this issue unless there is a need for another patch.

[chongkong]

I think the root cause is that distutils is imported before setuptools in commands.py, which breaks the monkeypatching behavior of setuptools. (setuptools monkeypatches some distutils methods thus should be imported before distutils). Setting SETUPTOOLS_USE_DISTUTILS=stdlib is doing the opposite (not monkey-patching the distutils explicitly), which is a legacy behavior and discouraged, only provided for the ease of transition.

[xerdink]

I have set SETUPTOOLS_USE_DISTUTILS=stdlib but still build fails in setuptools==50.0

Now I'm downgrading setuptools.

[gnossen]

@chongkong Thanks for the suggestion! Giving that a shot here.