-
Notifications
You must be signed in to change notification settings - Fork 3.9k
GH-48222: [CI][Dev] Fix shellcheck errors in ci/scripts/cpp_build.sh #48223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
GH-48222: [CI][Dev] Fix shellcheck errors in ci/scripts/cpp_build.sh #48223
Conversation
|
|
3e2ab62 to
77b408c
Compare
|
The failing check in CI seems to be unrelated to the PR I created. CI failures can be categorized into three categories: 1. OpenSSL build failure.https://github.com/apache/arrow/actions/runs/19607957272/job/56149467519?pr=48223
relates to this failure? 2. ccache relatesOther PRs also failed with the same error.
3. Docker build
|
| emcmake cmake \ | ||
| --preset=ninja-${ARROW_BUILD_TYPE:-debug}-emscripten \ | ||
| -DCMAKE_VERBOSE_MAKEFILE=${CMAKE_VERBOSE_MAKEFILE:-OFF} \ | ||
| --preset=ninja-"${ARROW_BUILD_TYPE:-debug}"-emscripten \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| --preset=ninja-"${ARROW_BUILD_TYPE:-debug}"-emscripten \ | |
| --preset="ninja-${ARROW_BUILD_TYPE:-debug}-emscripten" \ |
| -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}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this safe?
ARROW_CMAKE_ARGS may have multiple arguments such as -DXXX=111 -DYYY=222.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Deos ARROW_CMAKE_ARGS set via Environment variables? (A caller pass ARROW_CMAKE_ARGS from out side of a script)?
Should we convert ARROW_CMAKE_ARGS to ARROW_CMAKE_ARG_ARRAY using
read -r -a ARROW_CMAKE_ARG_ARRAY <<< "$ARROW_CMAKE_ARGS" ?
and Extract the variable using "${ARROW_CMAKE_ARG_ARRAY[@]}"
rg 'ARROW_CMAKE_ARGS' ci/scripts/cpp_build.sh
39: ARROW_CMAKE_ARGS+=" -DCMAKE_AR=${AR}"
42: ARROW_CMAKE_ARGS+=" -DCMAKE_RANLIB=${RANLIB}"
44: export ARROW_CMAKE_ARGS
180: "${ARROW_CMAKE_ARGS}" \
185: "${ARROW_CMAKE_ARGS}" \
292: "${ARROW_CMAKE_ARGS}" \
rg ARROW_CMAKE_ARGS
matlab/CMakeLists.txt
40: set(ARROW_CMAKE_ARGS
94: CMAKE_ARGS "${ARROW_CMAKE_ARGS}"
ci/scripts/cpp_build.sh
39: ARROW_CMAKE_ARGS+=" -DCMAKE_AR=${AR}"
42: ARROW_CMAKE_ARGS+=" -DCMAKE_RANLIB=${RANLIB}"
44: export ARROW_CMAKE_ARGS
180: "${ARROW_CMAKE_ARGS}" \
185: "${ARROW_CMAKE_ARGS}" \
292: "${ARROW_CMAKE_ARGS}" \
ci/docker/cpp-jni.dockerfile
110: ARROW_CMAKE_ARGS="-DARROW_BUILD_TESTS=ON" \
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Rationale for this change
This is the sub issue #44748.
$((..))instead of deprecated$[..]-ninstead of! -z.What changes are included in this PR?
$((..)).-nAre these changes tested?
Yes.
Are there any user-facing changes?
No.