Re: [PR] GH-48222: [CI][Dev] Fix shellcheck errors in ci/scripts/cpp_build.sh [arrow]

["hiroyuki-sato (via GitHub)" <gi...@apache.org>]

hiroyuki-sato commented on code in PR #48223:
URL: https://github.com/apache/arrow/pull/48223#discussion_r3555780991


##########
ci/scripts/cpp_build.sh:
##########
@@ -141,164 +144,166 @@ if [ "${ARROW_USE_MESON:-OFF}" = "ON" ]; then
     fi
   fi
   meson setup \
-    --prefix=${MESON_PREFIX:-${ARROW_HOME}} \
-    --buildtype=${ARROW_BUILD_TYPE:-debug} \
+    --prefix="${MESON_PREFIX:-${ARROW_HOME}}" \
+    --buildtype="${ARROW_BUILD_TYPE:-debug}" \
     --pkg-config-path="${CONDA_PREFIX}/lib/pkgconfig/" \
     -Dauto_features=enabled \
     -Dfuzzing=disabled \
     -Ds3=disabled \
     . \
-    ${source_dir}
+    "${source_dir}"
 
   CC="${ORIGINAL_CC}"
   CXX="${ORIGINAL_CXX}"
 elif [ "${ARROW_EMSCRIPTEN:-OFF}" = "ON" ]; then
   if [ "${UBUNTU}" = "20.04" ]; then
     echo "arrow emscripten build is not supported on Ubuntu 20.04, run with UBUNTU=22.04"
-    exit -1
+    exit 1
   fi
   n_jobs=2 # Emscripten build fails on docker unless this is set really low
+  # shellcheck source=/dev/null
   source ~/emsdk/emsdk_env.sh
-  export CMAKE_INSTALL_PREFIX=$(em-config CACHE)/sysroot
+  CMAKE_INSTALL_PREFIX=$(em-config CACHE)/sysroot
+  export CMAKE_INSTALL_PREFIX
   # conda sets LDFLAGS / CFLAGS etc. which break
   # emcmake so we unset them
   unset LDFLAGS CFLAGS CXXFLAGS CPPFLAGS
   emcmake cmake \
-    --preset=ninja-${ARROW_BUILD_TYPE:-debug}-emscripten \
-    -DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE:-OFF} \
+    --preset=ninja-"${ARROW_BUILD_TYPE:-debug}"-emscripten \
+    -DCMAKE_VERBOSE_MAKEFILE="${CMAKE_VERBOSE_MAKEFILE:-OFF}" \
     -DCMAKE_C_FLAGS="${CFLAGS:-}" \
     -DCMAKE_CXX_FLAGS="${CXXFLAGS:-}" \
     -DCMAKE_CXX_STANDARD="${CMAKE_CXX_STANDARD:-17}" \
-    -DCMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_LIBDIR:-lib} \
-    -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}} \
-    -DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
-    ${ARROW_CMAKE_ARGS} \
-    ${source_dir}
+    -DCMAKE_INSTALL_LIBDIR="${CMAKE_INSTALL_LIBDIR:-lib}" \
+    -DCMAKE_INSTALL_PREFIX="${CMAKE_INSTALL_PREFIX:-${ARROW_HOME}}" \
+    -DCMAKE_UNITY_BUILD="${CMAKE_UNITY_BUILD:-OFF}" \
+    "${ARROW_CMAKE_ARGS}" \

Review Comment:
   @kou @raulcd 
   
   Sorry for the delay. Work has been keeping me busy lately, but I'm back now and will continue working on this PR. Thanks for your patience!
   
   I added the follwing code. And It seems CI passed.
   
   ```bash
   # Convert the space-separated CMake options into a Bash array.
   # This avoids ShellCheck SC2086 and preserves argument boundaries.
   read -r -a ARROW_CMAKE_ARGS_ARRAY <<< "$ARROW_CMAKE_ARGS"
   ```
   
   Alternative idea
   
   ```bash
   # shellcheck disable=SC2086
   cmake \
     #...
     ${ARROW_CMAKE_ARGS} \
   ```
   
   Which approach do you prefer?
   
   Best regards,



-- 
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