From 250899953a498de8b8a31ff92e8ec4358133d5b5 Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Fri, 29 Nov 2024 00:22:12 +0000 Subject: [PATCH 1/8] util: make compiler helper language aware Include paths differ between C and C++ domains (typically C++ has additional search directories), so we need to take that into account. Furthermore, it makes sense to specify `-nostdinc` if we are including the standard include directories explicitly already. Not only we prevent potential duplication of search paths, we ensure that the resulting search is strictly confined to the paths the given compiler would search and not the disjunction of those and libclang's own default search paths. Since this is going to be called from within the directives now, it also makes sense to improve error handling, which we do. --- src/hawkmoth/util/compiler.py | 39 ++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/hawkmoth/util/compiler.py b/src/hawkmoth/util/compiler.py index ea35370f..102d5319 100644 --- a/src/hawkmoth/util/compiler.py +++ b/src/hawkmoth/util/compiler.py @@ -9,6 +9,9 @@ """ import subprocess +from sphinx.util import logging + +logger = logging.getLogger(__name__) def _removesuffix(s, suffix): if suffix and s.endswith(suffix): @@ -32,22 +35,38 @@ def _get_paths_from_output(output): yield line.strip() -def _get_include_paths(cc_path): - result = subprocess.run([cc_path, '-E', '-Wp,-v', '-'], - stdin=subprocess.DEVNULL, - capture_output=True, - check=True, - text=True) +def _get_include_paths(cpath, lang): + try: + result = subprocess.run([cpath, '-x', lang, '-E', '-Wp,-v', '-'], + stdin=subprocess.DEVNULL, + capture_output=True, + check=True, + text=True) + except FileNotFoundError: + logger.warning(f"get_include_args: {lang} compiler not found ('{cpath}')") + return [] + + except subprocess.CalledProcessError: + logger.warning(f"get_include_args: incompatible {lang} compiler ('{cpath}')") + return [] + + if result.returncode != 0: + logger.warning(f"get_include_args: incompatible {lang} compiler ('{cpath}')") + return [] return _get_paths_from_output(result.stderr) -def get_include_args(cc_path='clang'): - return [f'-I{path}' for path in _get_include_paths(cc_path)] +def get_include_args(cpath='clang', lang='c', cc_path=None): + if cc_path is not None: + cpath = cc_path + logger.warning('get_include_args: `cc_path` argument has been deprecated; use `cpath` instead') # noqa: E501 + + return ['-nostdinc'] + [f'-I{path}' for path in _get_include_paths(cpath, lang)] if __name__ == '__main__': import pprint import sys - cc_path = sys.argv[1] if len(sys.argv) > 1 else 'clang' + compiler = sys.argv[1] if len(sys.argv) > 1 else 'clang' - pprint.pprint(get_include_args(cc_path)) + pprint.pprint(get_include_args(compiler)) From 5767ddbd8484bdb6610b3d20136ed24938ef312d Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Fri, 29 Nov 2024 00:11:00 +0000 Subject: [PATCH 2/8] cfg: add compiler and autoconf options It's a common problem to want hawkmoth to parse the sources with the exact headers a given code is meant to be compiled with and doing so requires some verbosity upfront which can easily be avoided. Provide a couple of options to do just that. The defaults are chosen in a technically backwards incompatible way, but it would take a frankly adversarial case to break it. In fact the new defaults should yield the default behaviour everyone expected from the beginning: we all expected libclang's search paths to be an exact match to clang's, and indeed they are in some distributions. Finally, the new search paths are added after other user's options since the headers are searched in a left to right manner, giving the user the possibility of overriding specific headers despite using this option. For maximum flexibility, this functionality is gated on the 'autoconf' settings, which can easily be extended later to support additional features. --- doc/extension.rst | 43 ++++++++++++++++++++++++++++------------ src/hawkmoth/__init__.py | 35 +++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/doc/extension.rst b/doc/extension.rst index c2df8aac..9dfb673d 100644 --- a/doc/extension.rst +++ b/doc/extension.rst @@ -56,10 +56,39 @@ See also additional configuration options in the :ref:`built-in extensions :type: str The default transform parameter to be passed to the - :event:`hawkmoth-process-docstring` event. It can be overriden with the + :event:`hawkmoth-process-docstring` event. It can be overridden with the ``transform`` option of the :ref:`directives `. Defaults to ``None``. +.. py:data:: hawkmoth_compiler + :type: str + + The (path to the) default compiler used by the project. This is used to + determine the exact options needed to parse the code files by libclang + provided the relevant options are enabled in :data:`hawkmoth_autoconf`. + + Notably, it allows hawkmoth to override libclang's default search path for + system headers with those of the specified compiler. + + This presumes the compiler supports being called as + `` -x -E -Wp,-v /dev/null``. + + Defaults to ``clang``, which may differ from libclang's own default includes. + It will use libclang's defaults if set to ``None`` though. + +.. py:data:: hawkmoth_autoconf + :type: list + + List of options that control the automatic configuration features of + hawkmoth. Currently supported options: + + * ``'stdinc'``: override the standard include paths of libclang with those of + the specified compiler (see :data:`hawkmoth_compiler`). + + This is a shortcut to specify ``-nostdinc -I ... -I`` in + :data:`hawkmoth_clang` with the search directories of the specified + compiler. + .. py:data:: hawkmoth_clang :type: list @@ -73,18 +102,6 @@ See also additional configuration options in the :ref:`built-in extensions hawkmoth_clang = ['-I/path/to/include', '-DHAWKMOTH'] - Hawkmoth provides a convenience helper for querying the include path from the - compiler, and providing them as ``-I`` options: - - .. code-block:: python - - from hawkmoth.util import compiler - - hawkmoth_clang = compiler.get_include_args() - - You can also pass in the compiler to use, for example - ``get_include_args('gcc')``. - .. py:data:: hawkmoth_clang_c :type: list diff --git a/src/hawkmoth/__init__.py b/src/hawkmoth/__init__.py index e6e2aac7..514810f2 100644 --- a/src/hawkmoth/__init__.py +++ b/src/hawkmoth/__init__.py @@ -19,6 +19,7 @@ from sphinx.util.docutils import switch_source_input, SphinxDirective from sphinx.util import logging +from hawkmoth.util import compiler from hawkmoth.parser import parse, ErrorLevel from hawkmoth.util import strutil from hawkmoth import docstring @@ -54,16 +55,16 @@ def __display_parser_diagnostics(self, errors): location=(self.env.docname, self.lineno)) def __get_clang_args(self): - clang_args = [] - - clang_args.extend(self.env.config.hawkmoth_clang.copy()) + clang_args = self.env.config.hawkmoth_clang.copy() if self._domain == 'c': clang_args.extend(self.env.config.hawkmoth_clang_c.copy()) + clang_args.extend(self.options.get('clang', [])) + clang_args.extend(self.env.config._clang_args_post_c.copy()) else: clang_args.extend(self.env.config.hawkmoth_clang_cpp.copy()) - - clang_args.extend(self.options.get('clang', [])) + clang_args.extend(self.options.get('clang', [])) + clang_args.extend(self.env.config._clang_args_post_cpp.copy()) return clang_args @@ -354,10 +355,31 @@ def _doctree_read(app, doctree): onlynode += nodes.reference('', '', inline, internal=False, refuri=uri) signode += onlynode +def _autoconf(app, config): + logger = logging.getLogger(__name__) + cpath = config.hawkmoth_compiler + autoconf = config.hawkmoth_autoconf + + ignored_options = [x for x in autoconf if x not in ['stdinc']] + if len(ignored_options) > 0: + logger.warning(f'autoconf: {ignored_options} unsupported option(s) ignored') + + config._clang_args_post_c = [] + config._clang_args_post_cpp = [] + + if 'stdinc' in autoconf: + if cpath: + config._clang_args_post_c = compiler.get_include_args(cpath, 'c') + config._clang_args_post_cpp = compiler.get_include_args(cpath, 'c++') + else: + logger.warning('autoconf: \'stdinc\' option ignored (missing compiler)') + def setup(app): app.require_sphinx('3.0') app.add_config_value('hawkmoth_root', app.confdir, 'env', [str]) + app.add_config_value('hawkmoth_compiler', 'clang', 'env', [str, type(None)]) + app.add_config_value('hawkmoth_autoconf', ['stdinc'], 'env', [list]) app.add_config_value('hawkmoth_clang', [], 'env', [list]) app.add_config_value('hawkmoth_clang_c', [], 'env', [list]) app.add_config_value('hawkmoth_clang_cpp', [], 'env', [list]) @@ -387,6 +409,9 @@ def setup(app): app.add_event('hawkmoth-process-docstring') + # Auto configure once during initialization. + app.connect('config-inited', _autoconf) + # Source code link app.add_config_value('hawkmoth_source_uri', None, 'env', [str]) app.connect('doctree-read', _doctree_read) From f3e5603e4f5bcf792a0e035b60bd591842863bdc Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Fri, 29 Nov 2024 00:28:32 +0000 Subject: [PATCH 3/8] test: fix header search paths in tests This reverts the effects of commit 1243852, while fixing the inconsistent handling of C/C++ domains since libclang can't always figure out the right search paths for its own version of Clang despite prior claims. It is still unclear whether the upstream issue is an actual issue or if this is the intended behaviour (https://github.com/llvm/llvm-project/issues/18150), though it _is_ an issue and simply using the fixed helper function and configuration parameter comes at a suitably low cost to maintenance. Fixes #262. --- test/testenv.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/testenv.py b/test/testenv.py index 77f6e88a..07090988 100644 --- a/test/testenv.py +++ b/test/testenv.py @@ -7,6 +7,7 @@ import pytest import strictyaml +from hawkmoth.util import compiler from test import conf testext = '.yaml' @@ -49,10 +50,12 @@ def get_clang_args(self): if self.domain == 'c': clang_args.extend(getattr(conf, 'hawkmoth_clang_c', [])) + clang_args.extend(self.options.get('clang', [])) + clang_args.extend(compiler.get_include_args('clang', 'c')) else: clang_args.extend(getattr(conf, 'hawkmoth_clang_cpp', [])) - - clang_args.extend(self.options.get('clang', [])) + clang_args.extend(self.options.get('clang', [])) + clang_args.extend(compiler.get_include_args('clang', 'c++')) return clang_args From 0521c6985d52c6abe8e9263055b937bc4297c06f Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Fri, 29 Nov 2024 01:33:45 +0000 Subject: [PATCH 4/8] ci: examples: install dependencies The example check script has now an indirect import dependency on clang's bindings (testenv->hawkmoth.util.compiler) even though it doesn't actually need them. Fixing it in code implies changing the module structure, which we care for more than we care for this script that happens to take advantage of tooling that is otherwise used in scenarios where the bindings are very much needed (running the tests). --- .github/workflows/makefile.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/makefile.yml b/.github/workflows/makefile.yml index 91ea9f64..94be632f 100644 --- a/.github/workflows/makefile.yml +++ b/.github/workflows/makefile.yml @@ -46,6 +46,8 @@ jobs: steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 + - name: Install dependencies (apt) + run: sudo apt-get install -y python3-clang python3-pip - name: Examples check run: | . venv From 251b978e135e336591da92774b2dfd42beb486b7 Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Sat, 8 Feb 2025 00:01:09 +0000 Subject: [PATCH 5/8] test: optionally select active tests for a test case Allow each test case to select which of all the supported tests are active. By default, all of them are, but sometimes it's impossible to create tests that will pass as they target something specific to those tests. The motivation here is the expanding set of configuration variables, which have not been tested so far, but which only affect extension tests. Creating a test case for a breaking configuration would then not work since the parser / cli tests would not see the same breaking configuration. Technically, it's unlikely a test case will ever need to be _just_ for cli or parser, but not extension: anything that breaks those would break the extension ones. But we do it in a general way anyway for simplicity. --- test/test_cli.py | 8 +++++++- test/test_extension.py | 7 ++++++- test/test_parser.py | 7 ++++++- test/testenv.py | 3 +++ 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/test/test_cli.py b/test/test_cli.py index 554559d3..ef83719f 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -32,6 +32,10 @@ def _stderr_basename(errors_str): class CliTestcase(testenv.Testcase): + + def valid(self): + return 'cli' in self.options.get('test', ['cli']) + def set_monkeypatch(self, monkeypatch): self.monkeypatch = monkeypatch self.mock_args([]) @@ -88,7 +92,9 @@ def get_expected(self): def _get_cli_testcases(path): for f in testenv.get_testcase_filenames(path): - yield CliTestcase(f) + testcase = CliTestcase(f) + if testcase.valid(): + yield testcase @pytest.mark.full @pytest.mark.parametrize('testcase', _get_cli_testcases(testenv.testdir), diff --git a/test/test_extension.py b/test/test_extension.py index 4ee74dc6..a43a7eee 100755 --- a/test/test_extension.py +++ b/test/test_extension.py @@ -20,6 +20,9 @@ def __init__(self, filename, buildername): super().__init__(filename) self._buildername = buildername + def valid(self): + return 'extension' in self.options.get('test', ['extension']) + def _get_suffix(self): return 'txt' if self._buildername == 'text' else self._buildername @@ -83,7 +86,9 @@ def get_expected(self): def _get_extension_testcases(path, buildername): for f in testenv.get_testcase_filenames(path): - yield ExtensionTestcase(f, buildername) + testcase = ExtensionTestcase(f, buildername) + if testcase.valid(): + yield testcase # Test using Sphinx plain text builder @pytest.mark.parametrize('testcase', _get_extension_testcases(testenv.testdir, 'text'), diff --git a/test/test_parser.py b/test/test_parser.py index 43bc5003..af80e262 100755 --- a/test/test_parser.py +++ b/test/test_parser.py @@ -58,6 +58,9 @@ def _filter_members(directive): return members class ParserTestcase(testenv.Testcase): + def valid(self): + return 'parser' in self.options.get('test', ['parser']) + def get_output(self): roots = {} docs_str = '' @@ -114,7 +117,9 @@ def get_expected(self): def _get_parser_testcases(path): for f in testenv.get_testcase_filenames(path): - yield ParserTestcase(f) + testcase = ParserTestcase(f) + if testcase.valid(): + yield testcase @pytest.mark.parametrize('testcase', _get_parser_testcases(testenv.testdir), ids=testenv.get_testid) diff --git a/test/testenv.py b/test/testenv.py index 07090988..9ea6772f 100644 --- a/test/testenv.py +++ b/test/testenv.py @@ -61,6 +61,7 @@ def get_clang_args(self): class Testcase: _options_schema = strictyaml.Map({ + strictyaml.Optional('test'): strictyaml.Seq(strictyaml.Str()), 'directives': strictyaml.Seq(strictyaml.Map({ 'domain': strictyaml.Enum(['c', 'cpp']), 'directive': strictyaml.Str(), @@ -85,6 +86,8 @@ def __init__(self, filename): self.filename = filename with open(filename) as f: self.options = strictyaml.load(f.read(), self._options_schema).data + if self.options.get('test', None) is None: + self.options['test'] = ['cli', 'parser', 'extension'] self.testid = os.path.splitext(os.path.relpath(self.filename, testdir))[0] self.directives = [Directive(self, directive_config) for From fd743cbd25a0fe864a598411a072f0ff2bbdc392 Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Sat, 8 Feb 2025 00:01:24 +0000 Subject: [PATCH 6/8] test: allow overriding configuration variables per test Following the previous commit, we can now specify an 'extension only' test. This only gets us half the way there though. We now need to be able to vary the configuration variables. We could do so by pointing to a whole different conf file, but it's simpler and neater for now to keep a single standard one and allow arbitrary overrides instead. --- test/test_extension.py | 3 ++- test/testenv.py | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/test/test_extension.py b/test/test_extension.py index a43a7eee..121c6377 100755 --- a/test/test_extension.py +++ b/test/test_extension.py @@ -30,6 +30,7 @@ def _sphinx_build(self, srcdir): outdir = os.path.join(srcdir, self._buildername) doctreedir = os.path.join(srcdir, 'doctrees') confdir = testenv.testdir + confoverrides = self.get_conf_overrides() # Don't emit color codes in Sphinx status/warning output console.nocolor() @@ -39,7 +40,7 @@ def _sphinx_build(self, srcdir): with patch_docutils(confdir), docutils_namespace(): app = Sphinx(srcdir=srcdir, confdir=confdir, outdir=outdir, doctreedir=doctreedir, buildername=self._buildername, - warning=warning) + confoverrides=confoverrides, warning=warning) # Ensure there are no errors with app creation. assert warning.getvalue() == '' diff --git a/test/testenv.py b/test/testenv.py index 9ea6772f..af3c6d43 100644 --- a/test/testenv.py +++ b/test/testenv.py @@ -74,6 +74,9 @@ class Testcase: strictyaml.Optional('transform'): strictyaml.Str(), }), })), + strictyaml.Optional('conf-overrides'): strictyaml.MapPattern( + strictyaml.Str(), strictyaml.NullNone() | strictyaml.EmptyList() | strictyaml.Any(), + ), strictyaml.Optional('expected-failure'): strictyaml.Bool(), strictyaml.Optional('example-use-namespace'): strictyaml.Bool(), strictyaml.Optional('example-title'): strictyaml.Str(), @@ -108,6 +111,9 @@ def get_expected_filename(self): def get_stderr_filename(self): return self.get_relative_filename(self.options.get('errors')) + def get_conf_overrides(self): + return self.options.get('conf-overrides', {}) + def run_test(self): if self.options.get('expected-failure'): pytest.xfail() From d58ad4374cb30240a2d71e93a0dd6f0940d3ea9f Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Sun, 2 Feb 2025 23:43:08 +0000 Subject: [PATCH 7/8] test: allow testing for setup errors We are now issuing warnings during the extension setup and we want to check those too in our tests. This simply removes a 'fail early' condition while ensuring any setup error is checked against the .stderr files for a given test. --- test/test_extension.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test_extension.py b/test/test_extension.py index 121c6377..3cc8f208 100755 --- a/test/test_extension.py +++ b/test/test_extension.py @@ -42,9 +42,6 @@ def _sphinx_build(self, srcdir): doctreedir=doctreedir, buildername=self._buildername, confoverrides=confoverrides, warning=warning) - # Ensure there are no errors with app creation. - assert warning.getvalue() == '' - # Set root to the directory the testcase yaml is in, because the # filenames in yaml are relative to it. app.config.hawkmoth_root = os.path.dirname(self.filename) From d4666f97ae18f4dfffd55cba4c9f9c066546928e Mon Sep 17 00:00:00 2001 From: Bruno Santos Date: Mon, 30 Dec 2024 12:21:31 +0000 Subject: [PATCH 8/8] test: add compiler/autoconf option tests --- test/c/autoconf-invalid.stderr | 1 + test/c/autoconf-invalid.yaml | 12 ++++++++++++ test/c/autoconf.yaml | 13 +++++++++++++ test/c/compiler-autoconf-mismatch.stderr | 1 + test/c/compiler-autoconf-mismatch.yaml | 13 +++++++++++++ test/c/compiler-not-found.stderr | 2 ++ test/c/compiler-not-found.yaml | 11 +++++++++++ test/c/compiler-unknown.stderr | 2 ++ test/c/compiler-unknown.yaml | 11 +++++++++++ 9 files changed, 66 insertions(+) create mode 100644 test/c/autoconf-invalid.stderr create mode 100644 test/c/autoconf-invalid.yaml create mode 100644 test/c/autoconf.yaml create mode 100644 test/c/compiler-autoconf-mismatch.stderr create mode 100644 test/c/compiler-autoconf-mismatch.yaml create mode 100644 test/c/compiler-not-found.stderr create mode 100644 test/c/compiler-not-found.yaml create mode 100644 test/c/compiler-unknown.stderr create mode 100644 test/c/compiler-unknown.yaml diff --git a/test/c/autoconf-invalid.stderr b/test/c/autoconf-invalid.stderr new file mode 100644 index 00000000..2a565ee7 --- /dev/null +++ b/test/c/autoconf-invalid.stderr @@ -0,0 +1 @@ +WARNING: autoconf: ['invalid'] unsupported option(s) ignored diff --git a/test/c/autoconf-invalid.yaml b/test/c/autoconf-invalid.yaml new file mode 100644 index 00000000..6d2303fe --- /dev/null +++ b/test/c/autoconf-invalid.yaml @@ -0,0 +1,12 @@ +test: +- extension +directives: +- domain: c + directive: autodoc + arguments: + - doc.c +conf-overrides: + hawkmoth_autoconf: + - 'invalid' +errors: autoconf-invalid.stderr +expected: doc.rst diff --git a/test/c/autoconf.yaml b/test/c/autoconf.yaml new file mode 100644 index 00000000..5e3194be --- /dev/null +++ b/test/c/autoconf.yaml @@ -0,0 +1,13 @@ +test: +- extension +directives: +- domain: c + directive: autodoc + arguments: + - bool.c +conf-overrides: + hawkmoth_clang: -nostdinc + hawkmoth_compiler: clang + hawkmoth_autoconf: + - 'stdinc' +expected: bool.rst diff --git a/test/c/compiler-autoconf-mismatch.stderr b/test/c/compiler-autoconf-mismatch.stderr new file mode 100644 index 00000000..3e3bc1b0 --- /dev/null +++ b/test/c/compiler-autoconf-mismatch.stderr @@ -0,0 +1 @@ +WARNING: autoconf: 'stdinc' option ignored (missing compiler) diff --git a/test/c/compiler-autoconf-mismatch.yaml b/test/c/compiler-autoconf-mismatch.yaml new file mode 100644 index 00000000..87b63d4a --- /dev/null +++ b/test/c/compiler-autoconf-mismatch.yaml @@ -0,0 +1,13 @@ +test: +- extension +directives: +- domain: c + directive: autodoc + arguments: + - doc.c +conf-overrides: + hawkmoth_compiler: null + hawkmoth_autoconf: + - 'stdinc' +errors: compiler-autoconf-mismatch.stderr +expected: doc.rst diff --git a/test/c/compiler-not-found.stderr b/test/c/compiler-not-found.stderr new file mode 100644 index 00000000..2339ae30 --- /dev/null +++ b/test/c/compiler-not-found.stderr @@ -0,0 +1,2 @@ +WARNING: get_include_args: c compiler not found ('invalid') +WARNING: get_include_args: c++ compiler not found ('invalid') diff --git a/test/c/compiler-not-found.yaml b/test/c/compiler-not-found.yaml new file mode 100644 index 00000000..00ed457e --- /dev/null +++ b/test/c/compiler-not-found.yaml @@ -0,0 +1,11 @@ +test: +- extension +directives: +- domain: c + directive: autodoc + arguments: + - doc.c +conf-overrides: + hawkmoth_compiler: invalid +errors: compiler-not-found.stderr +expected: doc.rst diff --git a/test/c/compiler-unknown.stderr b/test/c/compiler-unknown.stderr new file mode 100644 index 00000000..03be0e53 --- /dev/null +++ b/test/c/compiler-unknown.stderr @@ -0,0 +1,2 @@ +WARNING: get_include_args: incompatible c compiler ('false') +WARNING: get_include_args: incompatible c++ compiler ('false') diff --git a/test/c/compiler-unknown.yaml b/test/c/compiler-unknown.yaml new file mode 100644 index 00000000..f86ee65f --- /dev/null +++ b/test/c/compiler-unknown.yaml @@ -0,0 +1,11 @@ +test: +- extension +directives: +- domain: c + directive: autodoc + arguments: + - doc.c +conf-overrides: + hawkmoth_compiler: false +errors: compiler-unknown.stderr +expected: doc.rst