Skip to content

Commit d6d8f03

Browse files
authored
GH-50774: [CI][Python] Match Protobuf symbol visibility in bundled Substrait and ORC (#50792)
### Rationale for this change Fix #50774. After #50650, the Pyodide build failed while loading PyArrow with ` ImportError: dynamic module does not define module export function (PyInit_lib)`. The ImportError is misleading because the `PyInit_lib` is present. The actual failure is an earlier abort during static initialization of `libarrow_python.so`, in Protobuf descriptor registration. #50650 changed the order of libraries merged into `libarrow_bundled_dependencies.a`. The archive contains duplicate weak Protobuf/Abseil symbols emitted by the Protobuf runtime and generated code in Substrait and in ORC. These symbols had inconsistent visibility: a) Protobuf was compiled with *hidden* visibility, b) Substrait and ORC generated Protobuf code used default visibility, so the order change caused `wasm-ld` to select symbols with different visibility. (e.g. `google::protobuf::internal::ZeroFieldsBase::Clear()` changed from hidden in Protobuf's `empty.pb.cc.o` to default visible in Substrait's `algebra.pb.cc.o`) The failure showed up in Emscripten, no failures on other platforms, but matching visibility globally is desirable to prevent any unintended exports and symbol interposition. ### What changes are included in this PR? Compile bundled Substrait and ORC with the same hidden visibility for symbols as Protobuf so not depending on archive order. Also add debug for Emscripten CI builds `--profiling-funcs`. ### Are these changes tested? Yes, `test-conda-python-emscripten` passes (Local repro, debug and fix verification with `ARCH=amd64 archery docker run conda-python-emscripten`, for symbols `llvm-nm` and `wasm-ld`) ### Are there any user-facing changes? No. * GitHub Issue: #50774 Authored-by: Tadeja Kadunc <tadeja.kadunc@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent 4049d6e commit d6d8f03

2 files changed

Lines changed: 16 additions & 1 deletion

File tree

ci/scripts/python_build_emscripten.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ cp -aL "${source_dir}" "${python_build_dir}"
3838
# emcmake so we unset them
3939
unset LDFLAGS CFLAGS CXXFLAGS CPPFLAGS
4040

41+
# Keep WebAssembly function names only in CI to limit wheel size
42+
if [ "${GITHUB_ACTIONS:-}" = "true" ]; then
43+
export PYARROW_CXXFLAGS="${PYARROW_CXXFLAGS:+${PYARROW_CXXFLAGS} }--profiling-funcs"
44+
fi
45+
4146
pushd "${python_build_dir}"
4247
pyodide build
4348
popd

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2447,7 +2447,11 @@ macro(build_substrait)
24472447
set(SUBSTRAIT_INCLUDES ${SUBSTRAIT_CPP_DIR} ${PROTOBUF_INCLUDE_DIR})
24482448

24492449
add_library(substrait STATIC ${SUBSTRAIT_SOURCES})
2450-
set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON)
2450+
# Match Protobuf's visibility because target contains generated Protobuf code
2451+
set_target_properties(substrait
2452+
PROPERTIES POSITION_INDEPENDENT_CODE ON
2453+
CXX_VISIBILITY_PRESET hidden
2454+
VISIBILITY_INLINES_HIDDEN ON)
24512455
target_compile_options(substrait PRIVATE "${SUBSTRAIT_SUPPRESSED_FLAGS}")
24522456
target_include_directories(substrait PUBLIC ${SUBSTRAIT_INCLUDES})
24532457
target_link_libraries(substrait PUBLIC ${ARROW_PROTOBUF_LIBPROTOBUF})
@@ -3888,6 +3892,10 @@ function(build_orc)
38883892

38893893
fetchcontent_makeavailable(orc)
38903894

3895+
# ORC compiles generated Protobuf code into its static library
3896+
set_target_properties(orc PROPERTIES CXX_VISIBILITY_PRESET hidden
3897+
VISIBILITY_INLINES_HIDDEN ON)
3898+
38913899
# ORC 2.2.1 unconditionally adds /std:c++17 on MSVC via
38923900
# add_compile_options, which overrides CMAKE_CXX_STANDARD and causes
38933901
# ABI mismatches with protobuf (GlobalEmptyStringConstexpr vs
@@ -3955,7 +3963,9 @@ function(build_orc)
39553963
set(ORC_CMAKE_ARGS
39563964
${EP_COMMON_CMAKE_ARGS}
39573965
"-DCMAKE_CXX_FLAGS=${ORC_CXX_FLAGS}"
3966+
-DCMAKE_CXX_VISIBILITY_PRESET=hidden
39583967
"-DCMAKE_INSTALL_PREFIX=${ORC_PREFIX}"
3968+
-DCMAKE_VISIBILITY_INLINES_HIDDEN=ON
39593969
-DSTOP_BUILD_ON_WARNING=OFF
39603970
-DBUILD_LIBHDFSPP=OFF
39613971
-DBUILD_JAVA=OFF

0 commit comments

Comments
 (0)