Skip to content

Commit

Permalink
Replace conda with pyenv to fix incorrect libstdc++ use in jammy CI. (#…
Browse files Browse the repository at this point in the history
…6966)

* Replace conda with pyenv to fix incorrect libstdc++ use in jammy CI.
* Use RPATH instead of RUNPATH to load libc++abi.so directly in Python. No need to find and load explicitly.
* Use libc++11 to build in Ubuntu 22.04. Warn if newer version is used.
* TODO: Solution for Ubuntu 24.04
  • Loading branch information
ssheorey authored Sep 30, 2024
1 parent f4e1fa9 commit e88c7b1
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 46 deletions.
9 changes: 9 additions & 0 deletions 3rdparty/find_dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,7 @@ if(BUILD_GUI)
if (CPP_LIBRARY AND CPPABI_LIBRARY)
set(CLANG_LIBDIR ${llvm_lib_dir})
message(STATUS "CLANG_LIBDIR found in ubuntu-default: ${CLANG_LIBDIR}")
set(LIBCPP_VERSION ${llvm_ver})
break()
endif()
endforeach()
Expand All @@ -1362,7 +1363,10 @@ if(BUILD_GUI)
llvm-8/lib
llvm-7/lib
)
file(REAL_PATH ${CPPABI_LIBRARY} CPPABI_LIBRARY)
get_filename_component(CLANG_LIBDIR ${CPPABI_LIBRARY} DIRECTORY)
string(REGEX MATCH "llvm-([0-9]+)/lib" _ ${CLANG_LIBDIR})
set(LIBCPP_VERSION ${CMAKE_MATCH_1})
endif()

# Find clang libraries at the exact path ${CLANG_LIBDIR}.
Expand All @@ -1378,6 +1382,11 @@ if(BUILD_GUI)
target_link_libraries(3rdparty_filament INTERFACE -lstdc++
${CPP_LIBRARY} ${CPPABI_LIBRARY})
message(STATUS "Filament C++ libraries: ${CPP_LIBRARY} ${CPPABI_LIBRARY}")
if (LIBCPP_VERSION GREATER 11)
message(WARNING "libc++ (LLVM) version ${LIBCPP_VERSION} > 11 includes libunwind that "
"interferes with the system libunwind.so.8 and may crash Python code when exceptions "
"are used. Please consider using libc++ (LLVM) v11.")
endif()
endif()
if (APPLE)
find_library(CORE_VIDEO CoreVideo)
Expand Down
25 changes: 6 additions & 19 deletions cpp/open3d/core/nns/NanoFlannImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,6 @@ void _KnnSearchCPU(NanoFlannIndexHolderBase *holder,
return;
}

auto points_equal = [](const T *const p1, const T *const p2,
size_t dimension) {
std::vector<T> p1_vec(p1, p1 + dimension);
std::vector<T> p2_vec(p2, p2 + dimension);
return p1_vec == p2_vec;
};

std::vector<std::vector<TIndex>> neighbors_indices(num_queries);
std::vector<std::vector<T>> neighbors_distances(num_queries);
std::vector<uint32_t> neighbors_count(num_queries, 0);
Expand All @@ -147,8 +140,9 @@ void _KnnSearchCPU(NanoFlannIndexHolderBase *holder,
for (size_t valid_i = 0; valid_i < num_valid; ++valid_i) {
TIndex idx = result_indices[valid_i];
if (ignore_query_point &&
points_equal(&queries[i * dimension],
&points[idx * dimension], dimension)) {
std::equal(&queries[i * dimension],
&queries[i * dimension] + dimension,
&points[idx * dimension])) {
continue;
}
neighbors_indices[i].push_back(idx);
Expand Down Expand Up @@ -222,13 +216,6 @@ void _RadiusSearchCPU(NanoFlannIndexHolderBase *holder,
return;
}

auto points_equal = [](const T *const p1, const T *const p2,
size_t dimension) {
std::vector<T> p1_vec(p1, p1 + dimension);
std::vector<T> p2_vec(p2, p2 + dimension);
return p1_vec == p2_vec;
};

std::vector<std::vector<TIndex>> neighbors_indices(num_queries);
std::vector<std::vector<T>> neighbors_distances(num_queries);
std::vector<uint32_t> neighbors_count(num_queries, 0);
Expand All @@ -255,9 +242,9 @@ void _RadiusSearchCPU(NanoFlannIndexHolderBase *holder,
int num_neighbors = 0;
for (const auto &idx_dist : search_result) {
if (ignore_query_point &&
points_equal(&queries[i * dimension],
&points[idx_dist.first * dimension],
dimension)) {
std::equal(&queries[i * dimension],
&queries[i * dimension] + dimension,
&points[idx_dist.first * dimension])) {
continue;
}
neighbors_indices[i].push_back(idx_dist.first);
Expand Down
3 changes: 3 additions & 0 deletions cpp/pybind/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ set(PYTHON_COMPILED_MODULE_DIR
if (APPLE)
set_target_properties(pybind PROPERTIES BUILD_RPATH "@loader_path;@loader_path/..")
elseif (UNIX)
# Use RPATH instead of RUNPATH in pybind so that needed libc++.so can find child dependant libc++abi.so in RPATH
# https://stackoverflow.com/questions/69662319/managing-secondary-dependencies-of-shared-libraries
target_link_options(pybind PRIVATE "LINKER:--disable-new-dtags")
set_target_properties(pybind PROPERTIES BUILD_RPATH "$ORIGIN;$ORIGIN/..")
endif()
set_target_properties(pybind PROPERTIES
Expand Down
7 changes: 7 additions & 0 deletions cpp/tests/t/geometry/TensorMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ TEST_P(TensorMapPermuteDevices, Constructor) {
// Primary key is required.
EXPECT_ANY_THROW(t::geometry::TensorMap());

// Delete primary key.
EXPECT_ANY_THROW(tm0.Erase("positions"));

// Reserved keys.
EXPECT_ANY_THROW(tm0.insert(
{"primary_key", core::Tensor::Zeros({2, 3}, dtype, device)}));

// Iterators.
std::map<std::string, core::Tensor> tensor_map(
{{"positions", core::Tensor::Zeros({10, 3}, dtype, device)},
Expand Down
44 changes: 29 additions & 15 deletions docker/Dockerfile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -67,34 +67,48 @@ RUN if [ "${BUILD_SYCL_MODULE}" = "ON" ]; then \
rm -rf /etc/apt/sources.list.d/oneAPI.list; \
fi

# Dependencies: basic
# Dependencies: basic and python-build
RUN apt-get update && apt-get install -y \
git \
wget \
curl \
build-essential \
pkg-config \
zlib1g \
zlib1g-dev \
libssl-dev \
libbz2-dev \
libreadline-dev \
libsqlite3-dev \
libncursesw5-dev \
xz-utils \
tk-dev \
libxml2-dev \
libxmlsec1-dev \
libffi-dev \
liblzma-dev \
&& rm -rf /var/lib/apt/lists/*

# Miniconda or Intel conda
# The **/open3d/bin paths are used during docker run, in this way docker run
# pyenv or Intel Python
# The pyenv python paths are used during docker run, in this way docker run
# does not need to activate the environment again.
ENV PATH="/root/miniconda3/bin:${PATH}"
ENV PATH="/root/miniconda3/envs/open3d/bin:${PATH}"
# The soft link from the python patch level version to the python mino version
# ensures python wheel commands (i.e. open3d) are in PATH, since we don't know
# which patch level pyenv will install (latest).
ENV PYENV_ROOT=/root/.pyenv
ENV PATH="$PYENV_ROOT/shims:$PYENV_ROOT/bin:$PYENV_ROOT/versions/$PYTHON_VERSION/bin:$PATH"
ENV PATH="/opt/intel/oneapi/intelpython/latest/bin:${PATH}"
ENV PATH="/opt/intel/oneapi/intelpython/latest/envs/open3d/bin:${PATH}"
RUN if [ "${BUILD_SYCL_MODULE}" = "OFF" ]; then \
wget -q https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh; \
bash Miniconda3-latest-Linux-x86_64.sh -b; \
rm Miniconda3-latest-Linux-x86_64.sh; \
curl https://pyenv.run | bash \
&& pyenv update \
&& pyenv install $PYTHON_VERSION \
&& pyenv global $PYTHON_VERSION \
&& pyenv rehash \
&& ln -s $PYENV_ROOT/versions/${PYTHON_VERSION}* $PYENV_ROOT/versions/${PYTHON_VERSION}; \
fi
RUN conda --version \
&& conda create -y -n open3d python=${PYTHON_VERSION}
RUN python --version && pip --version

# Activate open3d virtualenv
# This works during docker build. It becomes the prefix of all RUN commands.
# Ref: https://stackoverflow.com/a/60148365/1255535
SHELL ["conda", "run", "-n", "open3d", "/bin/bash", "-o", "pipefail", "-c"]
SHELL ["/bin/bash", "-o", "pipefail", "-c"]

# Dependencies: cmake
ENV PATH=${HOME}/${CMAKE_VERSION}/bin:${PATH}
Expand Down
2 changes: 1 addition & 1 deletion docker/docker_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ cpp_python_linking_uninstall_test() {
# Python test
echo "pytest is randomized, add --randomly-seed=SEED to repeat the test sequence."
${docker_run} -i --rm "${DOCKER_TAG}" /bin/bash -c " \
python -m pytest python/test ${pytest_args} -s"
python -W default -m pytest python/test ${pytest_args} -s"
restart_docker_daemon_if_on_gcloud

# Command-line tools test
Expand Down
8 changes: 0 additions & 8 deletions python/open3d/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ def load_cdll(path):
if sys.platform == "win32": # Unix: Use rpath to find libraries
_win32_dll_dir = os.add_dll_directory(str(Path(__file__).parent))

if _build_config["BUILD_GUI"] and not (find_library("c++abi") or
find_library("c++")):
try: # Preload libc++.so and libc++abi.so (required by filament)
load_cdll(str(next((Path(__file__).parent).glob("*c++abi.*"))))
load_cdll(str(next((Path(__file__).parent).glob("*c++.*"))))
except StopIteration: # Not found: check system paths while loading
pass

__DEVICE_API__ = "cpu"
if _build_config["BUILD_CUDA_MODULE"]:
# Load CPU pybind dll gracefully without introducing new python variable.
Expand Down
11 changes: 8 additions & 3 deletions util/install_deps_ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,22 @@ eval $(
echo DISTRIB_ID="$DISTRIB_ID";
echo DISTRIB_RELEASE="$DISTRIB_RELEASE"
)
# To avoid dependence on libunwind, we don't want to use clang / libc++ versions later than 11.
# Ubuntu 20.04's has versions 8, 10 or 12 while Ubuntu 22.04 has versions 11 and later.
if [ "$DISTRIB_ID" == "Ubuntu" -a "$DISTRIB_RELEASE" == "20.04" ]; then
# Ubuntu 20.04's clang/libc++-dev/libc++abi-dev are version 8, 10 or 12.
# To avoid dependence on libunwind, we don't want to use versions later than 10.
deps=("${deps[@]/clang/clang-10}")
deps=("${deps[@]/libc++-dev/libc++-10-dev}")
deps=("${deps[@]/libc++abi-dev/libc++abi-10-dev}")
fi
if [ "$DISTRIB_ID" == "Ubuntu" -a "$DISTRIB_RELEASE" == "22.04" ]; then
deps=("${deps[@]/clang/clang-11}")
deps=("${deps[@]/libc++-dev/libc++-11-dev}")
deps=("${deps[@]/libc++abi-dev/libc++abi-11-dev}")
fi

# Special case for ARM64
if [ "$(uname -m)" == "aarch64" ]; then
# For compling LAPACK in OpenBLAS
# For compiling LAPACK in OpenBLAS
deps+=("gfortran")
fi

Expand Down

0 comments on commit e88c7b1

Please sign in to comment.