ENH: capture and report build errors when rebuilding editable wheels - #859
ENH: capture and report build errors when rebuilding editable wheels#859dnicolodi wants to merge 3 commits into
Conversation
f5f1066 to
899cb5c
Compare
rgommers
left a comment
There was a problem hiding this comment.
I like the idea, and the implementation is nice and concise. A few comments around robustness (encoding, ninja errors, etc.)
| for line in log: | ||
| if line.startswith('FAILED: '): | ||
| break | ||
| error = log.read() |
There was a problem hiding this comment.
The first line that the break triggers on won't get printed here, right? Would be nice to capture that line as well.
There was a problem hiding this comment.
Leaving it out was a conscious decision. That line reads:
FAILED: [code={exitcode}] {target_name}
and I don't think it adds much information: the exit code is not relevant in the vast majority of the cases, and the target name is more often than not completely opaque to the user.
However, if you insist, I can include this line.
There was a problem hiding this comment.
To give an idea in a scikit-learn context, this line would look like this:
FAILED: [code=1] sklearn/tree/_splitter.cpython-314t-x86_64-linux-gnu.so.p/_splitter.c
I am fine with not having it, since one of the next line points directly at the Cython file (which is more useful than the ninja target I think).
/home/lesteve/dev/scikit-learn/sklearn/tree/_splitter.pyx:846:13: Syntax error in simple statement list
b41af25 to
c8de736
Compare
|
This should be ready to be merged, thus tentatively marking this to be included in the next release. |
|
@rgommers do you want to have another look at this? |
|
Hi there, thanks a lot @dnicolodi for the PR, this would be very helpful for scientific Python maintainers who are not necessarily meson-python or meson expert (my self included 😅). I gave it a go with scikit-learn and I am a bit confused because I don't get the Cython complication error in the output. I am probably missing something and I will probably try again in a few days and possibly debug what is happening. Here is what I am doing in the mean time:
And yes, in case you are wondering, I did triple-check that I was using the dev meson-python in this PR branch 😉. |
|
Thanks for testing @lesteve
And yet, you are not 😉
With this PR applied, line 346 should not look like this, actually the |
fba435c to
e8c67de
Compare
|
I've tested with a package containing Cython code and Cython errors are reported correctly: |
1912b94 to
7f7afbf
Compare
When editable-verbose is not enabled, redirect the build output to a file. When the build fails, parse this file to look for the build error and add it as a note to the ImportError exception. For Python versions that do not support exception notes, the error is appended to the exception message. Fixes mesonbuild#820.
7f7afbf to
8d1bc18
Compare
It's kind of 😭 and 🎉 at the same time. I got it to work eventually and this looks great, thanks 🙏! No idea what was going on (and I almost don't want to understand it 🙄) I decided to remove the file that the traceback was pointing to ( |
When editable-verbose is not enabled, redirect the build output to a file. When the build fails, parse this file to look for the build error and append it to the ImportError exception message.
Fixes #820.
Replaces #750.