Integrate cffi-buildtool into CFFI itself#241
Conversation
This adds code and documentation that was originally written by Rose Davidson (@inklesspen on GitHub) for the cffi-buildtool project: https://github.com/inklesspen/cffi-buildtool
|
Just commenting here to confirm I give my full support for this upstreaming, and after a release is cut which includes these changes, I will mark my existing project as deprecated. Also I'm available to answer any questions about the original code upon which this is based. |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
Some thoughts that occurred to me since yesterday and a typo fix
|
There is a clang-tsan failure |
|
Sorry for taking so long to come back to this. I think this is ready for another round of review now. |
mattip
left a comment
mattip
left a comment
There was a problem hiding this comment.
This seems very helpful, and will provide a path to move away from distutils/setuptools only cffi support.
Some of my comments might miss the mark. In general I think the concept of "build" is not clear enough. A cffi out-of-line module has three parts: generate C code, compile the C code into a c-extension, and import the module. Does "build" address all three?
An in-line mode module (no compiler used) is different, I assume this does not address the inline-mode usage.
|
I had a read through out of interest; overall this LGTM. The CLI taking the two main build modes and source as input, generated C code as output is useful and matches how Cython, f2py and other such codegen/binding tools are integrated into meson-python and scikit-build-core; the pattern should indeed work for other build backends that are able to build extension modules from C code as well. |
Thanks for the review comments! I'm going to apply the review suggestions that are pending now, pull, and address the remaining comments. Co-authored-by: Matti Picus <matti.picus@gmail.com>
|
I think I've responded to all the review comments. I'd appreciate another look :) |
mattip
left a comment
mattip
left a comment
There was a problem hiding this comment.
Sorry to be nit-picking but cffi is confusing anyway, and I think it is made more so by the use of generic “build” when a more specific concept is intended. As I understand it the intention of the tool is to expose the c-code generation mechanism provided by cffi. Then the c code is handed over to a Python compilation framework like meson or setuptools, and the resulting c-extension module can be packaged into a wheel by a wheel producing framework. I would be happier if the tool was called gen-cffi-code or gen-c-code and not buildtool. Additionally, it would be nice to scan for “build” and use a better word instead where possible.
Does that make sense or is it too restrictive?
| <https://scikit-build-core.readthedocs.io/>`_, or similar. | ||
|
|
||
| Installing CFFI installs the ``gen-cffi-src`` script; running ``python | ||
| -m cffi.buildtool`` invokes the same command line and behaves |
There was a problem hiding this comment.
I still find the name buildtool confusing. What does the module actually do? It does not replace mason/setuptools, does it do more than generate c code?
| ---------------------------- | ||
|
|
||
| This mode takes a Python script that dynamically defines an FFI interface and | ||
| accompanying C extension source code. The FFI definition script is the same |
There was a problem hiding this comment.
And does what exactly? What is the output?
| using | ||
| ref | ||
| cdef | ||
| buildtool |
There was a problem hiding this comment.
Can you change the references to not use the word “build”?
| buildtool | |
| gen-cffi-src |
| interface. Two subcommands: | ||
|
|
||
| ``exec-python`` | ||
| Execute a Python build script that constructs a :class:`cffi.FFI` |
There was a problem hiding this comment.
| Execute a Python build script that constructs a :class:`cffi.FFI` | |
| Execute a Python script that constructs a :class:`cffi.FFI` |
| describes) and emit the generated C source. | ||
|
|
||
| ``read-sources`` | ||
| Build the :class:`cffi.FFI` from a separate ``cdef`` file and C |
There was a problem hiding this comment.
| Build the :class:`cffi.FFI` from a separate ``cdef`` file and C | |
| Create the :class:`cffi.FFI` from a separate ``cdef`` file and C |
"buildtool" is the filename, the executable is now called |
Co-authored-by: Matti Picus <matti.picus@gmail.com>
|
I just pushed a commit that globally renames |
|
Perfect. Thanks! |
4 participants
Closes #47. Revives #76.
This adds a new submodule for cffi named
cffi.buildtooland a new CLI script distributed by CFFI namedgen-cffi-src. Also adds new tests and documentation. See the new documentation and tests for usage details.I realize that this is a big change that makes a number of opinionated choices. I'm very happy to adjust things to suit the maintainers' taste.
My ultimate goal here is to make it possible for projects using CFFI to use arbitrary build tools and remove the perceived tight coupling between CFFI and setuptools/distutils. If there isn't appetite to add the code in cffi-buildtool to CFFI itself, I'd also happily document how to depend on and use cffi-buildtool instead.
See inklesspen/cffi-buildtool#2 where I got @inklesspen's blessing to do this.
The is based on code and documentation that were originally written by Rose Davidson (@inklesspen on GitHub) for the cffi-buildtool project: https://github.com/inklesspen/cffi-buildtool.