dropbox / dbx_build_tools

Dropbox's Bazel rules and tools
Other
209 stars 37 forks source link

Rationale for not propagating -shared #24

Open jsharpe opened 3 years ago

jsharpe commented 3 years ago

https://github.com/dropbox/dbx_build_tools/blob/878fa997e7313c52692fd8d308660786a3442e59/build_tools/py/py.bzl#L136

Was wondering why '-shared is removed from the list of flags that is propagated to vpip? I've found that for at least one package with a binary component that -shared needs to be propagated for proper compilation.

benjaminp commented 3 years ago

We explicitly set it in our linker wrapper: https://github.com/dropbox/dbx_build_tools/blob/878fa997e7313c52692fd8d308660786a3442e59/build_tools/py/ldshared-wrapper.sh#L54

jsharpe commented 3 years ago

Ah ok, I think this is a specific issue to the mpi4py package then as it doesn't use the LD wrapper passed by vpip but instead the mpicc compiler wrapper alongside the ld flags passed and so the missing -shared from flags is an issue for that package.

benjaminp commented 3 years ago

The principle here is that we always build shared modules for Python extensions. So, it makes sense that we ignore whatever Bazel's C++ rules think the linking configuration is.

I'm confused why this would break mpi4py, though, since presumably it doesn't rely on -shared being passed in when built outside of Bazel.

jsharpe commented 3 years ago

I haven't checked exactly this but I suspect because the LD flags are being overridden in the environment then its not automatically adding -shared as it assumes you've already fully specified the link options for a shared library?

Surely though it doesn't hurt if -shared is passed twice to the linker, so why does it need to be filtered out? I've patched my version to do this and it seems to work for the limited set of libraries that I'm building.

benjaminp commented 3 years ago

It seems like the problem with -shared in default ldflags is when packages expect to build executables as part of their setup.py. For example, https://pypi.org/project/vmprof runs ./configure from its setup.py.