Print meson output in case of error during editable rebuild #750

tobiasdiez · 2025-05-16T14:12:20Z

In case there is an error during the call of meson build, it currently only prints

Traceback (most recent call last):
  File "/opt/conda/envs/sage-dev/lib/python3.11/site-packages/_sagemath_editable_loader.py", line 345, in _rebuild
    result = subprocess.run(
             ^^^^^^^^^^^^^^^
  File "/opt/conda/envs/sage-dev/lib/python3.11/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/opt/conda/envs/sage-dev/bin/ninja']' returned non-zero exit status 1.

in the console. This is not very helpful to diagonize the problem. With this PR, the full output of meson is printed.

dnicolodi · 2025-05-25T12:34:52Z

The traceback reported in the PR description is not the one that is emitted upon rebuild failure of an editable package, the patch comes without a test, the patch has obviously not been tested to verify that it does what it is being claimed it does, and the CI failure has not been addressed. I'm very tempted to simply close this.

tobiasdiez · 2025-05-26T11:23:09Z

dnicolodi · 2025-05-26T23:55:21Z

The traceback reported in the PR description is not the one that is emitted upon rebuild failure of an editable package,
the patch has obviously not been tested to verify that it does what it is being claimed it does

I'm pretty sure that I got this error while there was a problem with the meson files and I've used a preliminary version of this patch to figure out what the problem was.

I'm sure that this patch does not work. In all possible code paths, the exc.output attribute is None.

the patch comes without a test

Seems like there are no tests that cover this exception, otherwise I would have changed them to the new output.

meson-python/tests/test_editable.py

Lines 317 to 346 in 352ded1

    
           @pytest.mark.parametrize('verbose', [False, True], ids=('', 'verbose')) 
        
           def test_editable_rebuild_error(package_purelib_and_platlib, tmp_path, verbose): 
        
               with mesonpy._project({'builddir': os.fspath(tmp_path)}) as project: 
        
                   finder = _editable.MesonpyMetaFinder( 
        
                       project._metadata.name, {'plat', 'pure'}, 
        
                       os.fspath(tmp_path), project._build_command, 
        
                       verbose=verbose, 
        
                   ) 
        
                   path = package_purelib_and_platlib / 'plat.c' 
        
                   code = path.read_text() 
        
                   try: 
        
                       # Install editable hooks 
        
                       sys.meta_path.insert(0, finder) 
        
                       # Insert invalid code in the extension module source code 
        
                       path.write_text('return') 
        
                       # Import module and trigger rebuild: the build fails and ImportErrror is raised 
        
                       stdout = io.StringIO() 
        
                       with redirect_stdout(stdout): 
        
                           with pytest.raises(ImportError, match='re-building the purelib-and-platlib '): 
        
                               import plat  # noqa: F401 
        
                       assert not verbose or stdout.getvalue().startswith('meson-python: building ') 
        
                   finally: 
        
                       del sys.meta_path[0] 
        
                       sys.modules.pop('pure', None) 
        
                       path.write_text(code)

rgommers · 2025-05-27T05:49:46Z

I'm sure that this patch does not work.

Indeed. I did a very quick check. Default build (no verbose otuput turned on) with main:

pytest pywt/tests/test__pywt.py
ImportError while loading conftest '/Users/rgommers/code/pywt/pywt/conftest.py'.
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:311: in find_spec
    tree = self._rebuild()
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:347: in _rebuild
    raise ImportError(f're-building the {self._name} meson-python editable wheel package failed') from exc
E   ImportError: re-building the PyWavelets meson-python editable wheel package failed

With this PR:

ImportError while loading conftest '/Users/rgommers/code/pywt/pywt/conftest.py'.
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:311: in find_spec
    tree = self._rebuild()
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:348: in _rebuild
    raise ImportError(
E   ImportError: re-building the PyWavelets meson-python editable wheel package failed with:
E   No error details available

After uninstall, export MESONPY_EDITABLE_VERBOSE=1, and reinstall, the same difference (main looks better, this PR ends in E No error details available) while the actual build problem does get revealed:

pytest pywt/tests/test__pywt.py
meson-python: building PyWavelets: /Users/rgommers/mambaforge/envs/pywt-dev/bin/ninja
[1/5] Copying file pywt/_extensions/_pywt.pyx
[2/5] Generating pywt/generate-version with a custom command
[3/5] Generating 'pywt/_extensions/_pywt.cpython-312-darwin.so.p/_pywt.c'
FAILED: pywt/_extensions/_pywt.cpython-312-darwin.so.p/_pywt.c 
/Users/rgommers/mambaforge/envs/pywt-dev/bin/cython -3 --fast-fail --output-file pywt/_extensions/_pywt.cpython-312-darwin.so.p/_pywt.c --include-dir . pywt/_extensions/_pywt.pyx -Xfreethreading_compatible=True
warning: pywt/_extensions/_pywt.pxd:15:0: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310
warning: pywt/_extensions/c_wt.pxd:89:4: The 'IF' statement is deprecated and will be removed in a future Cython version. Consider using runtime conditions or C macros instead. See https://github.com/cython/cython/issues/4310

Error compiling Cython file:
------------------------------------------------------------
...

    # Caution: order in modes list below must match _old_modes above
    modes = ["zero", "constant", "symmetric", "periodic", "smooth",
             "periodization", "reflect", "antisymmetric", "antireflect"]

    alksfj
    ^
------------------------------------------------------------

pywt/_extensions/_pywt.pyx:116:4: undeclared name not builtin: alksfj
ninja: build stopped: subcommand failed.
ImportError while loading conftest '/Users/rgommers/code/pywt/pywt/conftest.py'.
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:311: in find_spec
    tree = self._rebuild()
../../mambaforge/envs/pywt-dev/lib/python3.12/site-packages/_pywavelets_editable_loader.py:347: in _rebuild
    raise ImportError(f're-building the {self._name} meson-python editable wheel package failed') from exc
E   ImportError: re-building the PyWavelets meson-python editable wheel package failed

@tobiasdiez were you looking for MESONPY_EDITABLE_VERBOSE?

tobiasdiez · 2025-05-27T06:27:41Z

I've now found the time to properly check-out the code and run the tests.
The new version should work as expected (not sure what went wrong while copying my local edits to this PR; I'm pretty sure I had a working solution :)).

The manylinux tests fail - apparently because the wrong code in the meson file is not triggering an exception?! For me the tests pass locally. Any idea what's wrong?

rgommers · 2025-05-27T06:51:06Z

This now does show the traceback in non-verbose mode, which is an improvement, however it is swallowing the output in verbose mode (MESONPY_EDITABLE_VERBOSE) set and all I see is:

collecting ... meson-python: building PyWavelets: /Users/rgommers/mambaforge/envs/pywt-dev/bin/ninja

Output under pytest after a change to an extension module:

pytest -k concurrent
=========================================================================== test session starts ===========================================================================
platform darwin -- Python 3.12.4, pytest-8.3.2, pluggy-1.5.0
rootdir: /Users/rgommers/code/pywt
configfile: pytest.ini
collecting ... meson-python: building PyWavelets: /Users/rgommers/mambaforge/envs/pywt-dev/bin/ninja
collected 1035 items / 1031 deselected / 4 selected                                                                                                                       

pywt/tests/test_concurrent.py ....

Likely related to the "did not raise" error in CI.

dnicolodi

I am not convinced that the functionality you are adding is useful: if you want to see the compilation errors, simply enable verbose editable mode. Anyhow, the patch is still broken. Please take some time to understand what the existing code does before trying to modify it.

dnicolodi · 2025-05-27T14:27:38Z

mesonpy/_editable.py

+                        stderr=subprocess.STDOUT,
+                        env=env,
+                        check=True,
+                    )


This now hides output that was directed to standard output before.

dnicolodi · 2025-05-27T14:28:27Z

mesonpy/_editable.py

+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.STDOUT,
+                    check=True,
+                )


The compilation output can be very long. Buffering it all in memory does not seems a very good idea to me.

dnicolodi · 2025-05-27T14:30:59Z

tests/test_editable.py

+
+
+@pytest.mark.parametrize('verbose', [False, True], ids=('', 'verbose'))
+def test_editable_meson_file_rebuild_error(package_purelib_and_platlib, tmp_path, verbose):


Why are you adding this test? Why do you expect that a failure in running meson produces a different outcome that a failure in running a compilation tool?

rgommers · 2025-05-27T14:42:50Z

I am not convinced that the functionality you are adding is useful: if you want to see the compilation errors, simply enable verbose editable mode

It is useful, I have had this need several times before. Just an "editable build failed" message isn't very helpful, locally it requires an extra full rebuild after toggling the verbosity setting (which is both slow and non-obvious to the average user), and in CI it's even worse to deal with this.

Assuming that this PR is updated to not negatively affect anything else (like how verbose mode behaves), we should merge it.

Print meson output in case of error during editable rebuild

09a119a

tobiasdiez added 2 commits May 26, 2025 11:16

improve handling of no output

7b7330a

Fix linter

84b650b

fix linter part 2

bce0787

tobiasdiez added 3 commits May 27, 2025 06:06

Add and adapt tests

284ebff

Fix linter

ba938be

Use really invalid meson code to trigger exception

8cfc7e6

dnicolodi requested changes May 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Print meson output in case of error during editable rebuild #750

Print meson output in case of error during editable rebuild #750

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!



		@pytest.mark.parametrize('verbose', [False, True], ids=('', 'verbose'))
		def test_editable_meson_file_rebuild_error(package_purelib_and_platlib, tmp_path, verbose):

Print meson output in case of error during editable rebuild #750

Are you sure you want to change the base?

Print meson output in case of error during editable rebuild #750

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!