Skip to content

Commit

Permalink
Change build.sh to find C++ library by default and avoid shadowing CM…
Browse files Browse the repository at this point in the history
…AKE_ARGS (NVIDIA#11013)

NVIDIA#10919 changed the Python build system to build its own internal copy of libcudf when invoked directly. It also modified our build.sh script (and associated CI scripts) accordingly so that everywhere that should be reusing a C++ build for a Python build would do so via CMake. This PR makes that behavior the default in build.sh, so any builds of the Python library using build.sh will revert to the prior behavior of searching for a C++ library and failing if one doesn't already exist on the path. Users can still provide the appropriate CMake arguments to the build.sh invocation to make it build libcudf within cuDF Python if they so desire.

Additionally, this PR avoids shadowing the `CMAKE_ARGS` environment variable in build.sh, which is important for conda-forge compatibility.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: rapidsai/cudf#11013
  • Loading branch information
vyasr authored Jun 3, 2022
1 parent 00f02a4 commit ef675dc
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
23 changes: 14 additions & 9 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ function cmakeArgs {
# There are possible weird edge cases that may cause this regex filter to output nothing and fail silently
# the true pipe will catch any weird edge cases that may happen and will cause the program to fall back
# on the invalid option error
CMAKE_ARGS=$(echo $ARGS | { grep -Eo "\-\-cmake\-args=\".+\"" || true; })
if [[ -n ${CMAKE_ARGS} ]]; then
# Remove the full CMAKE_ARGS argument from list of args so that it passes validArgs function
ARGS=${ARGS//$CMAKE_ARGS/}
EXTRA_CMAKE_ARGS=$(echo $ARGS | { grep -Eo "\-\-cmake\-args=\".+\"" || true; })
if [[ -n ${EXTRA_CMAKE_ARGS} ]]; then
# Remove the full EXTRA_CMAKE_ARGS argument from list of args so that it passes validArgs function
ARGS=${ARGS//$EXTRA_CMAKE_ARGS/}
# Filter the full argument down to just the extra string that will be added to cmake call
CMAKE_ARGS=$(echo $CMAKE_ARGS | grep -Eo "\".+\"" | sed -e 's/^"//' -e 's/"$//')
EXTRA_CMAKE_ARGS=$(echo $EXTRA_CMAKE_ARGS | grep -Eo "\".+\"" | sed -e 's/^"//' -e 's/"$//')
fi
fi
}
Expand Down Expand Up @@ -229,6 +229,11 @@ if hasArg --incl_cache_stats; then
BUILD_REPORT_INCL_CACHE_STATS=ON
fi

# Append `-DFIND_CUDF_CPP=ON` to EXTRA_CMAKE_ARGS unless a user specified the option.
if [[ "${EXTRA_CMAKE_ARGS}" != *"DFIND_CUDF_CPP"* ]]; then
EXTRA_CMAKE_ARGS="${EXTRA_CMAKE_ARGS} -DFIND_CUDF_CPP=ON"
fi


# If clean given, run it prior to any other steps
if hasArg clean; then
Expand Down Expand Up @@ -283,7 +288,7 @@ if buildAll || hasArg libcudf; then
-DDISABLE_DEPRECATION_WARNING=${BUILD_DISABLE_DEPRECATION_WARNING} \
-DCUDF_USE_PER_THREAD_DEFAULT_STREAM=${BUILD_PER_THREAD_DEFAULT_STREAM} \
-DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
${CMAKE_ARGS}
${EXTRA_CMAKE_ARGS}

cd ${LIB_BUILD_DIR}

Expand Down Expand Up @@ -324,9 +329,9 @@ fi
if buildAll || hasArg cudf; then

cd ${REPODIR}/python/cudf
python setup.py build_ext --inplace -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBCUDF_BUILD_DIR} ${CMAKE_ARGS} -- -j${PARALLEL_LEVEL:-1}
python setup.py build_ext --inplace -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBCUDF_BUILD_DIR} ${EXTRA_CMAKE_ARGS} -- -j${PARALLEL_LEVEL:-1}
if [[ ${INSTALL_TARGET} != "" ]]; then
python setup.py install --single-version-externally-managed --record=record.txt -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBCUDF_BUILD_DIR} ${CMAKE_ARGS} -- -j${PARALLEL_LEVEL:-1}
python setup.py install --single-version-externally-managed --record=record.txt -- -DCMAKE_PREFIX_PATH=${INSTALL_PREFIX} -DCMAKE_LIBRARY_PATH=${LIBCUDF_BUILD_DIR} ${EXTRA_CMAKE_ARGS} -- -j${PARALLEL_LEVEL:-1}
fi
fi

Expand All @@ -352,7 +357,7 @@ if hasArg libcudf_kafka; then
-DCMAKE_INSTALL_PREFIX=${INSTALL_PREFIX} \
-DBUILD_TESTS=${BUILD_TESTS} \
-DCMAKE_BUILD_TYPE=${BUILD_TYPE} \
${CMAKE_ARGS}
${EXTRA_CMAKE_ARGS}


cd ${KAFKA_LIB_BUILD_DIR}
Expand Down
2 changes: 1 addition & 1 deletion ci/gpu/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ if [[ -z "$PROJECT_FLASH" || "$PROJECT_FLASH" == "0" ]]; then
################################################################################

gpuci_logger "Build from source"
"$WORKSPACE/build.sh" clean libcudf cudf dask_cudf libcudf_kafka cudf_kafka benchmarks tests --ptds --cmake-args=\"-DFIND_CUDF_CPP=ON\"
"$WORKSPACE/build.sh" clean libcudf cudf dask_cudf libcudf_kafka cudf_kafka benchmarks tests --ptds

################################################################################
# TEST - Run GoogleTest
Expand Down
2 changes: 1 addition & 1 deletion conda/recipes/cudf/build.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2018-2022, NVIDIA CORPORATION.

# This assumes the script is executed from the root of the repo directory
./build.sh cudf --cmake-args=\"-DFIND_CUDF_CPP=ON\"
./build.sh cudf

0 comments on commit ef675dc

Please sign in to comment.