From a62884599b86292006c34d43b4d85b2cf9eb1f4b Mon Sep 17 00:00:00 2001 From: Juliusz Sosinowicz Date: Thu, 11 Jun 2026 19:50:33 +0000 Subject: [PATCH] CI review fixes: JSON validation, log volume, rm -rf, flag spelling Address the Copilot review: - parallel-make-check.py: validate "configure" (list of strings) and cflags/ldflags (strings) so a malformed entry fails the load instead of exploding a string into per-character configure arguments; print a single line for passing configs instead of dumping their full make-check.log into the CI log (failure dumps unchanged; the logs remain in build-/ for the failure artifacts). - Makefile.am: use rm -rf for the certs/input/quit setup and distclean cleanup. A --private-dir run replaces the certs symlink with a private directory copy that rm -f cannot remove (verified: make distclean in a build dir with a privatized certs/ now succeeds and removes it). - psk.yml, disable-pk-algs.yml: normalize the single-dash tokens (-disable-rsa, -disable-ecc, -disable-aescbc, -enable-cryptonly) carried verbatim from the old matrices to the canonical double-dash form. No coverage change: configure honors single-dash spellings (verified -disable-rsa sets NO_RSA with no unrecognized-option warning), so these were always in effect; both touched configs re-validated end-to-end. The --cc default stays "ccache gcc": ccache resolves the compiler through its own masquerade symlinks (verified: no recursion and normal cache hits with /usr/lib/ccache prepended to PATH), and the explicit CC= also covers jobs that use ccache without the PATH masquerade. --- .github/scripts/parallel-make-check.py | 18 +++++++++++++++--- .github/workflows/disable-pk-algs.yml | 2 +- .github/workflows/psk.yml | 10 +++++----- Makefile.am | 8 ++++++-- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/.github/scripts/parallel-make-check.py b/.github/scripts/parallel-make-check.py index 4f4cf2ce82..57d0b61d3a 100755 --- a/.github/scripts/parallel-make-check.py +++ b/.github/scripts/parallel-make-check.py @@ -142,6 +142,14 @@ def load_configs(opts, error): f"directory suffix: {entry!r}") if any(cfg.name == name for cfg in configs): error(f"{opts.json}: duplicate config name {name!r}") + configure = entry.get("configure", []) + if not (isinstance(configure, list) + and all(isinstance(a, str) for a in configure)): + error(f"{opts.json}: \"configure\" must be a list of argument " + f"strings in {name!r}") + for key in ("cflags", "ldflags"): + if not isinstance(entry.get(key, ""), str): + error(f"{opts.json}: \"{key}\" must be a string in {name!r}") minutes = entry.get("minutes", 1.0) if isinstance(minutes, bool) or not isinstance(minutes, (int, float)) \ or minutes < 0: @@ -165,7 +173,7 @@ def load_configs(opts, error): for cmd in cmds)): error(f"{opts.json}: \"{key}\" must be a list of argv lists " f"in {name!r}") - configs.append(Config(name, list(entry.get("configure", [])), cc, + configs.append(Config(name, list(configure), cc, entry.get("cflags", opts.cflags), entry.get("ldflags", opts.ldflags), float(minutes), user_settings, check, @@ -273,9 +281,13 @@ def run_config(cfg, opts): if failed == "aborted": print(f"{cfg.name}: aborted (fail-fast) [{minutes:.1f} min]") sys.stdout.flush() + elif not failed: + # 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]") + sys.stdout.flush() else: - verdict = f"FAIL ({failed})" if failed else "pass" - dump(f"{cfg.name}: {verdict} [{minutes:.1f} min]", log) + dump(f"{cfg.name}: FAIL ({failed}) [{minutes:.1f} min]", log) if failed == "configure": dump(f"{cfg.name}: config.log", bdir / "config.log") elif failed == "make check": diff --git a/.github/workflows/disable-pk-algs.yml b/.github/workflows/disable-pk-algs.yml index 1517e9ff0b..fa7cdaa210 100644 --- a/.github/workflows/disable-pk-algs.yml +++ b/.github/workflows/disable-pk-algs.yml @@ -94,7 +94,7 @@ jobs: "--disable-curve25519", "--disable-ed25519", "--disable-curve448", "--disable-ed448", "--enable-curve448", "--enable-ed448"]}, {"name": "cryptonly-rsa", "minutes": 0.8, - "configure": ["-enable-cryptonly", "--disable-rsa", "--disable-dh", + "configure": ["--enable-cryptonly", "--disable-rsa", "--disable-dh", "--disable-ecc", "--disable-curve25519", "--disable-ed25519", "--disable-curve448", "--disable-ed448", "--enable-rsa"]}, {"name": "cryptonly-dh", "minutes": 0.8, diff --git a/.github/workflows/psk.yml b/.github/workflows/psk.yml index 2bb512cb5b..6024db1b78 100644 --- a/.github/workflows/psk.yml +++ b/.github/workflows/psk.yml @@ -73,23 +73,23 @@ jobs: "--disable-ecc", "--disable-dh"]}, {"name": "static-psk-lowresource-tls12", "minutes": 0.8, "configure": ["--disable-oldtls", "--disable-tls13", "--enable-psk", - "-disable-rsa", "--disable-dh", "-disable-ecc", "--disable-asn", + "--disable-rsa", "--disable-dh", "--disable-ecc", "--disable-asn", "C_EXTRA_FLAGS=-DWOLFSSL_STATIC_PSK", "--enable-lowresource", "--enable-singlethreaded", "--disable-asm", "--disable-errorstrings", "--disable-pkcs12", "--disable-sha3", "--disable-sha224", "--disable-sha384", "--disable-sha512", - "--disable-sha", "--disable-md5", "-disable-aescbc", + "--disable-sha", "--disable-md5", "--disable-aescbc", "--disable-chacha", "--disable-poly1305", "--disable-coding", "--disable-sp-math-all", "--disable-mlkem"]}, {"name": "static-psk-lowresource-tls13", "minutes": 0.8, "configure": ["--disable-oldtls", "--disable-tlsv12", - "--enable-tls13", "--enable-psk", "-disable-rsa", "--disable-dh", - "-disable-ecc", "--disable-asn", + "--enable-tls13", "--enable-psk", "--disable-rsa", "--disable-dh", + "--disable-ecc", "--disable-asn", "C_EXTRA_FLAGS=-DWOLFSSL_STATIC_PSK", "--enable-lowresource", "--enable-singlethreaded", "--disable-asm", "--disable-errorstrings", "--disable-pkcs12", "--disable-sha3", "--disable-sha224", "--disable-sha384", "--disable-sha512", - "--disable-sha", "--disable-md5", "-disable-aescbc", + "--disable-sha", "--disable-md5", "--disable-aescbc", "--disable-chacha", "--disable-poly1305", "--disable-coding", "--disable-sp-math-all", "--disable-mlkem"]} ] diff --git a/Makefile.am b/Makefile.am index 6add8d5904..0215e0600a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -296,6 +296,10 @@ TESTS += $(check_SCRIPTS) # the build tree so the tests can find it. This is driven by a stamp file in # BUILT_SOURCES so it runs once, before anything else, for "make", "make all" # and "make check". It is a no-op for in-tree builds. +# +# The setup and the distclean cleanup use rm -rf: a --private-dir run of +# .github/scripts/parallel-make-check.py replaces the certs symlink with a +# private directory copy, which rm -f would not remove. ############################################################################## BUILT_SOURCES += wolfssl-test-data.stamp @@ -303,7 +307,7 @@ wolfssl-test-data.stamp: $(AM_V_at)if test "$(abs_top_srcdir)" != "$(abs_top_builddir)"; then \ $(MKDIR_P) tests scripts examples; \ for f in certs input quit; do \ - rm -f "$$f"; \ + rm -rf "$$f"; \ $(LN_S) "$(abs_top_srcdir)/$$f" "$$f"; \ done; \ rm -f examples/crypto_policies; \ @@ -335,7 +339,7 @@ DISTCLEANFILES += wolfssl-test-data.stamp # srcdir != builddir so an in-tree build never touches the real source files. distclean-local: $(AM_V_at)if test "$(abs_top_srcdir)" != "$(abs_top_builddir)"; then \ - rm -f certs input quit; \ + rm -rf certs input quit; \ rm -f tests/*.conf tests/*.cnf tests/TXT_DB.txt; \ rm -f scripts/*.pcap scripts/*.out scripts/*.sslkeylog \ scripts/multi-msg-record.py; \