Copilot commented on code in PR #64736: URL: https://github.com/apache/doris/pull/64736#discussion_r3458467275
########## build-support/selectdb-release-common.sh: ########## @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +selectdb_append_extra_module_spec() { + local var_name="$1" + local feature_name="$2" + local module_path="$3" + local current_value="${!var_name:-}" + + if [[ -n "${current_value}" ]]; then + current_value+="," + fi + current_value+="${feature_name}=${module_path}" + printf -v "${var_name}" '%s' "${current_value}" +} + +selectdb_configure_extra_modules() { + local enable_tde="$1" + local enable_tls="$2" + local enable_variant_nested_group="$3" + local enable_snapshot="$4" + + EXTRA_FE_MODULES="${EXTRA_FE_MODULES:-}" + EXTRA_BE_MODULES="${EXTRA_BE_MODULES:-}" + EXTRA_CLOUD_MODULES="${EXTRA_CLOUD_MODULES:-}" + + if [[ "${enable_tde}" -eq 1 ]]; then + selectdb_append_extra_module_spec EXTRA_FE_MODULES tde fe-enterprise/fe-tde + selectdb_append_extra_module_spec EXTRA_BE_MODULES tde enterprise/tde + fi + if [[ "${enable_tls}" -eq 1 ]]; then + selectdb_append_extra_module_spec EXTRA_FE_MODULES tls fe-enterprise/fe-tls + selectdb_append_extra_module_spec EXTRA_BE_MODULES tls enterprise/tls + selectdb_append_extra_module_spec EXTRA_CLOUD_MODULES tls enterprise/tls + fi + if [[ "${enable_variant_nested_group}" -eq 1 ]]; then + selectdb_append_extra_module_spec EXTRA_BE_MODULES variant-nested-group enterprise/variant-nested-group + fi + if [[ "${enable_snapshot}" -eq 1 ]]; then + selectdb_append_extra_module_spec EXTRA_FE_MODULES snapshot fe-enterprise/fe-snapshot + selectdb_append_extra_module_spec EXTRA_CLOUD_MODULES snapshot enterprise/snapshot + fi + + export EXTRA_FE_MODULES + export EXTRA_BE_MODULES + export EXTRA_CLOUD_MODULES +} + +selectdb_copy_extra_fe_libs() { + local output_root="${1:-output}" + local extra_module module_path target_dir jar_file jar_name + + if [[ -z "${EXTRA_FE_MODULES:-}" ]]; then + return + fi + + mkdir -p "${output_root}/fe/lib" + IFS=',' read -r -a extra_modules <<<"${EXTRA_FE_MODULES}" + for extra_module in "${extra_modules[@]}"; do + module_path="${extra_module#*=}" + target_dir="${DORIS_HOME}/fe/${module_path}/target" + + if [[ -d "${target_dir}/lib" ]]; then + cp -r -p "${target_dir}/lib/." "${output_root}/fe/lib/" + fi + + shopt -s nullglob + for jar_file in "${target_dir}"/*.jar; do + jar_name="$(basename "${jar_file}")" + case "${jar_name}" in + *-sources.jar | *-javadoc.jar | *-test-sources.jar | *tests.jar | original-*.jar) + continue + ;; + esac + cp -r -p "${jar_file}" "${output_root}/fe/lib/" + done + shopt -u nullglob Review Comment: `shopt -s nullglob` is a shell-global setting; with `set -e` in the calling scripts, any failure inside the loop (e.g., a failing `cp`) can exit before line 93, leaving `nullglob` enabled for the remainder of the build and potentially changing later glob behavior in hard-to-debug ways. Consider saving/restoring the previous `nullglob` state (or running the jar-glob loop in a subshell) so it is restored even on error; also, `cp -r -p` is meant for directories—use a non-recursive copy for individual jar files to avoid portability issues/warnings. ########## build-for-release-selectdb.sh: ########## @@ -0,0 +1,216 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +set -eo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)" +export DORIS_HOME="${ROOT}" + +. "${DORIS_HOME}/build-support/selectdb-release-common.sh" + +usage() { + echo " +Usage: $0 --vendor <vendor> --version <version> <options> + Optional options: + --noavx2 build without avx2 + --tar pack the output + --disable-tde disable TDE feature modules + --disable-tls disable TLS feature modules + --disable-variant-nested-group disable Variant NestedGroup feature module + --disable-snapshot disable Snapshot feature modules + --build_only build only without packaging + + Eg. + $0 --vendor selectdb --version 1.2.0 + $0 --vendor selectdb --version 1.2.0 --disable-tls + $0 --vendor selectdb --build_only --version 1.2.0 +" + exit 1 +} + +if ! OPTS="$(getopt \ + -n "$0" \ + -o '' \ + -l 'noavx2' \ + -l 'tar' \ + -l 'disable-tde' \ + -l 'disable-tls' \ + -l 'disable-variant-nested-group' \ + -l 'disable-snapshot' \ + -l 'version:' \ + -l 'vendor:' \ + -l 'help' \ + -l 'build_only' \ + -- "$@")"; then + usage +fi + +eval set -- "${OPTS}" + +_USE_AVX2=1 +TAR=0 +HELP=0 +VERSION= +VENDOR= +BUILD_ONLY=0 +ENABLE_TDE=1 +ENABLE_TLS=1 +ENABLE_VARIANT_NESTED_GROUP=1 +ENABLE_SNAPSHOT=1 + +while true; do + case "$1" in + --noavx2) + _USE_AVX2=0 + shift + ;; + --tar) + TAR=1 + shift + ;; + --disable-tde) + ENABLE_TDE=0 + shift + ;; + --disable-tls) + ENABLE_TLS=0 + shift + ;; + --disable-variant-nested-group) + ENABLE_VARIANT_NESTED_GROUP=0 + shift + ;; + --disable-snapshot) + ENABLE_SNAPSHOT=0 + shift + ;; + --version) + VERSION="$2" + shift 2 + ;; + --vendor) + VENDOR="$2" + shift 2 + ;; + --build_only) + BUILD_ONLY=1 + shift + ;; + --help) + HELP=1 + shift + ;; + --) + shift + break + ;; + *) + echo "Internal error" + exit 1 + ;; + esac +done + +if [[ "${HELP}" -eq 1 ]]; then + usage +fi +if [[ -z "${VENDOR}" ]]; then + echo "--vendor must be specified, choose one of selectdb, velodb" Review Comment: This message implies valid vendor values are restricted to `selectdb` and `velodb`, but the script only checks for non-empty `--vendor` and does not enforce an allowed set. Either validate the vendor against the stated allowed values (and fail if unsupported), or update the message to avoid implying constraints that don’t exist. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
