On 2024-10-06 17:58, Robin Jarry wrote:
Mattias Rönnblom, Oct 06, 2024 at 16:17:
I think you need to change run_command() to custom_target(). I was
thinking of this patch to be much simpler as follows:
I started off with a shell script, but I switched to Python when I
wanted to strip off comments. (Not that that can't be done in bourne
shell.)
The below script doesn't strip off comments, and provides no usage
information. Earlier I got the impression you wanted to improve
command-line usage.
I think we need to decide if this thing is an integral part of the
build system, custom-made for its needs, or if its something that
should also be friendly to a command-line human user.
I could go either way on that.
We don't have to choose. Being part of the build system does not mean
If it's an integral part of the build system, and the only user is
another program (e.g., meson.build), the script can be made simpler. For
example, you need not bother with usage information, since a computer
program will not have any use of it.
It would be helpful if Bruce could comment on this. How should we think
about small tools like this? That are more-or-less just an artifact of
the lack of expressiveness in meson.
I'm leaning toward having them as proper human-usable CLI tools,
provided it doesn't cause too much fuzz on the meson.build side of things.
the script cannot use the standard python tools to parse arguments. Here
is what I came up with. It is shorter, strips comments and deals with
arguments in a simpler way. We don't need to pass the "missing" or
"redundant" operation to the script, it can figure out what to do on its
own (see inline comment in main()):
Sure, you could check for both issues at the same time. The primary
reason for doing otherwise was that the code base wouldn't survive the
"redundant" check at the time of the script's introduction (in the patch
set).
diff --git a/buildtools/chkincs/check_extern_c.py b/buildtools/chkincs/
check_extern_c.py
new file mode 100755
index 000000000000..3e61719a5ea5
--- /dev/null
+++ b/buildtools/chkincs/check_extern_c.py
@@ -0,0 +1,72 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+
+"""
+Detect missing or redundant `extern "C"` statements in headers.
+"""
+
+import argparse
+import re
+import sys
+
+
+# regular expressions
+EXTERN_C_RE = re.compile(r'^extern\s+"C"', re.MULTILINE)
+CPP_LINE_COMMENT_RE = re.compile(r"//.*$", re.MULTILINE)
+C_BLOCK_COMMENT_RE = re.compile(r"/\*.+?\*/", re.DOTALL)
+C_SYMBOL_RE = [
+ # external variable definitions
+ re.compile(r"^extern\s+[a-z0-9_]+\s", re.MULTILINE),
+ # exported functions
+ re.compile(r"\brte_[a-z0-9_]+\("),
+ re.compile(r"\bcmdline_[a-z0-9_]+\("),
+ re.compile(r"\bvt100_[a-z0-9_]+\("),
+ re.compile(r"\brdline_[a-z0-9_]+\("),
+ re.compile(r"\bcirbuf_[a-z0-9_]+\("),
+ # windows compatibility
+ re.compile(r"\bpthread_[a-z0-9_]+\("),
+ re.compile(r"\bregcomp\("),
+ re.compile(r"\bcount_cpu\("),
+]
+
+
+def strip_comments(buf: str) -> str:
+ buf = CPP_LINE_COMMENT_RE.sub("", buf, re.MULTILINE)
+ return C_BLOCK_COMMENT_RE.sub("", buf, re.DOTALL)
+
+
+def has_c_symbols(buf: str) -> bool:
+ for expr in C_SYMBOL_RE:
+ if expr.search(buf, re.MULTILINE):
+ return True
+ return False
+
+
+def main() -> int:
+ parser = argparse.ArgumentParser(description=__doc__)
+ parser.add_argument("headers", nargs="+")
+ args = parser.parse_args()
+
+ ret = 0
+
+ for h in args.headers:
+ with open(h) as f:
+ buf = f.read()
+
+ buf = strip_comments(buf)
+
+ if has_c_symbols(buf):
+ if not EXTERN_C_RE.search(buf):
+ print('error: missing extern "C" in', h)
+ ret = 1
+ # Uncomment next block once all extraneous extern "C" have been
removed
+ #else:
+ # if EXTERN_C_RE.search(buf):
+ # print('error: extraneous extern "C" in', h)
+ # ret = 1
+
+ return ret
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/
meson.build
index f2dadcae18ef..a551b2df9268 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -38,14 +38,11 @@ if not add_languages('cpp', required: false)
endif
# check for extern C in files, since this is not detected as an error by
the compiler
-grep = find_program('grep', required: false)
-if grep.found()
- errlist = run_command([grep, '--files-without-match', '^extern
"C"', dpdk_chkinc_headers],
- check: false, capture: true).stdout().split()
- if errlist != []
- error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n-
'.join(errlist))
- endif
-endif
+custom_target('check-extern-c',
+ command: files('check_extern_c.py') + ['@INPUT@'],
+ input: dpdk_chkinc_headers,
+ output: 'check-extern-c',
+ build_by_default: true)
gen_cpp_files = generator(gen_c_file_for_header,
output: '@BASENAME@.cpp',
I won't review this in detail, but you can always submit a new patch
against what's on main. What is there now is somewhat half-way between
"proper CLI tool" and "meson.build delegate".
