Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions .github/workflows/bazel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ jobs:
run: |
bash tools/ci/test-bazel-cache-upload-mode
bash tools/ci/test-bazel-remote-probe
bash tools/ci/test-bazel-lock-touches-java
python3 tools/ci/test-bazel-lock-touches-java-properties

- name: Compute changed subtrees
id: detect
Expand Down Expand Up @@ -195,12 +197,21 @@ jobs:

run_all=false
changed=""
# The endpoints the path list was derived from. The content checks
# below must compare exactly these two revisions, not re-derive a
# range of their own, or they could answer about a different diff
# than the one that selected the rows.
diff_base=""
diff_head=""
case "$EVENT" in
workflow_dispatch) run_all=true ;;
pull_request)
if ! changed=$(git diff --name-only "origin/${BASE_REF}...HEAD" 2>/dev/null); then
echo "diff vs origin/${BASE_REF} failed; running full matrix"
run_all=true
else
diff_base=$(git merge-base "origin/${BASE_REF}" HEAD 2>/dev/null || true)
diff_head=$(git rev-parse HEAD 2>/dev/null || true)
fi
;;
merge_group)
Expand All @@ -219,6 +230,9 @@ jobs:
elif ! changed=$(git diff --name-only "${MERGE_BASE_SHA}...${HEAD_SHA}" 2>/dev/null); then
echo "diff ${MERGE_BASE_SHA}...${HEAD_SHA} failed; running full matrix"
run_all=true
else
diff_base=$(git merge-base "${MERGE_BASE_SHA}" "${HEAD_SHA}" 2>/dev/null || true)
diff_head="${HEAD_SHA}"
fi
;;
*)
Expand All @@ -227,6 +241,9 @@ jobs:
elif ! changed=$(git diff --name-only "${BEFORE_SHA}..${HEAD_SHA}" 2>/dev/null); then
echo "diff ${BEFORE_SHA}..${HEAD_SHA} failed (rewritten history?); running full matrix"
run_all=true
else
diff_base="${BEFORE_SHA}"
diff_head="${HEAD_SHA}"
fi
;;
esac
Expand Down Expand Up @@ -274,15 +291,70 @@ jobs:
tools/bazel/java/
tools/ci/stage-bazel-java-artifacts
)
# Two of these globs fire far more often than they carry Java
# meaning, so a path hit on them is confirmed against content before
# it schedules all seven Java rows.
#
# NVIDIA/nvcf#777 is the case: its whole lock delta was one Rust
# crate_universe extension and its MODULE.bazel delta was a single
# blank line, yet it scheduled every Java row. Those rows run on
# GitHub-hosted runners at max-parallel 4 and were the queue entry's
# long pole, cloud-functions alone taking 19 minutes, rebuilding from
# near scratch because a repin invalidates external repository state.
#
# Both checks fail closed. Without a usable diff range, or on any
# error, the path hit stands and Java runs.
java_content_gated() {
case "$1" in MODULE.bazel|MODULE.bazel.lock) return 0 ;; *) return 1 ;; esac
}
java_hit_is_real() {
local f="$1"
if [ -z "$diff_base" ] || [ -z "$diff_head" ]; then
echo " no diff range for $f; treating as Java-relevant"
return 0
fi
case "$f" in
MODULE.bazel)
# Whitespace and blank-line edits cannot change resolution.
if git diff --quiet --ignore-all-space --ignore-blank-lines \
"$diff_base" "$diff_head" -- MODULE.bazel 2>/dev/null; then
echo " MODULE.bazel changed only in whitespace; not Java-relevant"
return 1
fi
return 0
;;
MODULE.bazel.lock)
local verdict
verdict=$(python3 tools/ci/bazel-lock-touches-java "$diff_base" "$diff_head" 2>&1 >/dev/null || true)
[ -n "$verdict" ] && echo " $verdict"
if [ "$(python3 tools/ci/bazel-lock-touches-java "$diff_base" "$diff_head" 2>/dev/null)" = "false" ]; then
return 1
fi
return 0
;;
esac
return 0
}

java_shared_changed=false
if [ "$run_all" != "true" ]; then
for g in "${JAVA_SHARED_GLOBS[@]}"; do
# The prefix test means the MODULE.bazel glob also matches
# MODULE.bazel.lock, so match the gated files exactly and let the
# lock be judged by its own entry.
if java_content_gated "$g"; then
printf '%s\n' "$changed" | grep -qxF "$g" || continue
java_hit_is_real "$g" || continue
java_shared_changed=true
break
fi
if printf '%s\n' "$changed" | awk -v p="$g" 'index($0, p) == 1 { f = 1 } END { exit !f }'; then
java_shared_changed=true
break
fi
done
fi
echo "java_shared_changed=$java_shared_changed"

# Rows with ci_lane=docker-host own requires-docker (Testcontainers)
# tests. They run directly on the GitHub host where a Docker daemon is
Expand Down
134 changes: 134 additions & 0 deletions tools/ci/bazel-lock-touches-java
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
#!/usr/bin/env python3
# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
"""Decide whether a MODULE.bazel.lock change can affect the Java build.

Prints "true" or "false". Reads the two lock revisions from git.

bazel-lock-touches-java <base-ref> <head-ref>

MODULE.bazel.lock is in JAVA_SHARED_GLOBS, so any edit to it schedules all
seven Java rows. Those rows run on GitHub-hosted runners at max-parallel 4 and
are the long pole of a merge queue entry: on NVIDIA/nvcf#777, cloud-functions
alone took 19 minutes. That pull request's entire lock delta was one Rust
crate_universe extension, and the Java rows it scheduled rebuilt from near
scratch (168 remote cache hits out of 3073 actions) because a repin invalidates
external repository state.

The lock is JSON, so this compares structure rather than matching text. A
grep for "maven" or "jvm" would be worse than useless here: Java artifacts are
pinned in maven_install.json, which is a separate entry in JAVA_SHARED_GLOBS,
and the lock's moduleExtensions carries no rules_jvm_external entry at all. So
such a grep would answer "not Java" for almost every input, including inputs
that genuinely do affect Java.

Fail closed. Java is skipped only when every difference is positively
recognised as Java-irrelevant. Anything else, including a parse failure, a
missing file, an unreadable revision, a new top-level section, or an unfamiliar
module extension, answers "true". Wrongly skipping Java breaks main and is
found later by someone else; wrongly running it costs runner minutes.
"""

import json
import subprocess
import sys

LOCK = "MODULE.bazel.lock"

# Module extensions whose state cannot reach a Java action. Keep this list
# small and evidence-based: an extension earns a place here only when it is
# clear it produces no input to a Java target. Anything absent is treated as
# Java-relevant, so forgetting to add one costs time, never correctness.
JAVA_IRRELEVANT_EXTENSIONS = {
"@@rules_rust+//crate_universe:extension.bzl%crate",
"@@rules_rust+//crate_universe/private:internal_extensions.bzl%cu_nr",
}

# Top-level sections that may differ without implying a Java change, provided
# the differences inside them are themselves recognised. Every other top-level
# key differing means "true", so a future Bazel lock format that adds a section
# fails closed rather than being silently ignored.
INSPECTABLE_SECTIONS = {"moduleExtensions"}

# dict.get() cannot tell an absent key from one whose value is null, and
# cannot tell an absent section from an empty one. Both conflations were live
# bugs: a new top-level section added as null compared equal to its own
# absence and was never noticed, and deleting moduleExtensions outright made
# the head look like an empty extension set, so the only "changed" extensions
# were the base's, all recognised, and the tool answered false. Losing a whole
# section must never be a skip. Compare against a sentinel instead.
MISSING = object()


def die_true(reason):
print("true")
print(f"[lock-scope] {reason}; running Java", file=sys.stderr)
raise SystemExit(0)


def read_lock(ref):
try:
raw = subprocess.run(
["git", "show", f"{ref}:{LOCK}"],
capture_output=True,
check=True,
).stdout
except (subprocess.CalledProcessError, OSError) as exc:
die_true(f"cannot read {LOCK} at {ref}: {exc}")
try:
parsed = json.loads(raw)
except json.JSONDecodeError as exc:
die_true(f"cannot parse {LOCK} at {ref}: {exc}")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# "[]" and "null" are valid JSON but not a lock. Without this the section
# comparison below raises on a non-mapping and the tool dies with a
# traceback, which is an exit code nobody reads as "run Java".
if not isinstance(parsed, dict):
die_true(f"{LOCK} at {ref} is {type(parsed).__name__}, not an object")
return parsed


def main():
if len(sys.argv) != 3:
die_true("usage: bazel-lock-touches-java <base-ref> <head-ref>")
base, head = sys.argv[1], sys.argv[2]

a, b = read_lock(base), read_lock(head)

if a == b:
print("false")
print("[lock-scope] lock is unchanged; skipping Java", file=sys.stderr)
return

changed_sections = {
k for k in set(a) | set(b) if a.get(k, MISSING) != b.get(k, MISSING)
}

unexpected = changed_sections - INSPECTABLE_SECTIONS
if unexpected:
die_true(f"lock sections changed that are not inspectable: {sorted(unexpected)}")

# Require the section in both. Defaulting it to {} is what let a deletion
# read as "no extensions changed".
if "moduleExtensions" not in a or "moduleExtensions" not in b:
die_true("moduleExtensions is absent from one of the revisions")
ma, mb = a["moduleExtensions"], b["moduleExtensions"]
if not isinstance(ma, dict) or not isinstance(mb, dict):
die_true("moduleExtensions is not an object in one of the revisions")
Comment thread
coderabbitai[bot] marked this conversation as resolved.
changed_exts = {
k for k in set(ma) | set(mb) if ma.get(k, MISSING) != mb.get(k, MISSING)
}

unrecognised = changed_exts - JAVA_IRRELEVANT_EXTENSIONS
if unrecognised:
die_true(f"module extensions changed that may affect Java: {sorted(unrecognised)}")

print("false")
print(
f"[lock-scope] only Java-irrelevant extensions changed ({sorted(changed_exts)}); "
"skipping Java",
file=sys.stderr,
)


if __name__ == "__main__":
main()
Loading
Loading