From d911eca657ede07a691014e07c882a77b5faab12 Mon Sep 17 00:00:00 2001 From: Luca Toniolo <10792599+grandixximo@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:30:56 +1000 Subject: [PATCH 1/2] src: include exported headers with angle brackets where they are used The build copies every SRCHEADERS entry into include/, so an exported header exists twice: the source under src/ and the copy modules compile against. A quoted include searches the includer's own directory before the -I path, an angled include does not, so the two forms can reach different copies of the same header and both compile. An exported header travels to include/ and takes its siblings with it, so it keeps the quoted form and finds them wherever it lands, and so does its implementation, wanting the source rather than a stale export. The files changed here are users, and build out of tree where only the exported copies exist. --- src/emc/ini/iniaxis.cc | 2 +- src/emc/ini/inijoint.cc | 2 +- src/emc/ini/inispindle.cc | 2 +- src/emc/ini/initraj.cc | 2 +- src/emc/ini/inivalue.cc | 2 +- src/emc/motion-logger/motion-logger.c | 2 +- src/hal/components/matrixkins.comp | 4 ++-- src/hal/hal.hh | 2 +- src/tests/mathtest.c | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/emc/ini/iniaxis.cc b/src/emc/ini/iniaxis.cc index e15744185a3..ea8440209c4 100644 --- a/src/emc/ini/iniaxis.cc +++ b/src/emc/ini/iniaxis.cc @@ -19,7 +19,7 @@ #include "nml_intf/emcglb.h" #include "nml_intf/emccfg.h" #include "libnml/rcs/rcs_print.hh" -#include "inifile.hh" +#include #include "inihal.hh" #include "iniaxis.hh" diff --git a/src/emc/ini/inijoint.cc b/src/emc/ini/inijoint.cc index 37dff22f622..a4536b96c4c 100644 --- a/src/emc/ini/inijoint.cc +++ b/src/emc/ini/inijoint.cc @@ -17,7 +17,7 @@ #include "libnml/rcs/rcs_print.hh" #include "nml_intf/emcglb.h" #include "nml_intf/emccfg.h" -#include "inifile.hh" +#include #include "inihal.hh" #include "inijoint.hh" diff --git a/src/emc/ini/inispindle.cc b/src/emc/ini/inispindle.cc index 7438ae02bcd..ecbfcab5b8b 100644 --- a/src/emc/ini/inispindle.cc +++ b/src/emc/ini/inispindle.cc @@ -20,7 +20,7 @@ #include "libnml/rcs/rcs_print.hh" #include "nml_intf/emcglb.h" #include "nml_intf/emccfg.h" -#include "inifile.hh" +#include #include "inihal.hh" #include "inispindle.hh" diff --git a/src/emc/ini/initraj.cc b/src/emc/ini/initraj.cc index b3140082271..ace0882ca43 100644 --- a/src/emc/ini/initraj.cc +++ b/src/emc/ini/initraj.cc @@ -18,7 +18,7 @@ #include #include "libnml/rcs/rcs_print.hh" #include "nml_intf/emcglb.h" -#include "inifile.hh" +#include #include "inihal.hh" #include "initraj.hh" diff --git a/src/emc/ini/inivalue.cc b/src/emc/ini/inivalue.cc index f854db3f6bb..7bc5221f5be 100644 --- a/src/emc/ini/inivalue.cc +++ b/src/emc/ini/inivalue.cc @@ -22,7 +22,7 @@ #include #include -#include "inifile.hh" +#include using namespace linuxcnc; diff --git a/src/emc/motion-logger/motion-logger.c b/src/emc/motion-logger/motion-logger.c index e63d6abd785..520eda9a1d2 100644 --- a/src/emc/motion-logger/motion-logger.c +++ b/src/emc/motion-logger/motion-logger.c @@ -33,7 +33,7 @@ #include #include "motion/motion.h" #include "motion/motion_struct.h" -#include "motion_types.h" +#include #include "motion/mot_priv.h" #include "motion/axis.h" diff --git a/src/hal/components/matrixkins.comp b/src/hal/components/matrixkins.comp index c63a5e211c1..b12dedc2fcf 100644 --- a/src/hal/components/matrixkins.comp +++ b/src/hal/components/matrixkins.comp @@ -225,8 +225,8 @@ error: return -1; } -#include "kinematics.h" -#include "emcmotcfg.h" +#include +#include KINS_NOT_SWITCHABLE EXPORT_SYMBOL(kinematicsType); diff --git a/src/hal/hal.hh b/src/hal/hal.hh index e4c9a40e637..fe482033158 100644 --- a/src/hal/hal.hh +++ b/src/hal/hal.hh @@ -4,7 +4,7 @@ #include #include #include -#include "hal.h" +#include #warning "Do not use hal.hh. It will be removed (and, eventually, replaced)." diff --git a/src/tests/mathtest.c b/src/tests/mathtest.c index 1e33b4544dc..204aed5ffe4 100644 --- a/src/tests/mathtest.c +++ b/src/tests/mathtest.c @@ -34,7 +34,7 @@ #include /* sin(), cos(), isnan() etc. */ #include /* DBL_MAX */ #include /* errno, EDOM */ -#include "sincos.h" +#include /* math functions are: From b3e6a46c5363c2f10cecc1c426d1bbf1b7400948 Mon Sep 17 00:00:00 2001 From: Luca Toniolo <10792599+grandixximo@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:30:56 +1000 Subject: [PATCH 2/2] scripts: check the include style of exported headers Nothing catches the wrong form today, since both compile, and the quoted one silently resolves to whichever copy sits nearest. The check reads the SRCHEADERS list, so it follows whatever the build exports. Exported headers are left alone: they are copied to include/ and have to keep finding their siblings there. A header whose own directory also holds code that merely uses it needs its implementation named, inifile.hh and hal.h being those cases in tree today. Everywhere else being in the header's directory is enough. Findings are warnings by default and errors with --error, which is how CI runs it, and named files can be passed for use from a pre-commit hook. --- .github/workflows/ci.yml | 2 + scripts/include-style-check.py | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100755 scripts/include-style-check.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 518315a9d67..595e49ed309 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,6 +138,8 @@ jobs: run: | set -x scripts/cppcheck.sh + - name: Check include style of exported headers + run: scripts/include-style-check.py --enforce shellcheck: runs-on: ubuntu-24.04 diff --git a/scripts/include-style-check.py b/scripts/include-style-check.py new file mode 100755 index 00000000000..57136b7c37a --- /dev/null +++ b/scripts/include-style-check.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +# +# Check the include style of exported headers +# Copyright (C) 2026 L. Toniolo +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of the GNU General Public License version 2 or later. +# +# The build copies every SRCHEADERS entry into include/, so an exported header +# exists twice: the source under src/ and the copy every module compiles +# against. A quoted include searches the includer's own directory first, an +# angled include does not, so the two forms can reach different copies and both +# compile. +# +# An exported header travels to include/ and must take its siblings with it, so +# it includes them with quotes and finds the copies beside it wherever it ends +# up. Its implementation does the same, wanting the source next to it rather +# than a stale export. Everything else is a user, builds out of tree where only +# the exported copies exist, and uses angle brackets. +# +import sys +import os +import re +import getopt +import subprocess + +error_on_warning = False + +# The script lives in scripts/ and the sources are one level up, so it runs +# from anywhere. +topdir = os.path.normpath(os.path.join(os.path.dirname(os.path.realpath(__file__)), "..")) + +SUFFIXES = (".c", ".cc", ".cpp", ".h", ".hh", ".comp") + +# Headers whose own directory also holds code that merely uses them. Sitting +# beside the header says nothing there, so name the implementation and hold +# every other file in that directory to the angled form. +INTERFACES = { + "src/emc/ini/inifile.hh": ( + "src/emc/ini/inifile.cc", + ), + "src/hal/hal.h": ( + "src/hal/hal_lib.c", + "src/hal/hal_lib_extra.c", + "src/hal/hal_lib_query.c", + ), +} + +RE_QUOTED = re.compile(r'^[ \t]*#[ \t]*include[ \t]*"([^"]+)"', re.M) +RE_SRCHEADERS = re.compile(r"^SRCHEADERS\s*:=\s*\\\n((?:.*\\\n)*.*)$", re.M) + + +def usage(): + print("""Check the include style of exported headers. +Usage: + include-style-check.py [-e] [-h] [file...] + +Checks every source file under src/ when given no file, which is how CI runs +it. Named files are useful from a pre-commit hook. + +Options: + -e|--error Treat findings as errors (--enforce is accepted as well) + -h|--help This message +""") + sys.exit(2) + + +messages = [] + +# +# Collect messages +# +def pfind(path, lineno, msg): + global messages + messages.append((path, lineno, msg)) + +def flush_messages(): + kind = "error" if error_on_warning else "warning" + for path, lineno, msg in messages: + print("{}:{}: {}: {}".format(path, lineno, kind, msg)) + # Annotate the offending lines when running under CI + if os.environ.get("GITHUB_ACTIONS"): + for path, lineno, msg in messages: + print("::{} file={},line={},title=Include style::{}".format(kind, path, lineno, msg)) + if not messages: + return 0 + return 1 if error_on_warning else 0 + + +def exported_headers(): + """Map each exported header's basename onto its path under src/, taken + from the SRCHEADERS list the build installs into include/.""" + with open(os.path.join(topdir, "src", "Makefile"), encoding="utf-8", errors="replace") as f: + m = RE_SRCHEADERS.search(f.read()) + if not m: + print("No SRCHEADERS list found in src/Makefile", file=sys.stderr) + sys.exit(2) + headers = {} + for line in m.group(1).split("\n"): + entry = line.strip().rstrip("\\").strip() + if entry: + headers[os.path.basename(entry)] = "src/" + entry + return headers + + +def tracked_files(): + """All tracked files under src/, and of those the ones worth reading.""" + try: + out = subprocess.run(["git", "-C", topdir, "ls-files", "src"], + capture_output=True, text=True, check=True).stdout + except (OSError, subprocess.CalledProcessError) as err: + print(err, file=sys.stderr) + sys.exit(2) + tracked = set(out.split("\n")) + return tracked, sorted(f for f in tracked if f.endswith(SUFFIXES)) + + +def check_quoted_includes(tracked, files, headers): + exported = set(headers.values()) + for path in files: + if path in exported: + # An exported header is copied to include/ and has to keep finding + # its siblings there, so it includes them with quotes. + continue + directory = os.path.dirname(path) + with open(os.path.join(topdir, path), encoding="utf-8", errors="replace") as f: + text = f.read() + for m in RE_QUOTED.finditer(text): + name = m.group(1) + source = headers.get(os.path.basename(name)) + if source is None: + continue # not an exported header, nothing to say about it + # A file of that name beside the includer is a different header + # that happens to share the basename, not this one. + local = os.path.normpath(os.path.join(directory, name)) + if local != source and local in tracked: + continue + if source in INTERFACES: + if path in INTERFACES[source]: + continue # the implementation, taking its own header + why = "used from outside its implementation" + elif directory == os.path.dirname(source): + continue # the implementation, taking the header beside it + else: + why = "exported by {}".format(os.path.dirname(source)) + lineno = text[:m.start()].count("\n") + 1 + pfind(path, lineno, + 'include "{}" is {}, use <{}>'.format(name, why, os.path.basename(name))) + + +def main(): + try: + opts, args = getopt.getopt(sys.argv[1:], "eh", ["error", "enforce", "help"]) + except getopt.GetoptError as err: + print(err, file=sys.stderr) + usage() + + global error_on_warning + for o, unused_a in opts: + if o in ("-e", "--error", "--enforce"): + error_on_warning = True + elif o in ("-h", "--help"): + usage() + + if os.environ.get("INCLUDE_STYLE_CHECK_ENFORCE"): + error_on_warning = True + + # From here on we collect findings with pfind(). They get flushed when we + # are done. The program's return value depends on whether findings are + # treated as errors or not. + headers = exported_headers() + tracked, files = tracked_files() + if args: + # Named files, as a pre-commit hook would pass them. Anything outside + # the set the whole-tree run covers has nothing to say about it. + named = {os.path.relpath(os.path.abspath(a), topdir) for a in args} + files = [f for f in files if f in named] + check_quoted_includes(tracked, files, headers) + + return + +if __name__ == "__main__": + main() + sys.exit(flush_messages()) # Exit value depends on findings being errors