mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Keep Bazel startup options stable across commands (#26256)
## Why `just bazel-clippy` ran target discovery with `--noexperimental_remote_repo_contents_cache`, then ran the build with the workspace default `--experimental_remote_repo_contents_cache`. Bazel therefore killed and restarted its server on each transition, slowing repeated commands and discarding the in-memory analysis cache. An audit found the same class of startup-option variation in several CI command sequences. ## What changed - Keep local lint target-discovery queries on the workspace-default Bazel server, while making CI target discovery explicitly use the CI startup options. - Normalize GitHub Actions launches through the BuildBuddy wrapper to share `BAZEL_OUTPUT_USER_ROOT` and `--noexperimental_remote_repo_contents_cache`. - Route the CI lockfile check and Windows test-shard query through the same startup configuration. - Document the startup-option invariant and add wrapper regression coverage. ## Validation - Confirmed consecutive local clippy target-discovery runs retained the same Bazel server PID.
This commit is contained in:
committed by
GitHub
Unverified
parent
4de7a2b9d8
commit
1d9c9c9f33
@@ -4,7 +4,8 @@ set -euo pipefail
|
||||
|
||||
# Run target-discovery queries with the same startup settings as the main
|
||||
# build/test invocation so they can reuse the same Bazel server. Queries only
|
||||
# enumerate labels, so they intentionally do not select CI or remote configs.
|
||||
# enumerate labels, so they intentionally do not select a CI build/test config
|
||||
# or remote execution.
|
||||
|
||||
if [[ $# -lt 2 || "${@: -2:1}" != "--" ]]; then
|
||||
echo "Usage: $0 [<bazel query args>...] -- <query expression>" >&2
|
||||
@@ -14,21 +15,16 @@ fi
|
||||
query_args=("${@:1:$#-2}")
|
||||
query_expression="${@: -1}"
|
||||
|
||||
bazel_startup_args=()
|
||||
if [[ -n "${BAZEL_OUTPUT_USER_ROOT:-}" ]]; then
|
||||
bazel_startup_args+=("--output_user_root=${BAZEL_OUTPUT_USER_ROOT}")
|
||||
fi
|
||||
|
||||
run_bazel() {
|
||||
if [[ "${RUNNER_OS:-}" == "Windows" ]]; then
|
||||
MSYS2_ARG_CONV_EXCL='*' bazel "$@"
|
||||
MSYS2_ARG_CONV_EXCL='*' "$(dirname "${BASH_SOURCE[0]}")/run_bazel_with_buildbuddy.py" "$@"
|
||||
return
|
||||
fi
|
||||
|
||||
bazel "$@"
|
||||
"$(dirname "${BASH_SOURCE[0]}")/run_bazel_with_buildbuddy.py" "$@"
|
||||
}
|
||||
|
||||
bazel_query_args=(--noexperimental_remote_repo_contents_cache query)
|
||||
bazel_query_args=(query)
|
||||
|
||||
if [[ -n "${BAZEL_REPO_CONTENTS_CACHE:-}" ]]; then
|
||||
bazel_query_args+=("--repo_contents_cache=${BAZEL_REPO_CONTENTS_CACHE}")
|
||||
@@ -43,8 +39,4 @@ if (( ${#query_args[@]} > 0 )); then
|
||||
fi
|
||||
bazel_query_args+=("$query_expression")
|
||||
|
||||
if (( ${#bazel_startup_args[@]} > 0 )); then
|
||||
run_bazel "${bazel_startup_args[@]}" "${bazel_query_args[@]}"
|
||||
else
|
||||
run_bazel "${bazel_query_args[@]}"
|
||||
fi
|
||||
run_bazel "${bazel_query_args[@]}"
|
||||
|
||||
@@ -22,6 +22,47 @@ REMOTE_EXECUTION_CONFIGS = {
|
||||
"--config=ci-v8",
|
||||
"--config=ci-windows-cross",
|
||||
}
|
||||
# Honor either explicit setting so the wrapper never overrides the caller's
|
||||
# choice when it supplies the CI default below.
|
||||
REMOTE_REPO_CONTENTS_CACHE_STARTUP_OPTIONS = {
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"--noexperimental_remote_repo_contents_cache",
|
||||
}
|
||||
|
||||
|
||||
def startup_args(args: Sequence[str], env: Mapping[str, str]) -> list[str]:
|
||||
"""Return shared startup options that are missing from a Bazel invocation.
|
||||
|
||||
Bazel startup options must precede the command, and changing them restarts
|
||||
the server and discards its analysis cache. GitHub Actions invokes Bazel
|
||||
through several helpers, so normalize their startup options here while
|
||||
preserving any explicit choice made by the caller.
|
||||
"""
|
||||
command_idx = next(
|
||||
(idx for idx, arg in enumerate(args) if not arg.startswith("-")),
|
||||
len(args),
|
||||
)
|
||||
configured_startup_args = args[:command_idx]
|
||||
injected_args = []
|
||||
|
||||
output_user_root = env.get("BAZEL_OUTPUT_USER_ROOT")
|
||||
if output_user_root and not any(
|
||||
arg.startswith("--output_user_root=") for arg in configured_startup_args
|
||||
):
|
||||
injected_args.append(f"--output_user_root={output_user_root}")
|
||||
|
||||
if env.get("GITHUB_ACTIONS") == "true" and not any(
|
||||
arg in REMOTE_REPO_CONTENTS_CACHE_STARTUP_OPTIONS
|
||||
for arg in configured_startup_args
|
||||
):
|
||||
# Work around Bazel 9 overlay materialization failures seen in CI. This
|
||||
# disables only the startup-level repo contents cache; keyed runs still
|
||||
# use BuildBuddy.
|
||||
injected_args.append("--noexperimental_remote_repo_contents_cache")
|
||||
|
||||
return injected_args
|
||||
|
||||
|
||||
# Only authenticated workflow runs executing trusted upstream code may use the
|
||||
# OpenAI BuildBuddy host. A pull request event without proof that its head is
|
||||
# in the upstream repository fails closed to the generic host.
|
||||
@@ -114,7 +155,7 @@ def bazel_args_with_remote_config(
|
||||
def bazel_command(*args: str, env: Mapping[str, str] | None = None) -> list[str]:
|
||||
env = os.environ if env is None else env
|
||||
bazel = env.get("CODEX_BAZEL_BIN", "bazel")
|
||||
return [bazel, *bazel_args_with_remote_config(args, env)]
|
||||
return [bazel, *startup_args(args, env), *bazel_args_with_remote_config(args, env)]
|
||||
|
||||
|
||||
def main() -> None:
|
||||
|
||||
@@ -182,6 +182,38 @@ class RunBazelWithBuildBuddyTest(unittest.TestCase):
|
||||
["fake-bazel", "info", "execution_root"],
|
||||
)
|
||||
|
||||
def test_bazel_command_normalizes_github_actions_startup_options(self) -> None:
|
||||
env = {
|
||||
"BAZEL_OUTPUT_USER_ROOT": "/tmp/bazel-output",
|
||||
"GITHUB_ACTIONS": "true",
|
||||
}
|
||||
|
||||
self.assertEqual(
|
||||
run_bazel_with_buildbuddy.bazel_command("build", "//codex-rs/...", env=env),
|
||||
[
|
||||
"bazel",
|
||||
"--output_user_root=/tmp/bazel-output",
|
||||
"--noexperimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
],
|
||||
)
|
||||
self.assertEqual(
|
||||
run_bazel_with_buildbuddy.bazel_command(
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
env=env,
|
||||
),
|
||||
[
|
||||
"bazel",
|
||||
"--output_user_root=/tmp/bazel-output",
|
||||
"--experimental_remote_repo_contents_cache",
|
||||
"build",
|
||||
"//codex-rs/...",
|
||||
],
|
||||
)
|
||||
|
||||
def test_main_preserves_spaced_argument_and_child_exit_status(self) -> None:
|
||||
spaced_arg = (
|
||||
r"--test_env=PATH=C:\Program Files\PowerShell\7;C:\Program Files\Git\bin"
|
||||
@@ -191,7 +223,9 @@ class RunBazelWithBuildBuddyTest(unittest.TestCase):
|
||||
)
|
||||
env = os.environ.copy()
|
||||
env["CODEX_BAZEL_BIN"] = sys.executable
|
||||
env.pop("BAZEL_OUTPUT_USER_ROOT", None)
|
||||
env.pop("BUILDBUDDY_API_KEY", None)
|
||||
env.pop("GITHUB_ACTIONS", None)
|
||||
|
||||
result = subprocess.run(
|
||||
[
|
||||
|
||||
@@ -186,7 +186,7 @@ jobs:
|
||||
|
||||
bazel_test_query='tests(//...) except tests(//third_party/v8:all) except attr(tags, "manual", tests(//...))'
|
||||
mapfile -t bazel_targets < <(
|
||||
MSYS2_ARG_CONV_EXCL='*' bazel query --output=label "${bazel_test_query}" \
|
||||
./.github/scripts/run-bazel-query-ci.sh --output=label -- "${bazel_test_query}" \
|
||||
| LC_ALL=C sort
|
||||
)
|
||||
|
||||
|
||||
@@ -81,7 +81,10 @@ GitHub Actions routes Bazel build and output-resolution commands through
|
||||
`.github/scripts/run-bazel-ci.sh` and `.github/scripts/rusty_v8_bazel.py`
|
||||
delegate remote configuration selection to that wrapper. The wrapper reads the
|
||||
GitHub Actions repository and event payload rather than relying on workflow
|
||||
files to duplicate tenant-selection logic.
|
||||
files to duplicate tenant-selection logic. It also normalizes GitHub Actions
|
||||
startup options so all Bazel launches in a job reuse the same server and
|
||||
in-memory analysis cache. Target-discovery and lockfile helpers delegate to the
|
||||
same wrapper so their callers do not need to select CI-specific startup options.
|
||||
|
||||
Loading-phase target-discovery `bazel query` commands run locally because they
|
||||
only enumerate labels and do not need remote caches or execution.
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
if ! bazel mod deps --lockfile_mode=error; then
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
if ! "${repo_root}/.github/scripts/run_bazel_with_buildbuddy.py" mod deps --lockfile_mode=error; then
|
||||
echo "MODULE.bazel.lock is out of date."
|
||||
echo "Run 'just bazel-lock-update' and commit the updated lockfile."
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user