Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix include paths #263

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/makefile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 29 additions & 13 deletions doc/extension.rst
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,38 @@ 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 <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
``<compiler> -x <c|c++> -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.

This is a shortcut to specify ``-nostdinc -I<dir 1> ... -I<dir n>`` in
:data:`hawkmoth_clang` with the search directories of the specified compiler.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the documentation for hawkmoth_compiler should merely briefly reference what it's used for, and other places (i.e. hawkmoth_autoconf) should describe the details.

.. 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`).

.. py:data:: hawkmoth_clang
:type: list

Expand All @@ -73,18 +101,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

Expand Down
31 changes: 29 additions & 2 deletions src/hawkmoth/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,6 +40,31 @@ class _AutoBaseDirective(SphinxDirective):
_domain: Optional[str] = None
_docstring_types: Optional[list[type[docstring.Docstring]]] = None

def __init__(self, name, arguments, options,
content, lineno, content_offset,
block_text, state, state_machine):

super().__init__(name, arguments, options,
content, lineno, content_offset,
block_text, state, state_machine)

cpath = self.env.config.hawkmoth_compiler
autoconf = self.env.config.hawkmoth_autoconf

ignored_options = [x for x in autoconf if x not in ['stdinc']]
if len(ignored_options) > 0:
self.logger.warning(f'autoconf: {ignored_options} unsupported option(s) ignored')

self._clang_args_post = []
if 'stdinc' in autoconf:
if cpath:
if self._domain == 'c':
self._clang_args_post = compiler.get_include_args(cpath, 'c')
else:
self._clang_args_post = compiler.get_include_args(cpath, 'c++')
else:
self.logger.warning('autoconf: \'stdinc\' option ignored (missing compiler)')

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still gets called once per each directive used in the documentation. There's one instance for directive.

The hard part is that setup() can't use the config options. IIUC you'd have to use a Sphinx event to do it once.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative is to simplify and go back to the first version (instead of the constructor) and optimize later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course. Well, I did this on a crowded flight, it was bound to happen 😅

I'll look into it when I can. There are other (maybe less clean) alternatives like caching the result if we go for a temporary solution, but the events should work well enough if there's a suitable one.

def __display_parser_diagnostics(self, errors):
# Map parser diagnostic level to Sphinx level name
log_level_map = {
Expand All @@ -56,15 +82,14 @@ def __display_parser_diagnostics(self, errors):
def __get_clang_args(self):
clang_args = []

clang_args.extend(self.env.config.hawkmoth_clang.copy())

if self._domain == 'c':
clang_args.extend(self.env.config.hawkmoth_clang_c.copy())
else:
clang_args.extend(self.env.config.hawkmoth_clang_cpp.copy())

clang_args.extend(self.options.get('clang', []))

clang_args.extend(self._clang_args_post)
return clang_args

def __parse(self, filename):
Expand Down Expand Up @@ -358,6 +383,8 @@ 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])
Expand Down
39 changes: 29 additions & 10 deletions src/hawkmoth/util/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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: compiler not found ('{cpath}')")
return []

except subprocess.CalledProcessError:
logger.warning(f"get_include_args: incompatible compiler ('{cpath}')")
return []

if result.returncode != 0:
logger.warning(f"get_include_args: incompatible 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))
1 change: 1 addition & 0 deletions test/c/autoconf-invalid.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
WARNING: autoconf: ['invalid'] unsupported option(s) ignored
12 changes: 12 additions & 0 deletions test/c/autoconf-invalid.yaml
Original file line number Diff line number Diff line change
@@ -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
13 changes: 13 additions & 0 deletions test/c/autoconf.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/c/compiler-autoconf-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
WARNING: autoconf: 'stdinc' option ignored (missing compiler)
13 changes: 13 additions & 0 deletions test/c/compiler-autoconf-mismatch.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/c/compiler-not-found.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
WARNING: get_include_args: compiler not found ('invalid')
11 changes: 11 additions & 0 deletions test/c/compiler-not-found.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/c/compiler-unknown.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
WARNING: get_include_args: incompatible compiler ('false')
11 changes: 11 additions & 0 deletions test/c/compiler-unknown.yaml
Original file line number Diff line number Diff line change
@@ -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
32 changes: 32 additions & 0 deletions test/c/no-autoconf.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@

.. c:var:: static int convert_bool

Retain bool instead of using _Bool.


.. c:var:: static bool convert_Bool

Also convert _Bool to bool.


.. c:function:: int boolean(int bar, bool baz)

Bool function.


.. c:struct:: sample_struct

This is a sample struct

Woohoo.


.. c:member:: int bool_member

bool member


.. c:member:: bool _Bool_member

_Bool member

1 change: 1 addition & 0 deletions test/c/no-autoconf.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ERROR: bool.c:1: 'stdbool.h' file not found
12 changes: 12 additions & 0 deletions test/c/no-autoconf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
test:
- extension
directives:
- domain: c
directive: autodoc
arguments:
- bool.c
conf-overrides:
hawkmoth_clang: -nostdinc
hawkmoth_autoconf:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's this one failing in CI, though it passes locally. Don't have the time now (short lunch break). The idea was to add -nostdinc to normalize behaviour across distros and disable autoconf such that stdbool.h wouldn't be found that way either.

Not sure this one tests what we want to though, so I may just remove it: is it failing to find the header because we disabled auto configuring or because we set -nostdinc?

errors: no-autoconf.stderr
expected: no-autoconf.rst
8 changes: 7 additions & 1 deletion test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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([])
Expand Down Expand Up @@ -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),
Expand Down
10 changes: 8 additions & 2 deletions test/test_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ 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

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()
Expand All @@ -36,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() == ''
Expand Down Expand Up @@ -83,7 +87,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'),
Expand Down
7 changes: 6 additions & 1 deletion test/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading