Improve the error reporting in meson

[alexandre-janniaux]

Hi,

~This MR is in a RFC state for now, many things need to be improved before merging,~ but I'd like to have feedback on how the error is improved and how to handle the line reporting itself before.

This MR stems from a very simple case where I found the error reporting very confusing with the following code:

if host_system = 'android'

When running meson, we would only get the following confusing error:

meson.build:6:17: ERROR: Cannot perform 'if' operation on void statement.

This MR thrive to add the necessary infrastructure and convention to report better errors with a way to solve the issue. Typically, the previous example would now output the following error:

meson.build:6:3: ERROR: Assignment are returning void and not valid in if clause.
meson.build:6:3:  | if host_system = 'android'
meson.build:6:3:  |    ^^^^^^^^^^^^^^
meson.build:6:3:  |    Did you forget a = here?

The current implementation is conservative to where the errors were not extended to this new information.

Many questions are arising from that regarding the formatting:

• [ ] Should we append meson.build:6:3: in front of every line or lighten the presentation?
• [ ] Should we append the line number on location where there is code?
• [ ] How should we handle multi-line errors?
• [ ] Should we test those errors and the span (which looks wrong here) in the CI?

I also implemented the line reporting in an ugly way, by storing the relevant line in every (well for now, only IfClauseNode and AssignmentNode, but that's one of the hypothesis of the code) AST node, but some nodes are spanning on multiple lines and there can be multiple nodes per line, so it looks a bit ugly even though we can make python use a single string in the back. We can also only report the relevant part of the code by referencing self.code[] or referencing the relevant substring when raise is happening. What would you prefer?

[xclaesse]

Better error reporting is always great.

Played a bit with wrong statements in if condition, the problem is not limited to assignment:

• if message('hello') -> ERROR: Cannot perform 'if' operation on void statement..
• if fs.copyfile('foo.c') -> ERROR: Object <CustomTargetHolder foo.c@cus: ['/home/xclaesse/.local/bin/meson', '--internal', 'copy', '@INPUT@', '@OUTPUT@']> of type CustomTarget does not support the bool() operator.

[alexandre-janniaux]

Played a bit with wrong statements in if condition, the problem is not limited to assignment:

Of course yes, as of now this is just a PoC, I'd like to improve where the error marker lands also so that it's easier to track with the eyes, and improve the line reporting since every node would need to include that, which is a bit ugly as-is.

Thanks a lot for some additional checks! That's very interesting given we'll typically need to unwind back in the evaluation for the fs.copyfile back to improve the reporting. Would you like to improve the errors after this message yourself or should I include that in the MR?

[tristan957]

I like this idea. Developers love when tooling is very explicit and makes their live easier, which this does. Thanks for taking this on. FYI you can mark PRs as drafts too.

I recently started learning Typst, and the compiler is amazing with errors.

[tristan957]

All the linting jobs are failing

[alexandre-janniaux]

I've addressed the issues from the two linters, but there are still issues with mypy, mostly because of kwargs. I've made the necessary changes to make it work on recent python, but I'm not sure of what should be done. The remaining issues are because the needed type annotations are not available:

mesonbuild/utils/core.py:39:32: error: Name "T.TypedDict" is not defined  [name-defined]
mesonbuild/utils/core.py:40:11: error: Name "T.NotRequired" is not defined  [name-defined]
mesonbuild/utils/core.py:41:13: error: Name "T.NotRequired" is not defined  [name-defined]
mesonbuild/utils/core.py:42:12: error: Name "T.NotRequired" is not defined  [name-defined]
mesonbuild/utils/core.py:54: error: Name "T.Unpack" is not defined  [name-defined]
mesonbuild/interpreterbase/interpreterbase.py:76: error: Name "T.Unpack" is not defined  [name-defined]

Should I try to get rid of the necessity for kwargs?

[tristan957]

cc @dcbaker

[alexandre-janniaux]

For now, I've switched the **kwargs to T.Any for now, I'll wait for python 3.12 to improve the parameter typing for the exceptions.

I've also fixed the lint errors, and changed the error reporting for void function inside if conditions so that it works with every void function, but now they will be evaluated before. (Unfortunately, I don't have any other solution to get the type for now).

I have a few changes on commit message to make and the mlog patches to re-improve, and an issue with the project with assignment where the carets are not displayed correctly, and this should be good to go.

[tristan957]

Test failures.

[alexandre-janniaux]

Unfortunately, I'm not sure to understand the remaining errors:

CI:

=================================== FAILURES ===================================
___________ LinuxlikeTests.test_generate_gir_with_address_sanitizer ____________
[gw2] linux -- Python 3.11.6 /usr/bin/python3

self = <unittests.linuxliketests.LinuxlikeTests testMethod=test_generate_gir_with_address_sanitizer>

    @skip_if_not_base_option('b_sanitize')
    def test_generate_gir_with_address_sanitizer(self):
        if is_cygwin():
            raise SkipTest('asan not available on Cygwin')
        if is_openbsd():
            raise SkipTest('-fsanitize=address is not supported on OpenBSD')
    
        testdir = os.path.join(self.framework_test_dir, '7 gnome')
        self.init(testdir, extra_args=['-Db_sanitize=address', '-Db_lundef=false'])
>       self.build()

unittests/linuxliketests.py:318: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.11/subprocess.py:550: in run
    stdout, stderr = process.communicate(input, timeout=timeout)
/usr/lib/python3.11/subprocess.py:1209: in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
/usr/lib/python3.11/subprocess.py:2109: in _communicate
    self._check_timeout(endtime, orig_timeout, stdout, stderr)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <Popen: returncode: -9 args: ['/usr/bin/ninja', '-w', 'dupbuild=err', '-d', ...>
endtime = 865.065316947, orig_timeout = 300
stdout_seq = [b"ninja explain: deps for 'resources/simple-resources-test.p/meson-generated_.._simple-resources.c.o' are missing\nni...rated-resources-test.p/generated-main.c.o is dirty\nninja explain: resources/generated-resources-test is dirty\n", ...]
stderr_seq = None, skip_check_and_raise = False

    def _check_timeout(self, endtime, orig_timeout, stdout_seq, stderr_seq,
                       skip_check_and_raise=False):
        """Convenience for checking if a timeout has expired."""
        if endtime is None:
            return
        if skip_check_and_raise or _time() > endtime:
>           raise TimeoutExpired(
                    self.args, orig_timeout,
                    output=b''.join(stdout_seq) if stdout_seq else None,
                    stderr=b''.join(stderr_seq) if stderr_seq else None)
E           subprocess.TimeoutExpired: Command '['/usr/bin/ninja', '-w', 'dupbuild=err', '-d', 'explain']' timed out after 300 seconds

/usr/lib/python3.11/subprocess.py:1253: TimeoutExpired
...
FAILED unittests/linuxliketests.py::LinuxlikeTests::test_generate_gir_with_address_sanitizer - subprocess.TimeoutExpired: Command '['/usr/bin/ninja', '-w', 'dupbuild=err', '-d', 'explain']' timed out after 300 seconds

Local:

╰─$ python run_unittests.py -v LinuxlikeTests.test_generate_gir_with_address_sanitizer
 ____________

Meson build system 1.4.99 Unit Tests
================================================================================================ test session starts ================================================================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/alexandre/workspace/videolabs/meson
configfile: setup.cfg
plugins: xdist-3.5.0
collected 514 items / 513 deselected / 1 selected                                                                                                                                                                   

unittests/linuxliketests.py::LinuxlikeTests::test_generate_gir_with_address_sanitizer PASSED                                                                                                                  [100%]

================================================================================================= warnings summary ==================================================================================================
unittests/linuxliketests.py::LinuxlikeTests::test_generate_gir_with_address_sanitizer
unittests/linuxliketests.py::LinuxlikeTests::test_generate_gir_with_address_sanitizer
unittests/linuxliketests.py::LinuxlikeTests::test_generate_gir_with_address_sanitizer
  /home/alexandre/workspace/videolabs/meson/mesonbuild/utils/universal.py:1507: EncodingWarning: UTF-8 Mode affects locale.getpreferredencoding(). Consider locale.getencoding() instead.
    encoding = locale.getpreferredencoding()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================== 1 passed, 513 deselected, 3 warnings in 4.96s ===================================================================================
Total time: 5.199 seconds
zsh: command not found: ____________

Is it really related to my changes?

[alexandre-janniaux]

I do have an error with testsetup and test_cross_pkg_config_option, but they are also happening from the master branch for me too:

======================================== short test summary info =========================================
FAILED unittests/linuxcrosstests.py::LinuxCrossMingwTests::test_cross_pkg_config_option - subprocess.CalledProcessError: Command '['/usr/bin/python', '/home/alexandre/workspace/videolabs/meso...
FAILED unittests/allplatformstests.py::AllPlatformTests::test_testsetups - AssertionError: 'TEST_ENV is set' not found in 'Log of Meson test suite run on 2024-03-18T08:35:42.23...

[alexandre-janniaux]

testsetup was failing because I needed to export a valid DEBUGINFOD url, it's ok now.

I guess the pkg_config test is failing because it's using env[PKG_CONFIG]: /usr/bin/x86_64-w64-mingw32-pkg-config which will redirect to my system sysroot on archlinux. Since the test seems to be providing a dependency in a different sysroot, the dependency cannot be found.

[tristan957]

When do you think this PR will be ready for review?

[dcbaker]

If you're not ready for review or to land could you convert the PR to a draft? that helps people know what the current state of the PR is

[alexandre-janniaux]

I've removed the warning. I think that the MR is ready, now that I could check in the test what kind of result it would output and understand the contribution process of meson a bit more, but I'm still open to change if that's requested.

[alexandre-janniaux]

Ok, after rebasing another time, the tests are now passing.

[tristan957]

Looks like tests are still failing. A cleaned up git history would also be appreciated!

[alexandre-janniaux]

Looks like tests are still failing. A cleaned up git history would also be appreciated!

Indeed, I've pushed to synchronize between my two machines but the MR is back to not being 100% ready. I've added error handling in mparser.py in this MR because of test that were now failing (from the Failing/ directory, but still failing because they were not matching the expected output).

It led to a few fixes on the original MR though, which is nice. I've also changed the behaviour of the caret addition to default to one caret, as it hugely simplify when only a single caret needs to be added and avoid underlining the whole line if the end column is not yet specified.

I'm still working on the intermediate patches so that it doesn't break the code in the middle of the branch. Thanks for checking the MR! I'll post again when this is finished.