diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4dbaac1..504018a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -215,7 +215,7 @@ jobs: run: | cmake -B build/release -G Ninja \ -DCMAKE_BUILD_TYPE=Release \ - -DBUILD_TESTS=OFF + -DEBLDR_BUILD_TESTS=OFF cmake --build build/release --parallel $(nproc) - name: Package artifacts diff --git a/tests/unit/test_build_options_are_read.py b/tests/unit/test_build_options_are_read.py index 3c2bd4f..2afff29 100644 --- a/tests/unit/test_build_options_are_read.py +++ b/tests/unit/test_build_options_are_read.py @@ -19,8 +19,10 @@ """ import re +import shlex import pytest +import yaml from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[2] @@ -146,3 +148,179 @@ def test_no_option_offers_to_skip_verification_or_authentication(): declared = _declared_options() assert "EBLDR_REQUIRE_SIGNATURES" not in declared assert "EBLDR_RECOVERY_AUTH" not in declared + + +# ── Rule 3: a -D flag a workflow passes to CMake names a variable that exists ── +# +# CMake does not fail on a -D name the project never reads: it configures, +# prints "Manually-specified variables were not used by the project" as a +# warning, and the flag does nothing. ci.yml's test job once passed +# -DBUILD_TESTS=ON for EBLDR_BUILD_TESTS and ran zero tests (TASKS.md T-004, +# #41); the tag-only `release` job kept the same misspelling, as +# -DBUILD_TESTS=OFF, after that fix. +# +# Scope: the `run:` scripts of the workflows under .github/workflows/. Only +# -D arguments of a `cmake` configure command count (`-DNAME=v`, +# `-DNAME:TYPE=v` and the spaced `-D NAME=v`); `cmake --build`, other +# programs and a -D inside a quoted flags string such as CMAKE_C_FLAGS are +# not CMake cache variables. Variables that CMake or a standard module owns +# are not the project's to declare and are skipped (NON_PROJECT_PREFIXES). +# ci/ci_security.yml is not a workflow GitHub runs and is not scanned. + +WORKFLOWS = REPO_ROOT / ".github" / "workflows" +NON_PROJECT_PREFIXES = ("CMAKE_", "FETCHCONTENT_", "Python3_", "IDF_") +_NOT_CONFIGURE = {"--build", "--install", "-E", "-P", "--find-package", "--help", "--version"} + + +def _strip_shell_comment(line): + """Drop a full-line comment, or a trailing one that starts at a # after + whitespace, the way the shell would read it.""" + if line.lstrip().startswith("#"): + return "" + return re.split(r"\s#", line, maxsplit=1)[0] + + +# Words that can come before the program a simple command runs. A configure +# written as `then cmake ...`, `env CC=clang cmake ...` or `sudo cmake ...` +# is still a configure, so these are dropped before argv[0] is tested. +_LEADING_KEYWORDS = {"then", "do", "else", "elif", "if", "while", "until", "!", "{"} +_WRAPPERS = {"env", "time", "sudo", "nice", "exec", "command", "nohup"} + + +def _drop_leading_words(argv): + i = 0 + while i < len(argv): + word = argv[i] + if word in _LEADING_KEYWORDS or re.match(r"[A-Za-z_][A-Za-z0-9_]*=", word): + i += 1 + elif word in _WRAPPERS: + i += 1 + # the wrapper's own options, e.g. `sudo -E`, `nice -n 10` + while i < len(argv) and (argv[i].startswith("-") or argv[i].isdigit()): + i += 1 + else: + break + return argv[i:] + + +def _commands(script): + """Split a run script into argv lists, one per simple command. Command + substitutions, subshells and backticks are split out too, so a cmake + inside `$( ... )` or `( ... )` is a command of its own.""" + text = script.replace("\\\n", " ") + for line in text.splitlines(): + line = _strip_shell_comment(line) + for part in re.split(r"&&|\|\||;|\||\$\(|`|\(|\)", line): + try: + argv = shlex.split(part) + except ValueError: + argv = part.split() + argv = _drop_leading_words(argv) + if argv: + yield argv + + +def _cmake_defines_in_script(script): + names = set() + for argv in _commands(script): + if Path(argv[0]).name != "cmake" or _NOT_CONFIGURE & set(argv[1:]): + continue + args = iter(argv[1:]) + for arg in args: + if arg == "-D": + arg = "-D" + next(args, "") + m = re.match(r"-D([A-Za-z_][A-Za-z0-9_]*)(?::[A-Za-z]+)?=", arg) + if m: + names.add(m.group(1)) + return names + + +def _run_scripts(path): + doc = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + for job in (doc.get("jobs") or {}).values(): + for step in job.get("steps") or []: + if isinstance(step.get("run"), str): + yield step["run"] + + +def _undeclared(names): + declared = _declared_options() + return sorted(n for n in names + if not n.startswith(NON_PROJECT_PREFIXES) and n not in declared) + + +def test_every_cmake_define_in_a_workflow_is_declared(): + found = {} + for path in sorted(WORKFLOWS.glob("*.y*ml")): + names = set() + for script in _run_scripts(path): + names |= _cmake_defines_in_script(script) + if _undeclared(names): + found[path.name] = _undeclared(names) + assert not found, ( + "these workflows pass -D flags CMakeLists.txt does not declare; CMake " + "ignores them with a warning, so the flag does nothing: %s" % found) + + +def test_workflow_scan_reads_the_flags_it_must(): + # A scan that finds nothing passes trivially; make sure it sees the flags + # the CI test job really passes. + names = set() + for script in _run_scripts(WORKFLOWS / "ci.yml"): + names |= _cmake_defines_in_script(script) + assert {"EBLDR_BUILD_TESTS", "EBLDR_BOARD", "CMAKE_BUILD_TYPE"} <= names + + +# (script, names the scan must report) +SCAN_CASES = [ + ("cmake -B b -DEBLDR_BOARD:STRING=none", {"EBLDR_BOARD"}), + ("cmake -B b -D EBLDR_BOARD=none", {"EBLDR_BOARD"}), + ("cmake -B b \\\n -DEBLDR_BUILD_TESTS=ON \\\n -D EBLDR_BOARD=none", {"EBLDR_BUILD_TESTS", "EBLDR_BOARD"}), + ("cmake -B b -DEBLDR_BUILD_TESTS=ON # was -DOLD=1", {"EBLDR_BUILD_TESTS"}), + (" # was: cmake -B b -DBUILD_TESTS=ON", set()), + ("cmake -B b -DCMAKE_C_FLAGS=\"-DEBLDR_FOO=1 -O2\"", {"CMAKE_C_FLAGS"}), + ("gcc -DNDEBUG=1 -c x.c", set()), + ("export CFLAGS=\"-DUSE_X=1\"", set()), + ("cmake --build build -DIGNORED=1", set()), + ("mkdir -p b && cmake -S . -B b -DEBLDR_HARDENING=ON; ctest", {"EBLDR_HARDENING"}), + # A configure that does not start the line must still be seen. + ("if [ -d src ]; then cmake -B b -DEBLDR_TYPO=1 ..; fi", {"EBLDR_TYPO"}), + ("env FOO=1 cmake -B b -DEBLDR_TYPO=1 ..", {"EBLDR_TYPO"}), + ("CC=clang cmake -B b -DEBLDR_TYPO=1", {"EBLDR_TYPO"}), + ("for t in a b; do cmake -B b-$t -DEBLDR_TYPO=1; done", {"EBLDR_TYPO"}), + ("time cmake -B b -DEBLDR_TYPO=1", {"EBLDR_TYPO"}), + ("sudo -E cmake -B b -DEBLDR_TYPO=1", {"EBLDR_TYPO"}), + ("out=$(cmake -B b -DEBLDR_TYPO=1 2>&1)", {"EBLDR_TYPO"}), + ("(cd x && cmake -B b -DEBLDR_TYPO=1)", {"EBLDR_TYPO"}), + ("cmake --build b --parallel $(nproc)", set()), +] + + +@pytest.mark.parametrize("script, expected", SCAN_CASES, ids=[str(i) for i in range(len(SCAN_CASES))]) +def test_scan_reads_cmake_configure_defines_only(script, expected): + assert _cmake_defines_in_script(script) == expected + + +@pytest.mark.parametrize("name", [ + "CMAKE_CXX_COMPILER", "FETCHCONTENT_FULLY_DISCONNECTED", "Python3_EXECUTABLE", "IDF_TARGET", +]) +def test_non_project_variables_are_not_required_to_be_declared(name): + assert _undeclared({name}) == [] + + +@pytest.mark.parametrize("script", [ + "cmake -B build/release -G Ninja -DBUILD_TESTS=OFF", + "cmake -B build -DEBLDR_BUILD_TEST=ON", + "cmake -B build -D EBLDR_BUILD_TEST=ON", + "cmake -B build -DEBLDR_ENABLE_SECURITY_CHECKS=ON", +], ids=["BUILD_TESTS", "EBLDR_BUILD_TEST", "spaced", "EBLDR_ENABLE_SECURITY_CHECKS"]) +def test_an_undeclared_define_is_caught(script): + # The guard's negative control: the real ci.yml scripts plus one more -- + # the way a dead flag would come back. EBLDR_ENABLE_SECURITY_CHECKS is + # declared, but only in cmake/security_checks.cmake, which CMakeLists.txt + # never includes, so passing it does nothing either. + names = set() + for s in list(_run_scripts(WORKFLOWS / "ci.yml")) + [script]: + names |= _cmake_defines_in_script(s) + name = re.search(r"-D ?(\w+)=", script).group(1) + assert name in _undeclared(names)