[GitHub] [arrow] nealrichardson commented on a diff in pull request #14235: ARROW-17692: [R] Add support for building with system AWS SDK C++

[GitBox <gi...@apache.org>]

nealrichardson commented on code in PR #14235:
URL: https://github.com/apache/arrow/pull/14235#discussion_r991360709


##########
ci/scripts/r_docker_configure.sh:
##########
@@ -74,20 +74,105 @@ if [[ -n "$DEVTOOLSET_VERSION" ]]; then
   $PACKAGE_MANAGER install -y "devtoolset-$DEVTOOLSET_VERSION"
 fi
 
+case "$PACKAGE_MANAGER" in

Review Comment:
   Why does the R setup need Python installed now?



##########
r/configure:
##########
@@ -149,53 +150,42 @@ else
       if [ "$UNAME" = "Darwin" ] && [ "${OPENSSL_ROOT_DIR}" = "" ]; then
         brew --prefix openssl >/dev/null 2>&1
         if [ $? -eq 0 ]; then
-          OPENSSL_ROOT_DIR="`brew --prefix openssl`"; export OPENSSL_ROOT_DIR
+          export OPENSSL_ROOT_DIR="`brew --prefix openssl`"
+          export PKG_CONFIG_PATH="`brew --prefix openssl`/lib/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}"
         fi
       fi
 
       if [ "${ARROW_DEPENDENCY_SOURCE}" = "" ]; then
-        ARROW_DEPENDENCY_SOURCE=AUTO; export ARROW_DEPENDENCY_SOURCE
+        export ARROW_DEPENDENCY_SOURCE=AUTO
       fi
       if [ "${ARROW_DEPENDENCY_SOURCE}" = "AUTO" ]; then
         pkg-config --version >/dev/null 2>&1
         if [ $? -ne 0 ]; then
-          export ARROW_DEPENDENCY_SOURCE=BUNDLED
-          echo "**** pkg-config not installed, setting ARROW_DEPENDENCY_SOURCE=BUNDLED"
+          echo "**** pkg-config is required to use automatically installed Apache Arrow C++"

Review Comment:
   Why this change? 



##########
dev/tasks/r/github.packages.yml:
##########
@@ -309,12 +309,22 @@ jobs:
           curl -s \
             https://raw.githubusercontent.com/{{ arrow.github_repo }}/{{ arrow.head }}/ci/scripts/install_sccache.sh | \
             bash -s unknown-linux-musl /usr/local/bin
-      - run: sudo apt update && sudo apt install libcurl4-openssl-dev
-      - name: Prepare PKG_CONFIG_PATH for Homebrew
+      - name: Install curl and its dependencies for static link
         run: |
-          # zstd is installed by Homebrew on GitHub Actions.
-          echo "PKG_CONFIG_PATH=$(brew --prefix)/lib/pkgconfig${PKG_CONFIG_PATH:+:${PKG_CONFIG_PATH}}" \
-            >> "$GITHUB_ENV"
+          sudo apt update
+          sudo apt install -y \
+            libbrotli-dev \
+            libcurl4-openssl-dev \
+            libidn2-dev \

Review Comment:
   I don't understand, why do we need all of these extra dependencies now? If they're dependencies of libcurl, why isn't installing libcurl enough?



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org