From 7b2d19ca8673f047381931327b47ee5f31c61251 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 15 Jun 2026 08:36:04 +0000 Subject: [PATCH 1/3] parallel-make-check: warn when a job runtime drifts >50% from "minutes" The "minutes" field is only a scheduling estimate; when it goes stale it just packs the schedule a little worse, and there was no signal that a value needed updating. Emit a non-fatal warning when a config that explicitly sets "minutes" finishes more than 50% above or below it (a GitHub ::warning:: annotation in CI, a plain line locally) and flag the row in the step-summary table with the value to copy over. Configs that omit "minutes" keep riding the 1.0 default and are left alone. The warning never touches the exit status, so it cannot fail the job. --- .github/scripts/parallel-make-check.py | 36 ++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/.github/scripts/parallel-make-check.py b/.github/scripts/parallel-make-check.py index 3fd103fb76..95c04ca1ea 100755 --- a/.github/scripts/parallel-make-check.py +++ b/.github/scripts/parallel-make-check.py @@ -19,7 +19,9 @@ # minutes expected duration, from the Minutes column of a previous # run's summary (default 1.0). Schedule weight only - configs # run longest-first and --shard balances shards by it; a stale -# value just packs the schedule a little worse. +# value just packs the schedule a little worse, but one that +# drifts past +/-50% of the measured time draws a warning +# (never a failure) so it is easy to spot and update. # user_settings header staged as /user_settings.h before # configure (path relative to the source root); pair it with # --enable-usersettings in "configure" @@ -88,6 +90,9 @@ class Config: check: bool = True prepare: list[list[str]] = field(default_factory=list) run: list[list[str]] = field(default_factory=list) + # Whether "minutes" was given in the JSON (vs the 1.0 default); only an + # explicit estimate is checked for >50% drift against the real time. + minutes_provided: bool = False SRCDIR = Path(__file__).resolve().parents[2] ON_GITHUB = os.environ.get("GITHUB_ACTIONS") == "true" @@ -208,7 +213,8 @@ def load_configs(opts: argparse.Namespace, entry.get("ldflags", opts.ldflags), float(minutes), user_settings, check, list(entry.get("prepare", [])), - list(entry.get("run", [])))) + list(entry.get("run", [])), + minutes_provided="minutes" in entry)) if not configs: error(f"{opts.json}: no configs") return configs @@ -238,6 +244,23 @@ def dump(title: str, path: Path) -> None: sys.stdout.flush() +def warn(msg: str) -> None: + # GitHub surfaces ::warning:: as an annotation at the top of the run; + # locally it is just a line. Informational only - never fails the run. + print(f"::warning::{msg}" if ON_GITHUB else f"WARNING: {msg}") + + +def stale_estimate(cfg: Config, minutes: float) -> bool: + # "minutes" is only a scheduling estimate (configs run longest-first; + # --shard balances by it), never a pass/fail bound. Flag a finished + # config whose real time drifted past +/-50% of an explicitly given + # estimate so stale values - which pack the schedule worse - are easy + # to find and update. Configs that omit "minutes" ride the 1.0 default + # placeholder and are left alone. + return (cfg.minutes_provided + and not 0.5 * cfg.minutes <= minutes <= 1.5 * cfg.minutes) + + def run_config(cfg: Config, opts: argparse.Namespace) -> tuple[str | None, float]: if opts.fail_fast and stop_event.is_set(): @@ -344,6 +367,10 @@ def run_config(cfg: Config, opts: argparse.Namespace) -> tuple[str | None, # One line per passing config; the full logs would bloat the CI # log (they stay in build-/make-check.log). print(f"{cfg.name}: pass [{minutes:.1f} min]") + if stale_estimate(cfg, minutes): + warn(f"{cfg.name}: ran {minutes:.1f} min but \"minutes\" " + f"says {cfg.minutes:g} (>50% off) - update it in the " + f"config JSON") sys.stdout.flush() else: dump(f"{cfg.name}: FAIL ({failed}) [{minutes:.1f} min]", log) @@ -364,6 +391,11 @@ def summarize(results: list[tuple[Config, str | None, float]], ok = f":x: FAIL ({failed})" else: ok = ":white_check_mark: pass" + if stale_estimate(cfg, minutes): + # Non-fatal nudge mirroring the per-config warning, kept in + # the summary next to the Minutes value to copy over. + ok += (f' :warning: "minutes" {cfg.minutes:g} is >50% off, ' + f"update to ~{minutes:.1f}") lines.append(f"| {cfg.name} | {ok} | {minutes:.1f} |") # Two views of how efficiently the pool used the machine: thread # occupancy is the time the workers spent running configs out of the From d9079978ed21c314f07913a37e087c0d43fc7dd0 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 15 Jun 2026 11:14:06 +0000 Subject: [PATCH 2/3] parallel-make-check: percent-encode warn() workflow-command data A config name comes from JSON and is only checked for emptiness and a '/', so it can carry %, CR or LF. Passed straight into the ::warning:: workflow command those would truncate the annotation or be parsed as a second command, so escape them in the GitHub branch of warn() per GitHub's documented command-data encoding (% first). Local output is unchanged. --- .github/scripts/parallel-make-check.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/scripts/parallel-make-check.py b/.github/scripts/parallel-make-check.py index 95c04ca1ea..c88fff12ab 100755 --- a/.github/scripts/parallel-make-check.py +++ b/.github/scripts/parallel-make-check.py @@ -247,7 +247,14 @@ def dump(title: str, path: Path) -> None: def warn(msg: str) -> None: # GitHub surfaces ::warning:: as an annotation at the top of the run; # locally it is just a line. Informational only - never fails the run. - print(f"::warning::{msg}" if ON_GITHUB else f"WARNING: {msg}") + if ON_GITHUB: + # Percent-encode the command data (GitHub's documented escaping) so + # a stray %, CR or LF - e.g. from a config name out of the JSON - + # can't truncate the annotation or be read as a second command. + msg = msg.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + print(f"::warning::{msg}") + else: + print(f"WARNING: {msg}") def stale_estimate(cfg: Config, minutes: float) -> bool: From ea3bd56e9778db4b0df02d404596a5945b3ce8f7 Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Mon, 15 Jun 2026 11:30:00 +0000 Subject: [PATCH 3/3] parallel-make-check: fix warning-doc wording and escape every workflow command Addresses review feedback: - The "minutes" header comment described the check backwards (the estimate drifting from the measured time). Reword it to match the code, which warns when the measured time lands more than +/-50% away from the estimate. - Centralize the GitHub workflow-command escaping in gh_escape() and apply it to the ::group:: title in dump() and the ::error:: summary in main(), not just warn(), so a config name or step carrying %, CR or LF cannot corrupt those commands either. --- .github/scripts/parallel-make-check.py | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/.github/scripts/parallel-make-check.py b/.github/scripts/parallel-make-check.py index c88fff12ab..299fa46416 100755 --- a/.github/scripts/parallel-make-check.py +++ b/.github/scripts/parallel-make-check.py @@ -19,9 +19,10 @@ # minutes expected duration, from the Minutes column of a previous # run's summary (default 1.0). Schedule weight only - configs # run longest-first and --shard balances shards by it; a stale -# value just packs the schedule a little worse, but one that -# drifts past +/-50% of the measured time draws a warning -# (never a failure) so it is easy to spot and update. +# value just packs the schedule a little worse, but a run +# whose measured time lands more than +/-50% away from it +# draws a warning (never a failure) so it is easy to spot +# and update. # user_settings header staged as /user_settings.h before # configure (path relative to the source root); pair it with # --enable-usersettings in "configure" @@ -233,8 +234,16 @@ def privatize_dirs(bdir: Path, dirs: list[str]) -> None: shutil.copytree(SRCDIR / name, d, symlinks=True) +def gh_escape(data: str) -> str: + # Percent-encode workflow-command data (GitHub's documented encoding) + # so a stray %, CR or LF - e.g. from a config name or step out of the + # JSON - can't truncate the command or be parsed as a second one. + return data.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + def dump(title: str, path: Path) -> None: - print(f"::group::{title}" if ON_GITHUB else f"==== {title} ====") + # ::group:: is a workflow command; escape its title like warn() does. + print(f"::group::{gh_escape(title)}" if ON_GITHUB else f"==== {title} ====") try: sys.stdout.write(path.read_text(errors="replace")) except OSError as e: @@ -247,14 +256,7 @@ def dump(title: str, path: Path) -> None: def warn(msg: str) -> None: # GitHub surfaces ::warning:: as an annotation at the top of the run; # locally it is just a line. Informational only - never fails the run. - if ON_GITHUB: - # Percent-encode the command data (GitHub's documented escaping) so - # a stray %, CR or LF - e.g. from a config name out of the JSON - - # can't truncate the annotation or be read as a second command. - msg = msg.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") - print(f"::warning::{msg}") - else: - print(f"WARNING: {msg}") + print(f"::warning::{gh_escape(msg)}" if ON_GITHUB else f"WARNING: {msg}") def stale_estimate(cfg: Config, minutes: float) -> bool: @@ -534,7 +536,7 @@ def main() -> int: else "aborted without a recorded failure" if aborted: msg += f" ({aborted} config(s) aborted by fail-fast)" - print(f"::error::{msg}" if ON_GITHUB else msg) + print(f"::error::{gh_escape(msg)}" if ON_GITHUB else msg) return 1 return 0