[ATfL]: Update build.sh for shellcheck comments and other fixes#856
[ATfL]: Update build.sh for shellcheck comments and other fixes#856vrukesh wants to merge 1 commit into
Conversation
Update build.sh to continue execution on test failures and fix the TRACE=1 error.
| ZLIB_STATIC_PATH=${ZLIB_STATIC_PATH:-"/usr/lib/`uname -m`-linux-gnu/libz.a"} | ||
| ZLIB_STATIC_PATH=${ZLIB_STATIC_PATH:-"/usr/lib/$(uname -m)-linux-gnu/libz.a"} | ||
| COMMON_LINKER_FLAGS="-Wl,--build-id" | ||
| COMMON_CMAKE_FLAGS_FROM_ENV=() |
There was a problem hiding this comment.
In my other PR I'm trying to get rid of this passing of the COMMON_CMAKE_FLAGS from the outside entirely (from env, or from anything). I can't find any use case for it, but I'm finding it particularly problematic in the further refactor this script will need in order to accommodate bolting the compiler. Can we just remove this use of the COMMON_CMAKE_FLAGS entirely?
| fi | ||
| COMMON_CMAKE_FLAGS=( | ||
| ${COMMON_CMAKE_FLAGS} | ||
| "${COMMON_CMAKE_FLAGS_FROM_ENV[@]}" |
| else | ||
| return 1 | ||
| fi | ||
| directory_has_entries "${LIBRARIES_DIR}" |
There was a problem hiding this comment.
this is an awkward syntax. To avoid confusing any further reader of this code, is there a way we could see return in this function and other functions below?
| run_command ninja ${NINJA_ARGS} check-cxxabi 2>&1 | tee -a "${LOGS_DIR}/libcpp.txt" | ||
| export LD_LIBRARY_PATH="${ATFL_DIR}/lib:${ATFL_DIR}/lib/${ATFL_TARGET_TRIPLE}:${LD_LIBRARY_PATH}" | ||
|
|
||
| LIT_OPTS="${LIT_OPTS} --xunit-xml-output=${LOGS_DIR}/check_cxx.xml" \ |
There was a problem hiding this comment.
I'm having an impression that too much is happening in one PR. Could the feature extensions like this be addressed by a separate PR?
| cxxflags: -Wno-error=implicit-function-declaration | ||
| operating_system: $(source /etc/os-release && echo ${ID}${VERSION_ID}) | ||
|
|
||
| operating_system: $(. /etc/os-release && echo "${ID}${VERSION_ID}") |
There was a problem hiding this comment.
hey, I explicitly used source instead of . here in order to make this code less cryptic...
| mkdir -p "${OUTPUT_DIR}" | ||
| mkdir -p "${LOGS_DIR}" | ||
|
|
||
| # If a test fails, lit will ordinarily return a non-zero result, |
There was a problem hiding this comment.
Again, this goes beyond a spellcheck-induced refactor.
| if ${INTERACTIVE}; then | ||
| bash | ||
| fi | ||
|
|
There was a problem hiding this comment.
please remove an extra newline.
3 participants
[ATfL]: Update build.sh for: