Copilot commented on code in PR #746: URL: https://github.com/apache/hugegraph-toolchain/pull/746#discussion_r3628742412
########## .codex/scripts/verify.sh: ########## @@ -0,0 +1,84 @@ +#!/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 -euo pipefail +unset CDPATH + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +REPO_ROOT=$(cd "${SCRIPT_DIR}/../.." && pwd) +WORKTREE=${CODEX_WORKTREE_PATH:-${REPO_ROOT}} +NODE_HOME=${HUBBLE_NODE_HOME:-"${HOME}/.nvm/versions/node/v18.20.8"} +if [[ -n "${JAVA11_HOME:-}" ]] && [[ -x "${JAVA11_HOME}/bin/java" ]]; then + JAVA_HOME=${JAVA11_HOME} +elif [[ -x /usr/libexec/java_home ]]; then + JAVA_HOME=$(/usr/libexec/java_home -v 11) +elif command -v java >/dev/null 2>&1 && \ + java -version 2>&1 | head -1 | grep -Eq 'version "11\.'; then + JAVA_HOME=$(cd "$(dirname "$(command -v java)")/.." && pwd) +else + echo "ERROR: Java 11 is required; set JAVA11_HOME" >&2 + exit 1 +fi Review Comment: `/usr/libexec/java_home -v 11` can exit non-zero (e.g., Java 11 not installed) and, due to `set -e`, will terminate the script before reaching the fallback checks/error message. Guard this call like the setup script does (redirect stderr and only accept the result when the command succeeds), so the script reliably emits the intended error when Java 11 is unavailable. ########## .codex/tests/environment_test.sh: ########## @@ -0,0 +1,185 @@ +#!/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 -euo pipefail +unset CDPATH + +CODEX_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +REPO_ROOT=$(cd "${CODEX_DIR}/.." && pwd) +TEST_TMP=$(mktemp -d "${TMPDIR:-/tmp}/toolchain-env-test.XXXXXX") +TEST_TMP=$(cd "${TEST_TMP}" && pwd) +trap 'rm -rf "${TEST_TMP}"' EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +assert_contains() { + local actual=$1 + local expected=$2 + [[ "${actual}" == *"${expected}"* ]] || + fail "expected '${actual}' to contain '${expected}'" +} + +test_environment_toml() { + python3 - "${CODEX_DIR}/environments/environment.toml" <<'PY' +import pathlib +import sys +import tomllib Review Comment: This test requires Python >= 3.11 (`tomllib`). On machines where `python3` is 3.10 or older, it will fail even if the Codex environment is otherwise usable. Consider either: (1) using a small fallback to `tomli` when `tomllib` is unavailable, or (2) explicitly checking the Python version/module availability and producing a clear skip/failure message describing the requirement. ########## .codex/scripts/setup.sh: ########## @@ -0,0 +1,109 @@ +#!/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 -euo pipefail +unset CDPATH + +WORKTREE=${CODEX_WORKTREE_PATH:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)} +NODE_HOME=${HUBBLE_NODE_HOME:-"${HOME}/.nvm/versions/node/v18.20.8"} +FE_DIR="${WORKTREE}/hugegraph-hubble/hubble-fe" +DRY_RUN=false + +if [[ "${1:-}" == "--dry-run" ]]; then + DRY_RUN=true +fi + +echo "[setup] worktree: ${WORKTREE}" +echo "[setup] require Node 18.20.8 from ${NODE_HOME}" + +if ${DRY_RUN}; then + echo "[setup] yarn install --frozen-lockfile --prefer-offline --non-interactive" + exit 0 +fi + +if [[ ! -x "${NODE_HOME}/bin/node" ]]; then + echo "ERROR: Node 18.20.8 is not installed at ${NODE_HOME}" >&2 + exit 1 +fi + +export PATH="${NODE_HOME}/bin:${PATH}" +[[ "$(node --version)" == "v18.20.8" ]] || { + echo "ERROR: expected Node v18.20.8, got $(node --version)" >&2 + exit 1 +} +command -v yarn >/dev/null || { + echo "ERROR: yarn is not available with Node 18.20.8" >&2 + exit 1 +} Review Comment: Node distributions typically do not include Yarn; requiring `yarn` to already exist globally can make the environment non-deterministic across machines. To better match the PR goal of a managed environment, consider using Corepack (`corepack enable` and invoking `corepack yarn ...`) and/or enforcing a pinned Yarn version (e.g., via the repo’s `packageManager` field) rather than assuming a preinstalled `yarn` binary. ########## .codex/scripts/hubble.sh: ########## @@ -0,0 +1,473 @@ +#!/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 -euo pipefail +unset CDPATH + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +REPO_ROOT=$(cd "${SCRIPT_DIR}/../.." && pwd) +WORKTREE=${CODEX_WORKTREE_PATH:-${REPO_ROOT}} +SOURCE_TREE=${CODEX_SOURCE_TREE_PATH:-${REPO_ROOT}} +NODE_HOME=${HUBBLE_NODE_HOME:-"${HOME}/.nvm/versions/node/v18.20.8"} +CODEX_HOME=${CODEX_HOME:-"${HOME}/.codex"} + +hash_text() { + printf '%s' "$1" | LC_ALL=C shasum -a 256 | awk '{print substr($1, 1, 12)}' +} Review Comment: `shasum` is not a guaranteed dependency on many Linux distributions by default (it’s often provided by a separate perl digest package). Since this hash is required for state directory layout, missing `shasum` will break all actions. Consider a small helper that prefers `sha256sum` when available and falls back to `shasum` (or `openssl dgst -sha256`) to improve portability. ########## .codex/scripts/infra.sh: ########## @@ -0,0 +1,191 @@ +#!/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 -euo pipefail +unset CDPATH + +SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +CODEX_DIR=$(cd "${SCRIPT_DIR}/.." && pwd) +REPO_ROOT=$(cd "${CODEX_DIR}/.." && pwd) +SOURCE_TREE=${CODEX_SOURCE_TREE_PATH:-${REPO_ROOT}} + +resolve_server_repo() { + if [[ -n "${HUGEGRAPH_SERVER_REPO:-}" ]]; then + echo "${HUGEGRAPH_SERVER_REPO}" + return + fi + local sibling + sibling=$(cd "${SOURCE_TREE}/.." 2>/dev/null && pwd)/server + if [[ -d "${sibling}/docker" ]]; then + echo "${sibling}" + return + fi + echo "ERROR: set HUGEGRAPH_SERVER_REPO or place server beside toolchain" >&2 + exit 1 +} + +compose_files() { + local profile=${1:-low} + local server_repo=$2 + COMPOSE_ARGS=( + -f "${server_repo}/docker/docker-compose.yml" + -f "${CODEX_DIR}/infra/compose.low-memory.yml" + ) + if [[ "${profile}" == "balanced" ]]; then + COMPOSE_ARGS+=(-f "${CODEX_DIR}/infra/compose.balanced.yml") + elif [[ "${profile}" != "low" ]]; then + echo "ERROR: unknown profile ${profile}" >&2 + exit 2 + fi +} + +print_compose_command() { + local profile=${1:-low} server_repo arg + server_repo=$(resolve_server_repo) + [[ -f "${server_repo}/docker/docker-compose.yml" ]] || { + echo "ERROR: missing server Compose file" >&2 + exit 1 + } + compose_files "${profile}" "${server_repo}" + printf 'TOOLCHAIN_CODEX_DIR=%q HUGEGRAPH_VERSION=latest docker compose' "${CODEX_DIR}" + for arg in "${COMPOSE_ARGS[@]}"; do printf ' %q' "${arg}"; done + printf ' up -d --pull never --wait\n' +} + +warn_dirty_server() { + local server_repo=$1 dirty + dirty=$(git -C "${server_repo}" status --short 2>/dev/null || true) + if [[ -n "${dirty}" ]]; then + echo "WARN: server repo has local changes; latest images do not include them:" >&2 + echo "${dirty}" | head -20 >&2 + fi +} + +assert_docker() { + docker info >/dev/null 2>&1 || { + echo "ERROR: Docker is not running" >&2 + exit 1 + } +} + +assert_port_available_for_compose() { + local port=$1 expected_container=$2 listener container + local project='' service='' mapping='' + listener=$(lsof -tiTCP:"${port}" -sTCP:LISTEN 2>/dev/null | head -1 || true) + [[ -z "${listener}" ]] && return + container=$(docker ps --filter "name=^/${expected_container}$" \ + --format '{{.ID}}' | head -1) + if [[ -n "${container}" ]]; then + project=$(docker inspect --format \ + '{{index .Config.Labels "com.docker.compose.project"}}' "${container}") + service=$(docker inspect --format \ + '{{index .Config.Labels "com.docker.compose.service"}}' "${container}") + mapping=$(docker port "${container}" "${port}/tcp" 2>/dev/null || true) + fi + if [[ -z "${container}" || "${project}" != hugegraph-single || \ + "${service}" != "${expected_container#hg-}" || \ + "${mapping}" != *":${port}"* ]]; then + echo "ERROR: port ${port} is owned by non-Compose pid ${listener}" >&2 + exit 1 + fi Review Comment: This hard-codes the expected Compose project name (`hugegraph-single`). If users run the stack with a different project name (e.g., `COMPOSE_PROJECT_NAME` or a different directory name), the script can incorrectly fail even when the port is owned by the correct Compose-managed container. Consider accepting any Compose project (only validate the service label), or allowing the expected project name to be overridden via an environment variable. -- 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]
