Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ext_modules in static setup.cfg #2220

Closed
embray opened this issue Jun 26, 2020 · 25 comments · Fixed by #4568
Closed

Add support for ext_modules in static setup.cfg #2220

embray opened this issue Jun 26, 2020 · 25 comments · Fixed by #4568

Comments

@embray
Copy link
Contributor

embray commented Jun 26, 2020

In the process of porting an old project from the now defunct d2to1 I noticed a feature of d2to1 that is still missing from setuptools is support for configuring extension modules in setup.cfg.

At least, that's what my reading of the source suggests, especially ConfigOptionsHandler

In d2to1 this looks something like:

[extension=<module_name>]
sources = <file1> <file2> ...
language = c
<keyword> = <value>

where each key/value pair correspond to setuptools.Extension arguments.

For setuptools it might look something more like:

[options.ext_modules.<module_name>]
<key>=<value>

Though I'm open to other suggestions for the format, and would be happy to provide a PR once we agree on and nail down a format.

@jaraco
Copy link
Member

jaraco commented Jul 4, 2020

Thanks for the report. Your instinct seems right here.

I worry a little bit about the arbitrarily-named sections. Is there precedent for that? A quick look through the code, I suspect the options.ext_modules.<module_name> would be handled by the ConfigOptionsHandler.

Yes, I think it could work.

I'm a little concerned about the . being used to designate the setuptools configuration namespace, but also the user's own namespace (module name), and wonder if the separator should be different to indicate a different naming domain, maybe something like [options.ext_modules:<module_name>] (=, ;, - also seem like suitable separators; I'll refrain from suggesting the separator-that-shall-not-be-named). It's probably not super-important what the separator is unless it's possible that the module_name could contain the separator.

Quickly scanning the docs, I see that pkg.foo is a possible module name, so I'd avoid using . to separate options.ext_modules from the module name. What do you think about :?

Feel free to get started on a PR; I'm hopeful there are few decisions to be made before an implementation can be accepted.

@embray
Copy link
Contributor Author

embray commented Jul 6, 2020

I agree with your point about using a different delimiter to separate the config namespace from the module name. I'm fine with :, I think it's a good choice.

One problem with all this I realized, is that while it would work fine for simple cases, I noticed that it's still fairly common in several of my packages to involve a little bit of code in configuring extension modules. Most commonly was using numpy.get_include() to get the path to the Numpy headers. Another is using cythonize to build Cython modules. In d2to1 I believe I included special cases for both of these, but hard-coding special cases doesn't scale well.

Of course, this could get arbitrarily complex to the point where not every case can be supported in a static config. I think there could be a way for code to provide extensions to the syntax while still allowing most of the configuration to be static. But maybe as a first pass it would be best to just implement the "obvious" static configuration and then figure out more complex cases later.

@samvv
Copy link

samvv commented Jun 8, 2021

Does there happen to be a roadmap for this? I'd be very interested to see this happen.

@jaraco
Copy link
Member

jaraco commented Jul 16, 2021

Another is using cythonize to build Cython modules.

I believe Setuptools has built-in support for building Cython modules without anything extra. Just needs (a) cython installed, which is possible through build-requires and even legacy setup_requries, and (b) the source is a .pyx file. I'd hope that would cover a lot of cases.

Most commonly was using numpy.get_include() to get the path to the Numpy headers.

Numpy is important enough that it might deserve its own support... although I would be interested in exploring a generalizable technique for soliciting includes, independent of the configuration mechanism. For example, if Setuptools could solicit an entry point for setuptools.build_dirs or similar, then numpy (or perhaps a separate numpy-build) could implement setuptools.build_dirs as numpy.get_include(), thereby enabling the behavior simply by including the library in the build (no two-step process). In any case, let's explore the zero-config ideas before committing to a syntax in the config.

Does there happen to be a roadmap for this? I'd be very interested to see this happen.

No roadmap; just what you see above.

@bostonrwalker
Copy link

@embray I like this feature idea a lot. Did you start any work on it?

If possible, you should also be able to compile and include C libraries through the "libraries" argument.

@eggplants
Copy link

eggplants commented May 16, 2022

+1

It will help to greatly reduce the code from setup.py, especially for projects that provide C bindings.

@tvladyslav
Copy link

Would like to have this feature for C bindings.

@bostonrwalker
Copy link

I created a PR to add a similar feature to Python Poetry here. The PR is ready but the maintainers have said they don't have enough time to review it. If you guys want to have that functionality in Poetry, please go over there and help bug the maintainers for me.

@eli-schwartz
Copy link
Contributor

It's not clear to me why setuptools users care what some other tool does? How does this help achieve a world where setuptools can encode ext_modules statically?

@bostonrwalker
Copy link

It's not clear to me why setuptools users care what some other tool does? How does this help achieve a world where setuptools can encode ext_modules statically?

I made this out of frustration with the lack of this feature originally in setuptools. You can feel free to take note and maybe create something similar in setuptools. Or not.

@eli-schwartz
Copy link
Contributor

Since the poetry developers rejected the feature, maybe it would be interesting to rewrite it as a contribution to setuptools instead?

@jaraco
Copy link
Member

jaraco commented Dec 18, 2022

I took a very brief look at the poetry proposal. Yikes! 43 files changed to add support for ext_modules? Not to impugn the motives of the contributor, but I can see why such a change might be daunting. I'm imagining here that an initial contribution to Setuptools would affect 2-5 files. I'd very much be interested in seeing at least a basic implementation that satisfies cases similar to what d2to1 did. You may want to reference the poetry PR, but I'd advise not to try to port it.

@eli-schwartz
Copy link
Contributor

To be fair, it was only 6 files if you don't count the directory of test fixtures. :) Granted, the actual core change was still a diffstat of +394, -9.

Seems like the poetry developers were more daunted by the idea of causing people to depend on the setuptools API (?) which would prevent "more generic" tools.

@jaraco
Copy link
Member

jaraco commented Dec 18, 2022

Seems like the poetry developers were more daunted by the idea of causing people to depend on the setuptools API (?) which would prevent "more generic" tools.

Thanks for the summary. I agree, it's probably wise for them not to rely on Setuptools APIs. I would like to expose the distutils.compilers and other extension-supporting APIs outside of Setuptools and distutils, but when I set out to do that, I immediately ran into trouble due to how setuptools vendors distutils (there's no clean way to extract a dependency).

@boston-zesty
Copy link

@eli-schwartz Poetry already relies very heavily on setuptools to package distributions, so IMO adding an additional dependency on setuptools in a sub-step of the packaging process shouldn't rule out consideration of a new feature in anticipation of a big effort to remove setuptools that they may or may not complete one day.

And thanks for the suggestion, I am planning to look into it during my Christmas vacation.

@boston-zesty
Copy link

@jaraco @eli-schwartz And thanks for looking at my PR. I appreciate not dismissing it from an armchair. The json validation schema alone is +157 lines, so the true core diff is closer to (+237, -9), which is big but in my view not unreasonable to implement a complex new feature.

If I take a crack at it in setuptools I will make sure to keep complexity to a minimum. I don't think most of the Python code can be "ported" since the files and functions changed are unique to Poetry's packaging pipeline.

@eli-schwartz
Copy link
Contributor

Thanks for the summary. I agree, it's probably wise for them not to rely on Setuptools APIs.

You're welcome for the summary. Although actually I disagree (hence my question mark when mentioning it).

I think it is perfectly reasonable to use the setuptools APIs. It's at least as reasonable as any project writing their own setup.py and specifying setuptools as their build backend!

Note that they specifically mentioned the (in)ability for poetry to change away from setuptools after users start to rely on it.

I simply don't see how poetry providing their own configuration approach and guaranteeing nothing beyond "this will produce a compiled extension through whatever mechanism poetry pleases to use internally" causes projects to rely on the setuptools API. Poetry simply needs to make sure that the internal implementation detail is isolated and doesn't leak (for example, do not let projects control it by importing their own Extension objects). This would leave poetry free to refactor their approach to use any alternately end up preferring.

It is of course entirely possible that they are concerned about the internal maintainability of poetry itself if it becomes tied to setuptools API (more than it is now?) but that's an entirely different point and IMHO it's not too hard to simply rip everything out as long as the initial implementation is reasonably isolated.

So I don't think it's too problematic for poetry or other tools to rely on setuptools.setup() for now and switch over to another compilation backend when that becomes available and desirable (even to distutils.compilers if/when the cleanup for that is completed). At least it's not going to break users!

@guywilsonjr
Copy link

The poetry maintainers make me sad. Especially the part about too many people relying on it. Yikes!

@jaraco

This comment was marked as off-topic.

@tvladyslav

This comment was marked as off-topic.

@jaraco

This comment was marked as off-topic.

@jaraco
Copy link
Member

jaraco commented Jul 20, 2024

Restating from my hidden comment:

I'd still love to see some static config for extension modules. I'd also like to consider some automatic detection such that static configuration is unnecessary for many cases (e.g. each C file in the package represents a module, or a particular arrangement of C files can represent a module).

@webknjaz
Copy link
Member

FTR, there's a related proposed PoC @ https://ofek.dev/extensionlib/. Not sure if it's been discussed on this tracker yet.

@eli-schwartz
Copy link
Contributor

That proposal is not related, as far as I'm aware. That proposal suggests an API for defining a PEP 517 build backend, which implements "extensionlib" as a mechanism for using pyproject.toml static configuration to declare that C-API extension "foobar" located in directory packagename/ gets built by running setuptools.setup() and expecting it to create packagename/foobar.*.so.

It is designed to let people build part of their project with cmake, part of their project with meson, part of their project with mypyc, part of their project with maturin, and part of their project with setuptools.

@abravalheri
Copy link
Contributor

Supported was added not exactly for setup.cfg, but for pyproject.toml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.