Skip to content

Commit a7e9711

Browse files
fix(ci): a PR cannot trigger a full workspace run any more (#394)
Every reason to distrust the diff used to escalate to the whole workspace. On a PR that is the wrong trade twice over: it buries the change under ~22 shards of unrelated work, and it puts the PR's own legs behind an hour of queue, so the thing under review is the last thing to report. Touching this file was enough to trigger it, which is the case that keeps biting: a one-line CI edit cannot be reviewed against a fast signal. Measured three times today -- #390, #391 and #392 each ran a full matrix to validate a change whose blast radius was one job. `full` now answers only where "check everything" IS the request: the weekly cron and a manual `workflow_dispatch`. On a PR or a push, the same reasons call `widen` instead, which records them, prints them, and writes them to the run summary so a reviewer sees WITHOUT opening a job that this run deliberately tested less -- and that the sweep is one manual dispatch away. Degraded, with their reasons kept: .github/workflows/validate.yml, tests/*.sh mcpp.toml non-member change unclassified path push with no predecessor / predecessor not in history NOT weakened: descriptor and member changes select their members exactly as before. The only thing that changed is what happens when the diff cannot be classified. One consequence handled. `widen` does not exit, so the push-with-no- predecessor path has to leave a usable range behind. `HEAD` alone would diff root-to-HEAD, name every file and select every member -- a full run by another name, which is what this change exists to stop. It leaves the range EMPTY instead: nothing selected, the reason already said, the summary carrying it to a human. Rare either way, since merges here are squashes and `event.before` is present on every normal push. The weekly Sunday 06:00 sweep is deliberately kept. It is the only thing that would have caught the windows vulkan regression before a user did -- main's leg was pinned to `windows-2022` in #385 and the vulkan members were never re-tested there until a full run happened to fire today. Removing the automatic full run on PRs and keeping the periodic net is the same shape openxlings/xim-pkgindex#815 just added on the other side. Verified: the rendered fragment on `pull_request` with a validate.yml edit plus an unclassified path selects only the member its descriptor change names, and the same input under `workflow_dispatch` still goes full. Co-authored-by: sunrisepeak <x.d2learn.org@gmail.com>
1 parent e5bc33a commit a7e9711

1 file changed

Lines changed: 75 additions & 10 deletions

File tree

.github/workflows/validate.yml

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -496,13 +496,43 @@ jobs:
496496
id: plan
497497
shell: bash
498498
run: |
499+
# A FULL RUN IS NO LONGER SOMETHING A PR CAN TRIGGER BY ACCIDENT.
500+
#
501+
# Every reason to distrust the diff used to escalate to the whole
502+
# workspace. On a PR that is the wrong trade twice over: it buries
503+
# the change under ~22 shards of unrelated work, and it puts the
504+
# PR's own legs behind an hour of queue, so the thing being reviewed
505+
# is the last thing to report. Touching this file was enough to
506+
# trigger it -- which meant a one-line CI edit could not be reviewed
507+
# against a fast signal, measured repeatedly on #390, #391 and #392.
508+
#
509+
# So `full` now answers only where "check everything" is the actual
510+
# request: the weekly cron and a manual `workflow_dispatch`. On a PR
511+
# or a push, the same reasons instead WIDEN NOTHING and say so --
512+
# `widen` records them, the summary prints them, and a human who
513+
# wants the sweep runs the workflow by hand.
514+
#
515+
# What is deliberately NOT weakened: descriptor and member changes
516+
# still select their members exactly as before. The only thing that
517+
# changed is what happens when the diff cannot be classified.
499518
full() {
500519
echo "MEMBERS=__ALL__" >> "$GITHUB_ENV"
501520
echo "pkgs=__ALL__" >> "$GITHUB_OUTPUT"
502521
echo "full run: $1"
503522
exit 0
504523
}
505524
525+
UNTRUSTED=""
526+
widen() {
527+
case "${{ github.event_name }}" in
528+
schedule|workflow_dispatch) full "$1" ;;
529+
esac
530+
UNTRUSTED="${UNTRUSTED}${UNTRUSTED:+; }$1"
531+
echo "note: '$1' would once have forced a full workspace run;" \
532+
"selecting only what the diff names. Run this workflow" \
533+
"manually (workflow_dispatch) for the full sweep."
534+
}
535+
506536
# A push to main has a diff too — it was just never asked for.
507537
#
508538
# This used to be `event != pull_request -> full`, so every merge
@@ -528,33 +558,49 @@ jobs:
528558
base="${{ github.event.before }}"
529559
# All-zero on branch creation; absent object after a
530560
# force-push that dropped it. Either way there is nothing to
531-
# diff against, and guessing is worse than re-testing.
561+
# diff against.
562+
#
563+
# `widen` no longer exits on a push, so this has to leave a
564+
# USABLE range behind. `HEAD` alone (the root-to-HEAD diff)
565+
# would name every file in the repo and select every member --
566+
# a full run by another name, which is what this change exists
567+
# to stop. An empty range selects nothing, and `widen` has
568+
# already said why; the summary carries it to a human, who can
569+
# dispatch the sweep. Rare either way: merges here are squashes,
570+
# so `event.before` is present on every normal push.
571+
usable=1
532572
case "$base" in
533573
""|0000000000000000000000000000000000000000)
534-
full "push with no predecessor" ;;
574+
widen "push with no predecessor"; usable=0 ;;
535575
esac
536-
git cat-file -e "$base^{commit}" 2>/dev/null \
537-
|| full "push predecessor $base not in history"
538-
range="$base..HEAD" ;;
576+
if [ "$usable" = 1 ]; then
577+
git cat-file -e "$base^{commit}" 2>/dev/null \
578+
|| { widen "push predecessor $base not in history"; usable=0; }
579+
fi
580+
if [ "$usable" = 1 ]; then range="$base..HEAD"; else range=""; fi ;;
539581
*)
540582
full "event=${{ github.event_name }}" ;;
541583
esac
542-
changed=$(git diff --name-only $range)
543-
printf 'changed files vs %s:\n%s\n' "$base" "$changed"
584+
if [ -n "$range" ]; then
585+
changed=$(git diff --name-only $range)
586+
else
587+
changed=""
588+
fi
589+
printf 'changed files vs %s:\n%s\n' "${base:-<none>}" "$changed"
544590
sel=""; pkgsel=""
545591
add() { case " $sel " in *" $1 "*) ;; *) sel="$sel $1" ;; esac; }
546592
while IFS= read -r f; do
547593
[ -n "$f" ] || continue
548594
case "$f" in
549-
.github/workflows/validate.yml|tests/*.sh) full "$f" ;;
595+
.github/workflows/validate.yml|tests/*.sh) widen "$f" ;;
550596
mcpp.toml)
551597
# Workspace manifest. Every new-package PR appends to the
552598
# members list, so that alone must NOT force a full run:
553599
# select the added members; anything else in this file
554600
# (indices, settings) affects everyone → full.
555601
if ! diff -q <(git show "$base:mcpp.toml" | grep -v 'tests/examples/') \
556602
<(grep -v 'tests/examples/' mcpp.toml) >/dev/null; then
557-
full "mcpp.toml non-member change"
603+
widen "mcpp.toml non-member change"
558604
fi
559605
for p in $(comm -13 <(git show "$base:mcpp.toml" | grep -o 'tests/examples/[A-Za-z0-9._-]*' | sort -u) \
560606
<(grep -o 'tests/examples/[A-Za-z0-9._-]*' mcpp.toml | sort -u)); do
@@ -595,7 +641,7 @@ jobs:
595641
# here. Guard it in `lint` if that ever bites.
596642
tests/member-timings.tsv) : ;;
597643
*.md|docs/*|.agents/*|.github/*|tools/*) : ;;
598-
*) full "unclassified change: $f" ;;
644+
*) widen "unclassified change: $f" ;;
599645
esac
600646
done <<EOF
601647
$changed
@@ -606,6 +652,25 @@ jobs:
606652
pkgsel=${pkgsel# }
607653
echo "pkgs=$pkgsel" >> "$GITHUB_OUTPUT"
608654
echo "changed descriptors: ${pkgsel:-<none>}"
655+
656+
# Carried to the run summary, not just the log. A widened reason is
657+
# the one thing here a reviewer has to see WITHOUT opening a job:
658+
# it says this run deliberately tested less than the old behaviour
659+
# would have, and what to do about it.
660+
if [ -n "$UNTRUSTED" ]; then
661+
{
662+
echo "### Selective run (full sweep not triggered)"
663+
echo
664+
echo "These would once have forced a full workspace run:"
665+
echo
666+
echo "- $UNTRUSTED"
667+
echo
668+
echo "Selected members: \`${sel:-<none>}\`"
669+
echo
670+
echo "Run this workflow manually (**Run workflow** /"
671+
echo "\`workflow_dispatch\`) for the full sweep."
672+
} >> "$GITHUB_STEP_SUMMARY"
673+
fi
609674
# Sharding is for the FULL run only, and the shard count per platform is
610675
# that platform's RUNNER CONCURRENCY — not a round number.
611676
#

0 commit comments

Comments
 (0)