Skip to content

Commit

Permalink
[yugabyte#13840] Do not error out if the LLVM URL in thirdparty is di…
Browse files Browse the repository at this point in the history
…fferent than in llvm_installer

Summary:
If the third-party dependencies specify a LLVM package URL different from the one determined by llvm_installer, we should log a warning but let the build proceed.

Also fix a number of insecure variable evaluations in shell scripts when trying to check if some boolean variable is true.

Test Plan: Jenkins

Reviewers: neil, steve.varnau

Reviewed By: steve.varnau

Subscribers: jenkins-bot, ybase, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D19310
  • Loading branch information
mbautin committed Sep 11, 2022
1 parent 1182be1 commit 820bfee
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 96 deletions.
16 changes: 8 additions & 8 deletions bin/repeat_unit_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ set_build_root
set_sanitizer_runtime_options

declare -i -r num_pos_args=${#positional_args[@]}
if "$is_java_test"; then
if [[ ${is_java_test} == "true" ]]; then
if [[ $num_pos_args -ne 2 ]]; then
fatal "Expected two positional arguments for Java tests, not including build type:" \
"<maven_module_name> <class_name_with_package>"
Expand Down Expand Up @@ -254,7 +254,7 @@ if [[ $iteration -gt 0 ]]; then
# One iteration with a specific "id" ($iteration).
test_log_path_prefix=$log_dir/$iteration
test_log_path=$test_log_path_prefix.log
if "$is_java_test"; then
if [[ ${is_java_test} == "true" ]]; then
export YB_SUREFIRE_REPORTS_DIR=$test_log_path_prefix.reports
fi
export YB_FATAL_DETAILS_PATH_PREFIX=$test_log_path_prefix.fatal_failure_details
Expand All @@ -269,7 +269,7 @@ if [[ $iteration -gt 0 ]]; then
fi

# TODO: deduplicate the setup here against run_one_cxx_test() in common-test-env.sh.
if "$is_java_test"; then
if [[ ${is_java_test} == "true" ]]; then
test_wrapper_cmd_line=(
"$YB_BUILD_SUPPORT_DIR"/run-test.sh "${positional_args[@]}"
)
Expand All @@ -287,7 +287,7 @@ if [[ $iteration -gt 0 ]]; then
start_time_sec=$( date +%s )
(
cd "$TEST_TMPDIR"
if "$verbose"; then
if [[ ${verbose} == "true" ]]; then
log "Iteration $iteration logging to $test_log_path"
fi
ulimit -c unlimited
Expand Down Expand Up @@ -316,9 +316,9 @@ if [[ $iteration -gt 0 ]]; then
keep_log=true
fi
fi
if "$keep_log"; then
if [[ ${keep_log} == "true" ]]; then
if ! "$skip_log_compression"; then
if "$is_java_test"; then
if [[ ${is_java_test} == "true" ]]; then
# Compress Java test log.
mv "$test_log_path" "$YB_SUREFIRE_REPORTS_DIR"
pushd "$log_dir"
Expand Down Expand Up @@ -362,7 +362,7 @@ if [[ $iteration -gt 0 ]]; then
comment+="; test log path: $test_log_path"
else
rm -f "$test_log_path"
if "$is_java_test"; then
if [[ ${is_java_test} == "true" ]]; then
set +e
rm -rf "$YB_SUREFIRE_REPORTS_DIR"
set -e
Expand Down Expand Up @@ -397,7 +397,7 @@ else
log "$gtest_filter_info"
if [[ -n ${YB_EXTRA_GTEST_FLAGS:-} ]]; then
log "Extra test flags from YB_EXTRA_GTEST_FLAGS: $YB_EXTRA_GTEST_FLAGS"
elif "$verbose"; then
elif [[ ${verbose} == "true" ]]; then
log "YB_EXTRA_GTEST_FLAGS is not set"
fi
log "Saving repeated test execution logs to: $log_dir"
Expand Down
102 changes: 56 additions & 46 deletions build-support/common-build-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -283,27 +283,27 @@ is_clean_build=false
yb_thirdparty_dir_origin=""

if [[ -n ${YB_THIRDPARTY_DIR:-} ]]; then
yb_thirdparty_dir_origin=" (from environment)"
yb_thirdparty_dir_origin="from environment"
fi

yb_thirdparty_url_origin=""
if [[ -n ${YB_THIRDPARTY_URL:-} ]]; then
yb_thirdparty_url_origin=" (from environment)"
yb_thirdparty_url_origin="from environment"
fi

yb_linuxbrew_dir_origin=""
if [[ -n ${YB_LINUXBREW_DIR:-} ]]; then
yb_linuxbrew_dir_origin=" (from environment)"
yb_linuxbrew_dir_origin="from environment"
fi

yb_llvm_toolchain_url_origin=""
if [[ -n ${YB_LLVM_TOOLCHAIN_URL:-} ]]; then
yb_llvm_toolchain_url_origin=" (from environment)"
yb_llvm_toolchain_url_origin="from environment"
fi

yb_llvm_toolchain_dir_origin=""
if [[ -n ${YB_LLVM_TOOLCHAIN_DIR:-} ]]; then
yb_llvm_toolchain_dir_origin=" (from environment)"
yb_llvm_toolchain_dir_origin="from environment"
fi

# To deduplicate Maven arguments
Expand Down Expand Up @@ -395,7 +395,7 @@ set_build_root() {

normalize_build_root

if "$make_build_root_readonly"; then
if [[ ${make_build_root_readonly} == "true" ]]; then
readonly BUILD_ROOT
fi

Expand Down Expand Up @@ -714,7 +714,7 @@ create_mvn_repo_path_file() {
}

set_mvn_parameters() {
if "$yb_mvn_parameters_already_set"; then
if [[ ${yb_mvn_parameters_already_set} == "true" ]]; then
return
fi
if is_jenkins; then
Expand Down Expand Up @@ -1217,7 +1217,7 @@ download_thirdparty() {
"'$extracted_dir'"
fi
export YB_THIRDPARTY_DIR=$extracted_dir
yb_thirdparty_dir_origin=" (downloaded from $YB_THIRDPARTY_URL)"
yb_thirdparty_dir_origin="downloaded from $YB_THIRDPARTY_URL"
save_thirdparty_info_to_build_dir
download_toolchain
}
Expand Down Expand Up @@ -1251,6 +1251,7 @@ download_toolchain() {
if [[ -z ${YB_LLVM_TOOLCHAIN_URL:-} &&
${YB_COMPILER_TYPE:-} =~ ^clang[0-9]+$ ]] && is_linux; then
YB_LLVM_TOOLCHAIN_URL=$(
activate_virtualenv &>/dev/null
python3 -m llvm_installer --print-url "--llvm-major-version=${YB_COMPILER_TYPE#clang}"
)
if [[ ${YB_LLVM_TOOLCHAIN_URL} != https://* ]]; then
Expand Down Expand Up @@ -1292,20 +1293,24 @@ download_toolchain() {
"'$extracted_dir'"
fi
export YB_LINUXBREW_DIR=$extracted_dir
yb_linuxbrew_dir_origin=" (downloaded from $toolchain_url)"
yb_linuxbrew_dir_origin="downloaded from $toolchain_url"
save_brew_path_to_build_dir
fi

if [[ ${is_llvm} == "true" ]]; then
if [[ -n ${YB_LLVM_TOOLCHAIN_DIR:-} &&
$YB_LLVM_TOOLCHAIN_DIR != "$extracted_dir" ]]; then
$YB_LLVM_TOOLCHAIN_DIR != "$extracted_dir" &&
${YB_LLVM_TOOLCHAIN_DIR_MISMATCH_WARNING_LOGGED:-0} == "0" ]]; then
log_thirdparty_and_toolchain_details
fatal "YB_LLVM_TOOLCHAIN_DIR is already set to '$YB_LLVM_TOOLCHAIN_DIR', cannot set it " \
"to '$extracted_dir'"
log "Warning: YB_LLVM_TOOLCHAIN_DIR is already set to '$YB_LLVM_TOOLCHAIN_DIR', cannot " \
"set it to '$extracted_dir'. This may happen in case the LLVM toolchain version used " \
"to built third-party dependencies is different from the most recent one for the " \
"same major version that we have just picked to build YugabyteDB with."
export YB_LLVM_TOOLCHAIN_DIR_MISMATCH_WARNING_LOGGED=1
fi
export YB_LLVM_TOOLCHAIN_DIR=$extracted_dir
yb_llvm_toolchain_dir_origin=" (downloaded from $toolchain_url)"
save_llvm_toolchain_path_to_build_dir
yb_llvm_toolchain_dir_origin="downloaded from $toolchain_url"
save_llvm_toolchain_info_to_build_dir
fi
done
}
Expand Down Expand Up @@ -1375,9 +1380,10 @@ save_brew_path_to_build_dir() {
fi
}

save_llvm_toolchain_path_to_build_dir() {
save_llvm_toolchain_info_to_build_dir() {
if is_linux; then
save_var_to_file_in_build_dir "${YB_LLVM_TOOLCHAIN_DIR:-}" "llvm_path.txt"
save_var_to_file_in_build_dir "${YB_LLVM_TOOLCHAIN_URL:-}" "llvm_url.txt"
fi
}

Expand All @@ -1386,10 +1392,10 @@ save_thirdparty_info_to_build_dir() {
save_var_to_file_in_build_dir "${YB_THIRDPARTY_URL:-}" "thirdparty_url.txt"
}

save_paths_to_build_dir() {
save_paths_and_archive_urls_to_build_dir() {
save_brew_path_to_build_dir
save_thirdparty_info_to_build_dir
save_llvm_toolchain_path_to_build_dir
save_llvm_toolchain_info_to_build_dir
}

detect_linuxbrew() {
Expand All @@ -1411,7 +1417,7 @@ detect_linuxbrew() {
then
YB_LINUXBREW_DIR=$(<"$BUILD_ROOT/linuxbrew_path.txt")
export YB_LINUXBREW_DIR
yb_linuxbrew_dir_origin=" (from file '$BUILD_ROOT/linuxbrew_path.txt')"
yb_linuxbrew_dir_origin="from file '$BUILD_ROOT/linuxbrew_path.txt')"
return
fi
}
Expand All @@ -1425,7 +1431,7 @@ detect_llvm_toolchain() {
if [[ $is_clean_build != "true" && -n ${BUILD_ROOT:-} && -f $BUILD_ROOT/llvm_path.txt ]]; then
YB_LLVM_TOOLCHAIN_DIR=$(<"$BUILD_ROOT/llvm_path.txt")
export YB_LLVM_TOOLCHAIN_DIR
yb_llvm_toolchain_dir_origin=" (from file '$BUILD_ROOT/llvm_path.txt')"
yb_llvm_toolchain_dir_origin="from file '$BUILD_ROOT/llvm_path.txt')"
fi
}

Expand Down Expand Up @@ -1583,7 +1589,7 @@ get_build_worker_list() {
break
fi
done
if "$all_worker_names_valid"; then
if [[ ${all_worker_names_valid} == "true" ]]; then
return
fi
fi
Expand Down Expand Up @@ -1715,7 +1721,6 @@ debugging_remote_compilation() {
}

cmd_line_to_env_vars_for_remote_cmd() {
declare -i i=1
YB_ENCODED_REMOTE_CMD_LINE=""
# This must match the separator in remote_cmd.sh.
declare -r ARG_SEPARATOR=$'=:\t:='
Expand Down Expand Up @@ -1851,7 +1856,7 @@ find_or_download_thirdparty() {
"'$BUILD_ROOT/thirdparty_path.txt' contains '$thirdparty_dir_from_file'"
fi
export YB_THIRDPARTY_DIR=$thirdparty_dir_from_file
yb_thirdparty_dir_origin=" (from file '$BUILD_ROOT/thirdparty_path.txt')"
yb_thirdparty_dir_origin="from file '$BUILD_ROOT/thirdparty_path.txt')"

# Check if we've succeeded in setting YB_THIRDPARTY_DIR now.
if [[ -n ${YB_THIRDPARTY_DIR:-} ]]; then
Expand All @@ -1871,7 +1876,7 @@ find_or_download_thirdparty() {
"'$BUILD_ROOT/thirdparty_url.txt' contains '$thirdparty_url_from_file'"
fi
export YB_THIRDPARTY_URL=$thirdparty_url_from_file
yb_thirdparty_url_origin=" (from file '$BUILD_ROOT/thirdparty_url.txt')"
yb_thirdparty_url_origin="from file '$BUILD_ROOT/thirdparty_url.txt')"
if [[ ${YB_DOWNLOAD_THIRDPARTY:-} == "0" ]]; then
fatal "YB_DOWNLOAD_THIRDPARTY is explicitly set to 0 but file" \
"$BUILD_ROOT/thirdparty_url.txt exists"
Expand All @@ -1895,7 +1900,7 @@ find_or_download_thirdparty() {

if [[ -z ${YB_THIRDPARTY_DIR:-} ]]; then
export YB_THIRDPARTY_DIR=$YB_SRC_ROOT/thirdparty
yb_thirdparty_dir_origin=" (default)"
yb_thirdparty_dir_origin="default"
fi
save_thirdparty_info_to_build_dir
}
Expand All @@ -1920,28 +1925,33 @@ find_or_download_ysql_snapshots() {
done
}

log_env_var() {
expect_num_args 2 "$@"
local env_var_name=$1
local env_var_value=${!env_var_name:-}
if [[ -z ${env_var_value} ]]; then
return
fi
local description=$2
if [[ -n ${description} ]]; then
description=" (${description})"
fi
echo " ${env_var_name}: ${env_var_value}${description}"
}

log_thirdparty_and_toolchain_details() {
(
echo "Details of third-party dependencies:"
echo " YB_THIRDPARTY_DIR: ${YB_THIRDPARTY_DIR:-undefined}$yb_thirdparty_dir_origin"
if is_linux && [[ -n ${YB_LINUXBREW_DIR:-} ]]; then
echo " YB_LINUXBREW_DIR: $YB_LINUXBREW_DIR$yb_linuxbrew_dir_origin"
fi
if [[ -n ${YB_LLVM_TOOLCHAIN_URL:-} ]]; then
echo " YB_LLVM_TOOLCHAIN_URL: $YB_LLVM_TOOLCHAIN_URL$yb_llvm_toolchain_url_origin"
fi
if [[ -n ${YB_LLVM_TOOLCHAIN_DIR:-} ]]; then
echo " YB_LLVM_TOOLCHAIN_DIR: $YB_LLVM_TOOLCHAIN_DIR$yb_llvm_toolchain_dir_origin"
fi
if [[ -n ${YB_THIRDPARTY_URL:-} ]]; then
echo " YB_THIRDPARTY_URL: $YB_THIRDPARTY_URL$yb_thirdparty_url_origin"
fi
if [[ -n ${YB_DOWNLOAD_THIRDPARTY:-} ]]; then
echo " YB_DOWNLOAD_THIRDPARTY: $YB_DOWNLOAD_THIRDPARTY"
fi
if [[ -n ${NO_REBUILD_THIRDPARTY:-} ]]; then
echo " NO_REBUILD_THIRDPARTY: ${NO_REBUILD_THIRDPARTY}"
log_env_var YB_THIRDPARTY_DIR "${yb_thirdparty_dir_origin}"
log_env_var YB_THIRDPARTY_URL "${yb_thirdparty_url_origin}"

if is_linux; then
log_env_var YB_LINUXBREW_DIR "${yb_linuxbrew_dir_origin}"
fi
log_env_var YB_LLVM_TOOLCHAIN_URL "${yb_llvm_toolchain_url_origin}"
log_env_var YB_LLVM_TOOLCHAIN_DIR "${yb_llvm_toolchain_dir_origin}"
log_env_var YB_DOWNLOAD_THIRDPARTY ""
log_env_var NO_REBUILD_THIRDPARTY ""
) >&2
}

Expand Down Expand Up @@ -2229,7 +2239,7 @@ activate_virtualenv() {
fi

if [[ ! -d $virtualenv_dir ]]; then
if "$yb_readonly_virtualenv"; then
if [[ ${yb_readonly_virtualenv} == "true" ]]; then
fatal "virtualenv does not exist at '$virtualenv_dir', and we are not allowed to create it"
fi
if [[ -n ${VIRTUAL_ENV:-} && -f $VIRTUAL_ENV/bin/activate ]]; then
Expand Down Expand Up @@ -2438,7 +2448,7 @@ run_with_retries() {
debug_log_boolean_function_result() {
expect_num_args 1 "$@"
local fn_name=$1
if "$fn_name"; then
if [[ ${fn_name} == "true" ]]; then
log "$fn_name is true"
else
log "$fn_name is false"
Expand Down Expand Up @@ -2489,7 +2499,7 @@ set_prebuilt_thirdparty_url() {
"${thirdparty_tool_cmd_line[@]}"
YB_THIRDPARTY_URL=$(<"$BUILD_ROOT/thirdparty_url.txt")
export YB_THIRDPARTY_URL
yb_thirdparty_url_origin=" (determined automatically based on the OS and compiler type)"
yb_thirdparty_url_origin="determined automatically based on the OS and compiler type"
if [[ -z $YB_THIRDPARTY_URL ]]; then
fatal "Could not automatically determine the third-party archive URL to download."
fi
Expand All @@ -2499,7 +2509,7 @@ set_prebuilt_thirdparty_url() {
if [[ -f $llvm_url_file_path ]]; then
YB_LLVM_TOOLCHAIN_URL=$(<"$llvm_url_file_path")
export YB_LLVM_TOOLCHAIN_URL
yb_llvm_toolchain_url_origin=" (determined automatically based on the OS and compiler type)"
yb_llvm_toolchain_url_origin="determined automatically based on the OS and compiler type"
log "Setting LLVM toolchain URL to $YB_LLVM_TOOLCHAIN_URL"
save_var_to_file_in_build_dir "$YB_LLVM_TOOLCHAIN_URL" llvm_url.txt
fi
Expand Down
8 changes: 4 additions & 4 deletions build-support/common-test-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ prepare_for_running_cxx_test() {
mkdir_safe "$test_dir"

rel_test_log_path_prefix="$test_binary_sanitized"
if "$run_at_once"; then
if [[ ${run_at_once} == "true" ]]; then
# Make this similar to the case when we run tests separately. Pretend that the test binary name
# is the test name.
rel_test_log_path_prefix+="/${rel_test_binary##*/}"
Expand Down Expand Up @@ -802,7 +802,7 @@ handle_cxx_test_xml_output() {
log "Process tree supervisor reported that the test failed"
test_failed=true
fi
if "$test_failed"; then
if [[ ${test_failed} == "true" ]]; then
log "Test failed, updating $xml_output_file"
else
log "Test succeeded, updating $xml_output_file"
Expand Down Expand Up @@ -962,7 +962,7 @@ handle_cxx_test_failure() {
test_failed=true
fi

if "$test_failed"; then
if [[ ${test_failed} == "true" ]]; then
(
rewrite_test_log "${test_log_path}"
echo
Expand Down Expand Up @@ -2083,7 +2083,7 @@ run_cmake_unit_tests() {
fi
done
done
if "$error"; then
if [[ ${error} == "true" ]]; then
fatal "Found some disallowed patterns in CMake files"
else
log "Validated ${#cmake_files[@]} CMake files using light-weight grep checks"
Expand Down
Loading

0 comments on commit 820bfee

Please sign in to comment.