Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
deb0c72
Make dependency source configurable
thisisnic Jul 13, 2021
9321021
Move $ prefix to right place
thisisnic Jul 13, 2021
c6c3dea
Check if pkg-config installed before allowing DEPENDENCY_SOURCE="AUTO"
thisisnic Jul 13, 2021
b4a1b16
Add new CI job that tests both AUTO and BUNDLED dependencies
thisisnic Jul 14, 2021
3bdd22f
Update to have correct matrix param
thisisnic Jul 14, 2021
29d43ac
Update name and env vars
thisisnic Jul 14, 2021
34349b8
Try updating in docker-compose file
thisisnic Jul 14, 2021
e191f92
Fix naming
thisisnic Jul 14, 2021
65bd169
Try passing the var through
thisisnic Jul 14, 2021
933113b
Add print to see what's happening
thisisnic Jul 14, 2021
fb2c199
jon's suggestions
thisisnic Jul 14, 2021
1a6b4ea
Remove diagnostic messaging
thisisnic Jul 27, 2021
894b428
Could it be this space?
jonkeane Jul 26, 2021
2245842
update var name for clarity
thisisnic Jul 27, 2021
af0a681
Add back in ??
thisisnic Jul 27, 2021
1de55e0
typo
thisisnic Jul 27, 2021
6e83b5d
Remove unnecessary var
thisisnic Jul 27, 2021
fe7c6c4
Reset build_arrow_static.sh
thisisnic Jul 27, 2021
f4dbe36
Allow pkg-config is arrow-dependency_source is auto
thisisnic Jul 28, 2021
e1d4edc
Call pkgconfig on the install dir
thisisnic Jul 28, 2021
e5e8262
Allow dependency source to be configurable
thisisnic Jul 30, 2021
a5091bd
Relocate code
thisisnic Jul 30, 2021
4dd959f
Remove references to unnecessary variable
thisisnic Jul 30, 2021
4753b48
Update new CI job to read in ARROW_DEPENDENCY_SOURCE via docker env var
thisisnic Aug 2, 2021
a3f291f
Various suggested changes
thisisnic Aug 2, 2021
5fde64a
Fix docker run call
thisisnic Aug 2, 2021
66fc156
double to single equals
thisisnic Aug 2, 2021
cd6ffe1
Install pkg-config
thisisnic Aug 2, 2021
fdd3f2a
Change arbitrary things
thisisnic Aug 2, 2021
2c51df3
Logic round way round
thisisnic Aug 2, 2021
7c526bd
Update comments for clarity
thisisnic Aug 2, 2021
4b4e665
this is why we don't randomly change things
thisisnic Aug 2, 2021
fb6d77f
Unquote vals
thisisnic Aug 2, 2021
b74d5b8
Update dev/tasks/tasks.yml
thisisnic Aug 3, 2021
861c03f
Use existing job
thisisnic Aug 3, 2021
afac895
fix service spec
thisisnic Aug 3, 2021
d4259ca
add flags
thisisnic Aug 3, 2021
20bed92
test with bundled
thisisnic Aug 3, 2021
b30b74d
Don't use only-r image
thisisnic Aug 3, 2021
68dcb1b
Remove bundled job
thisisnic Aug 3, 2021
00e4611
Use different job as base
thisisnic Aug 3, 2021
9cd8f00
Bad spacing
thisisnic Aug 3, 2021
77a3d57
incorrectly named param
thisisnic Aug 3, 2021
8775d48
Update azure.linux.yml
thisisnic Aug 3, 2021
3325981
Update tasks.yml
thisisnic Aug 3, 2021
e116151
pass in flags arg
thisisnic Aug 3, 2021
659dd69
Add default argument
nealrichardson Aug 3, 2021
dd78fd1
loosen the matching for dumping install logs to catch ubuntu-r
jonkeane Aug 8, 2021
ba87c9e
Update r/configure
thisisnic Aug 9, 2021
c3ea16b
Add if: always() to make the dump step actually work
jonkeane Aug 9, 2021
ab1d149
oops
jonkeane Aug 9, 2021
593b930
more bash
jonkeane Aug 9, 2021
956b3d6
pass AWSSDK_SOURCE
jonkeane Aug 9, 2021
acb276b
Kou's patch
jonkeane Aug 10, 2021
2593146
[[ -> [
jonkeane Aug 10, 2021
4765f8e
Add find_package PKGConfig
thisisnic Aug 10, 2021
2533871
Run ldconfig
thisisnic Aug 10, 2021
3ef25a4
use ubuntu-r-only-r for 21.04
jonkeane Aug 10, 2021
39156ee
add pkgconfig to the pkg-config path
jonkeane Aug 10, 2021
9e2c3e2
cleanup a whitespace cruft
jonkeane Aug 10, 2021
27baa42
Move PkgConfig finding to correct location
thisisnic Aug 11, 2021
e841f80
a ARROW_DEPENDENCY_SOURCE=SYSTEM task + trying to unsent ARROW_DEPEND…
jonkeane Aug 11, 2021
a3b0f18
try backing out --static
jonkeane Aug 11, 2021
13b6b6b
no --static anywhere
jonkeane Aug 11, 2021
bcba8ba
Update cpp/cmake_modules/ThirdpartyToolchain.cmake
jonkeane Aug 15, 2021
ec61d2d
try adding back in one --static?
jonkeane Aug 15, 2021
d2f6d1a
remove an overly-zealous addition
jonkeane Aug 17, 2021
2cf3a26
shuffle where we unset ARROW_DEPENDENCY_SOURCE
jonkeane Aug 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ci/docker/linux-apt-r.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ ENV \
ARROW_BUILD_STATIC=OFF \
ARROW_BUILD_TESTS=OFF \
ARROW_BUILD_UTILITIES=OFF \
ARROW_DEPENDENCY_SOURCE=SYSTEM \
ARROW_FLIGHT=OFF \
ARROW_GANDIVA=OFF \
ARROW_NO_DEPRECATED_API=ON \
Expand Down
14 changes: 12 additions & 2 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,17 @@ set(Boost_ADDITIONAL_VERSIONS
# so we first need to determine whether we're building it
if(ARROW_WITH_THRIFT AND Thrift_SOURCE STREQUAL "AUTO")
find_package(Thrift 0.11.0 MODULE COMPONENTS ${ARROW_THRIFT_REQUIRED_COMPONENTS})
if(NOT Thrift_FOUND AND NOT THRIFT_FOUND)
if(Thrift_FOUND)
find_package(PkgConfig QUIET)
pkg_check_modules(THRIFT_PC
thrift
NO_CMAKE_PATH
NO_CMAKE_ENVIRONMENT_PATH
QUIET)
if(THRIFT_PC_FOUND)
string(APPEND ARROW_PC_REQUIRES_PRIVATE " thrift")
endif()
else()
set(Thrift_SOURCE "BUNDLED")
endif()
endif()
Expand Down Expand Up @@ -1306,7 +1316,7 @@ endmacro()
if(ARROW_WITH_THRIFT)
# We already may have looked for Thrift earlier, when considering whether
# to build Boost, so don't look again if already found.
if(NOT Thrift_FOUND AND NOT THRIFT_FOUND)
if(NOT Thrift_FOUND)
# Thrift c++ code generated by 0.13 requires 0.11 or greater
resolve_dependency(Thrift
REQUIRED_VERSION
Expand Down
3 changes: 2 additions & 1 deletion dev/tasks/docker-tests/github.linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ jobs:
shell: bash
run: archery docker run -e SETUPTOOLS_SCM_PRETEND_VERSION="{{ arrow.no_rc_version }}" {{ flags|default("") }} {{ image }} {{ command|default("") }}

{% if '-r-' in image %}
{% if '-r' in image %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? Is this too broad?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this change, some builds (e.g. test-r-ubuntu-21.04 which uses the docker-compose service ubuntu-r-only-r which extends ubuntu-r) didn't have the log dumping step showing up at all. https://github.com/ursacomputing/crossbow/runs/3272438137?check_suite_focus=true

The strange thing is that it does have -r- in the underlying image name (e.g. ${REPO}:${ARCH}-ubuntu-${UBUNTU}-r-${R}), but somehow the jinja matching isn't quite getting it there (though -r works). It might be overly-broad though that step should be harmless since it continue-on-error.

- name: Dump R install logs
run: cat arrow/r/check/arrow.Rcheck/00install.out
continue-on-error: true
if: always()
{% endif %}

{% if arrow.branch == 'master' %}
Expand Down
22 changes: 21 additions & 1 deletion dev/tasks/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,26 @@ tasks:
test-r-devdocs:
ci: github
template: r/github.devdocs.yml

test-r-depsource-auto:
ci: azure
template: r/azure.linux.yml
params:
r_org: rocker
r_image: r-base
r_tag: latest
flags: '-e ARROW_DEPENDENCY_SOURCE=AUTO'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we try and add a task that also tests ARROW_DEPENDENCY_SOURCE=SYSTEM? To test when we can't fall back to building? Maybe I'm paranoid, but I'm a bit worried if the rocker base happens to not have any of our dependencies installed, this isn't actually exercising linking to them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also make a follow up for this, I don't think that ARROW_DEPENDENCY_SOURCE=SYSTEM will work with this image without extending it a bit to install the other dependencies we need.


test-r-depsource-system:
ci: github
template: docker-tests/github.linux.yml
params:
env:
UBUNTU: 21.04
CLANG_TOOLS: 9 # can remove this when >=9 is the default
flags: '-e ARROW_SOURCE_HOME="/arrow" -e FORCE_BUNDLED_BUILD=TRUE -e LIBARROW_BUILD=TRUE -e ARROW_DEPENDENCY_SOURCE=SYSTEM'
image: ubuntu-r-only-r


{% for r_org, r_image, r_tag in [("rhub", "ubuntu-gcc-release", "latest"),
("rocker", "r-base", "latest"),
Expand Down Expand Up @@ -1050,7 +1070,7 @@ tasks:
UBUNTU: 21.04
CLANG_TOOLS: 9 # can remove this when >=9 is the default
flags: '-e ARROW_SOURCE_HOME="/arrow" -e FORCE_BUNDLED_BUILD=TRUE -e LIBARROW_BUILD=TRUE'
image: ubuntu-r
image: ubuntu-r-only-r

# This also has -flto=auto
test-r-gcc-11:
Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,8 @@ services:
/arrow/ci/scripts/r_test.sh /arrow"

ubuntu-r-only-r:
environment:
ARROW_DEPENDENCY_SOURCE: ''
extends: ubuntu-r
command: >
/bin/bash -c "
Expand Down
24 changes: 21 additions & 3 deletions r/configure
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ elif [ "$INCLUDE_DIR" ] && [ "$LIB_DIR" ]; then
PKG_CFLAGS="-I$INCLUDE_DIR $PKG_CFLAGS"
PKG_DIRS="-L$LIB_DIR"
else
# Use pkg-config if available and allowed
# Use pkg-config to find libarrow if available and allowed
pkg-config --version >/dev/null 2>&1
if [ $? -eq 0 ] && [ "$ARROW_USE_PKG_CONFIG" != "false" ]; then
# Set the search paths and compile flags
PKGCONFIG_CFLAGS=`pkg-config --cflags --silence-errors ${PKG_CONFIG_NAME}`
PKGCONFIG_LIBS=`pkg-config --libs-only-l --silence-errors ${PKG_CONFIG_NAME}`
PKGCONFIG_LIBS=`pkg-config --libs-only-l --libs-only-other --silence-errors ${PKG_CONFIG_NAME}`
PKGCONFIG_DIRS=`pkg-config --libs-only-L --silence-errors ${PKG_CONFIG_NAME}`
# TODO: what about --libs-only-other?
fi

if [ "$PKGCONFIG_CFLAGS" ] && [ "$PKGCONFIG_LIBS" ]; then
Expand Down Expand Up @@ -156,6 +156,19 @@ else
fi
fi

if [ "${ARROW_DEPENDENCY_SOURCE}" = "" ]; then
# TODO: BUNDLED is still default for now, but we plan to change it to AUTO
ARROW_DEPENDENCY_SOURCE=BUNDLED; export ARROW_DEPENDENCY_SOURCE
fi
if [ "${ARROW_DEPENDENCY_SOURCE}" = "AUTO" ]; then
pkg-config --version >/dev/null 2>&1
if [ $? -ne 0 ]; then
export ARROW_DEPENDENCY_SOURCE=BUNDLED
echo "**** Warning: ARROW_DEPENDENCY_SOURCE set to 'AUTO' but pkg-config not installed"
echo "**** ARROW_DEPENDENCY_SOURCE has been set to 'BUNDLED'"
fi
fi

${R_HOME}/bin/Rscript tools/nixlibs.R $VERSION
PKG_CFLAGS="-I`pwd`/libarrow/arrow-${VERSION}/include $PKG_CFLAGS"

Expand All @@ -173,6 +186,11 @@ else
BUNDLED_LIBS=`echo "$BUNDLED_LIBS" | sed -e "s/\\.a lib/ -l/g" | sed -e "s/\\.a$//" | sed -e "s/^lib/-l/" | tr '\n' ' ' | sed -e "s/ $//"`
PKG_DIRS="-L`pwd`/$LIB_DIR"

# Use pkg-config to do static linking of libarrow's dependencies
if [ "$ARROW_DEPENDENCY_SOURCE" = "AUTO" ] || [ "$ARROW_DEPENDENCY_SOURCE" = "SYSTEM" ]; then
PKG_LIBS="$PKG_LIBS `PKG_CONFIG_PATH=${LIB_DIR}/pkgconfig pkg-config --libs-only-l --libs-only-other --static --silence-errors ${PKG_CONFIG_NAME}`"
fi

# When using brew's openssl it is not bundled and it is not on the system
# search path and so we must add the lib path to BUNDLED_LIBS if we are
# using it. Note the order is important, this must be after the arrow
Expand Down
3 changes: 2 additions & 1 deletion r/inst/build_arrow_static.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ ${CMAKE} -DARROW_BOOST_USE_SHARED=OFF \
-DARROW_COMPUTE=ON \
-DARROW_CSV=ON \
-DARROW_DATASET=${ARROW_DATASET:-ON} \
-DARROW_DEPENDENCY_SOURCE=BUNDLED \
-DARROW_DEPENDENCY_SOURCE=${ARROW_DEPENDENCY_SOURCE:-BUNDLED} \
-DAWSSDK_SOURCE=${AWSSDK_SOURCE:-} \
-DARROW_FILESYSTEM=ON \
-DARROW_JEMALLOC=${ARROW_JEMALLOC:-$ARROW_DEFAULT_PARAM} \
-DARROW_MIMALLOC=${ARROW_MIMALLOC:-ON} \
Expand Down