[PATCH] RFC on providing a more meaningful error message for swig-py

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] RFC on providing a more meaningful error message for swig-py

Troy Curtis Jr-2
Howdy guys,

One of the TODO items I put in the swig-py3 branch README is to provide a more useful error message if the user tries to build the swig-py bindings, but they are disabled/failed to configure. There are several failure modes that would prevent the Python binding from building, however currently if they are not successfully configured and the user types 'make swig-py' this is what they get:

$ make swig-py
/usr/bin/swig -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/subversion -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/include -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig/include -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig/proxy -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/subversion/bindings/swig/proxy -I/usr/include/apr-1  -I/usr/include/apr-1   none  -o subversion/bindings/swig/python/svn_client.c ../subversion/bindings/swig/svn_client.i
swig error : Unrecognized option none
Use 'swig -help' for available options.
make: *** [../build-outputs.mk:286: subversion/bindings/swig/python/svn_client.c] Error 1

Which isn't very helpful. I even kinda knew this, and it was still a confusing message when I first tried doing a test build on the macos BuildBot recently. (Luckily Brane dug through the log quickly and found the issue [1].)

So instead with the attached patch you get something like:

$ make swig-py
SWIG Python bindings not configured, py3c library not found
make: *** [Makefile:924: verify-swig-py] Error 1

or for the more generic case

$ make swig-py
SWIG Python bindings not configured, check config.log for details
make: *** [Makefile:924: verify-swig-py] Error 1

I think it is clear these errors are more meaningful, but I'd like thoughts on:

1) Whether I should bother
2) My implementation
3) Whether you think I should add similar functionality to the other (SWIG) bindings

Proposed patch attached.

Troy
--

swig-py-build-errmsg.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] RFC on providing a more meaningful error message for swig-py

Branko Čibej
On 04.01.2019 04:33, Troy Curtis Jr wrote:

> Howdy guys,
>
> One of the TODO items I put in the swig-py3 branch README is to provide a
> more useful error message if the user tries to build the swig-py bindings,
> but they are disabled/failed to configure. There are several failure modes
> that would prevent the Python binding from building, however currently if
> they are not successfully configured and the user types 'make swig-py' this
> is what they get:
>
> $ make swig-py
> /usr/bin/swig
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/subversion
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/include
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig/include
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/../subversion/bindings/swig/proxy
> -I/home/tscurti/working/oss/subversion/swig-py3-4/py2-build/subversion/bindings/swig/proxy
> -I/usr/include/apr-1  -I/usr/include/apr-1   none  -o
> subversion/bindings/swig/python/svn_client.c
> ../subversion/bindings/swig/svn_client.i
> swig error : Unrecognized option none
> Use 'swig -help' for available options.
> make: *** [../build-outputs.mk:286:
> subversion/bindings/swig/python/svn_client.c] Error 1
>
> Which isn't very helpful. I even kinda knew this, and it was still a
> confusing message when I first tried doing a test build on the macos
> BuildBot recently. (Luckily Brane dug through the log quickly and found the
> issue [1].)
>
> So instead with the attached patch you get something like:
>
> $ make swig-py
> SWIG Python bindings not configured, py3c library not found
> make: *** [Makefile:924: verify-swig-py] Error 1
>
> or for the more generic case
>
> $ make swig-py
> SWIG Python bindings not configured, check config.log for details
> make: *** [Makefile:924: verify-swig-py] Error 1
>
> I think it is clear these errors are more meaningful, but I'd like thoughts
> on:
>
> 1) Whether I should bother


Please do! I know I've been bitten by the same helpful message, and I
know my way around the build scripts better than the average user.


> 2) My implementation


I'm (slightly) worried about SWIG_PY_ERRMSG being set in so many places,
making it easy to miss some in future changes. But I don't have a better
idea for making the message detailed enough, the way it is now.


> 3) Whether you think I should add similar functionality to the other (SWIG)
> bindings


Please do, if you have the time.

-- Brane

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] RFC on providing a more meaningful error message for swig-py

Daniel Shahaf-2
Branko Čibej wrote on Fri, 04 Jan 2019 08:11 +0100:
> On 04.01.2019 04:33, Troy Curtis Jr wrote:
> > $ make swig-py

> > swig error : Unrecognized option none

> > So instead with the attached patch you get something like:
> >
> > $ make swig-py
> > SWIG Python bindings not configured, py3c library not found
> > make: *** [Makefile:924: verify-swig-py] Error 1

> > 1) Whether I should bother
>
>
> Please do! I know I've been bitten by the same helpful message, and I
> know my way around the build scripts better than the average user.
>

+1, those "none: command not found" errors are unhelpful and a bug.

We already handle this for the «@SWIG@ = none» case (see
build/generator/templates/build-outputs.mk.ezt:87) but it would be nice to
handle other cases as well.