Skip to content

Commit

Permalink
apacheGH-38902: [R] Handle failing library detection with pkg-config (a…
Browse files Browse the repository at this point in the history
…pache#38970)

### Rationale for this change

We can get into a broken state with a working test compile in `nixlibs.R` but empty `PKG_LIBS` when pkg-config fails to find some libraries (e.g. libcurl on mac due to missing system stubs) in `configure`. This leads to a failed test compile in configure with pc errors silenced.

### What changes are included in this PR?

Catch this and rerun the pkg-config-less library detection that should fix this in most cases.

### Are these changes tested?

locally and on cran (where this error first surfaced)
* Closes: apache#38902

Lead-authored-by: Jacob Wujciak-Jens <jacob@wujciak.de>
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jacob Wujciak-Jens <jacob@wujciak.de>
  • Loading branch information
assignUser and jonkeane committed Dec 1, 2023
1 parent 661ba3b commit 1b43f53
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 19 deletions.
39 changes: 23 additions & 16 deletions r/configure
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ VERSION=`grep '^Version' DESCRIPTION | sed s/Version:\ //`
UNAME=`uname -s`
: ${PKG_CONFIG:="pkg-config"}

# These will only be set in the bundled build
S3_LIBS=""
GCS_LIBS=""

# If in development mode, run the codegen script to render arrowExports.*
if [ "$ARROW_R_DEV" = "true" ] && [ -f "data-raw/codegen.R" ]; then
Expand Down Expand Up @@ -116,7 +113,9 @@ fi
# Test if pkg-config is available to use
if ${PKG_CONFIG} --version >/dev/null 2>&1; then
PKG_CONFIG_AVAILABLE="true"
echo "*** pkg-config found."
else
echo "*** pkg-config not found."
PKG_CONFIG_AVAILABLE="false"
ARROW_USE_PKG_CONFIG="false"
fi
Expand Down Expand Up @@ -245,12 +244,6 @@ do_bundled_build () {
${LIB_DIR}/pkgconfig/*.pc
rm -f ${LIB_DIR}/pkgconfig/*.pc.bak
fi
else
# This case must be ARROW_DEPENDENCY_SOURCE=BUNDLED.
# These would be identified by pkg-config, in Requires.private and Libs.private.
# Rather than try to re-implement pkg-config, we can just hard-code them here.
S3_LIBS="-lcurl -lssl -lcrypto"
GCS_LIBS="-lcurl -lssl -lcrypto"
fi
else
# If the library directory does not exist, the script must not have been successful
Expand Down Expand Up @@ -293,15 +286,15 @@ set_pkg_vars () {

# If we have pkg-config, it will tell us what libarrow needs
set_lib_dir_with_pc () {
LIB_DIR="`${PKG_CONFIG} --variable=libdir --silence-errors ${PKG_CONFIG_NAME}`"
LIB_DIR="`${PKG_CONFIG} --variable=libdir ${PKG_CONFIG_NAME}`"
}
set_pkg_vars_with_pc () {
pkg_config_names="${PKG_CONFIG_NAME} ${PKG_CONFIG_NAMES_FEATURES}"
PKG_CFLAGS="`${PKG_CONFIG} --cflags --silence-errors ${pkg_config_names}` $PKG_CFLAGS"
PKG_CFLAGS="`${PKG_CONFIG} --cflags ${pkg_config_names}` $PKG_CFLAGS"
PKG_CFLAGS="$PKG_CFLAGS $PKG_CFLAGS_FEATURES"
PKG_LIBS=`${PKG_CONFIG} --libs-only-l --libs-only-other --silence-errors ${pkg_config_names}`
PKG_LIBS=`${PKG_CONFIG} --libs-only-l --libs-only-other ${pkg_config_names}`
PKG_LIBS="$PKG_LIBS $PKG_LIBS_FEATURES"
PKG_DIRS=`${PKG_CONFIG} --libs-only-L --silence-errors ${pkg_config_names}`
PKG_DIRS=`${PKG_CONFIG} --libs-only-L ${pkg_config_names}`
}

# If we don't have pkg-config, we can make some inferences
Expand All @@ -322,7 +315,7 @@ set_pkg_vars_without_pc () {
if [ -n "$(find "$LIB_DIR" -name 'libarrow_bundled_dependencies.*')" ]; then
PKG_LIBS="$PKG_LIBS -larrow_bundled_dependencies"
fi
PKG_LIBS="$PKG_LIBS $PKG_LIBS_FEATURES"
PKG_LIBS="$PKG_LIBS $PKG_LIBS_FEATURES $SSL_LIBS_WITHOUT_PC"

# If on Raspberry Pi, need to manually link against latomic
# See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358 for similar example
Expand Down Expand Up @@ -380,11 +373,13 @@ add_feature_flags () {
fi
if arrow_built_with ARROW_S3; then
PKG_CFLAGS_FEATURES="$PKG_CFLAGS_FEATURES -DARROW_R_WITH_S3"
PKG_LIBS_FEATURES="$PKG_LIBS_FEATURES $S3_LIBS"
fi
if arrow_built_with ARROW_GCS; then
PKG_CFLAGS_FEATURES="$PKG_CFLAGS_FEATURES -DARROW_R_WITH_GCS"
PKG_LIBS_FEATURES="$PKG_LIBS_FEATURES $GCS_LIBS"
fi
if arrow_built_with ARROW_GCS || arrow_built_with ARROW_S3; then
# If pkg-config is available it will handle this for us automatically
SSL_LIBS_WITHOUT_PC="-lcurl -lssl -lcrypto"
fi
fi
}
Expand All @@ -405,6 +400,18 @@ find_or_build_libarrow
# Now set `PKG_LIBS`, `PKG_DIRS`, and `PKG_CFLAGS` based on that.
if [ "$_LIBARROW_FOUND" != "false" ] && [ "$_LIBARROW_FOUND" != "" ]; then
set_pkg_vars ${_LIBARROW_FOUND}

# If we didn't find any libraries with pkg-config, try again without pkg-config
FOUND_PKG_LIBS=`echo "$PKG_LIBS" | tr -d '[:space:]'`
if [ -z "$FOUND_PKG_LIBS" ] && [ "$PKG_CONFIG_AVAILABLE" = "true" ]; then
echo "*** pkg-config failed to find libraries. Running detection without pkg-config."
PKG_CONFIG_AVAILABLE="false"
set_pkg_vars ${_LIBARROW_FOUND}
fi
else
# To make it easier to debug which code path was taken add a specific
# message to the log in addition to the 'NOTE'
echo "*** Failed to find Arrow C++ libraries."
fi

# Test that we can compile something with those flags
Expand Down
2 changes: 1 addition & 1 deletion r/tools/nixlibs.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ find_latest_nightly <- function(description_version,
lg("Failed to find latest nightly for %s", description_version)
latest <- description_version
} else {
lg("Found latest nightly for %s: %s", description_version, res)
lg("Latest available nightly for %s: %s", description_version, res)
latest <- res
}
latest
Expand Down
4 changes: 2 additions & 2 deletions r/tools/test-nixlibs.R
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,15 @@ test_that("find_latest_nightly()", {
find_latest_nightly(package_version("13.0.1.9000"), list_uri = tf_uri),
package_version("13.0.0.100000335")
),
"Found latest nightly"
"Latest available nightly"
)

expect_output(
expect_identical(
find_latest_nightly(package_version("14.0.0.9000"), list_uri = tf_uri),
package_version("14.0.0.100000001")
),
"Found latest nightly"
"Latest available nightly"
)

expect_output(
Expand Down

0 comments on commit 1b43f53

Please sign in to comment.