nealrichardson commented on a change in pull request #10710:
URL: https://github.com/apache/arrow/pull/10710#discussion_r677425010
##########
File path: r/inst/build_arrow_static.sh
##########
@@ -45,6 +45,19 @@ else
ARROW_DEFAULT_PARAM="OFF"
fi
+if [ "$R_STATIC_DEPENDENCY_SOURCE" = "AUTO" ]; then
Review comment:
This isn't quite right/complete, and I think if we look at what is
required to be complete, we'll see that we don't actually want this logic in
this script. Working backwards, the point of Kou's change is that the pkgconfig
files that the arrow C++ build does now contain all of the `-l` libs that the
build requires from the system (which shouldn't be any if we're doing a BUNDLED
build, but with AUTO there will be libs in it). So we need to make sure the
`PKG_LIBS` env var in `r/configure` gets them. And since this script, and even
`r/tools/nixlibs.R`, can't return anything or set anything in the env that
`r/configure` is running in, we need to call `pkg-config` on the arrow C++
install dir from `r/configure`. (This is the missing par.) But that will only
work if `pkg-config` exists, so we need to check for it like you do here--but I
think we need to check for it in `r/configure` so that that script know whether
to set AUTO or BUNDLED.
##########
File path: .env
##########
@@ -67,6 +67,7 @@ R_ORG=rhub
R_IMAGE=ubuntu-gcc-release
R_TAG=latest
TZ=UTC
+R_STATIC_DEPENDENCY_SOURCE=BUNDLED
Review comment:
I don't think we need this here. We have to put env vars here that
affect what the Dockerfiles build, but this is a runtime env var, so we can
just pass it with `-e` and don't need to declare it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
