Repository navigation
Conversation
savannahostrowski
left a comment
There was a problem hiding this comment.
I checked the current build-details.json generation and couldn’t find any existing values under arbitrary_data; this appears to be its first use in CPython. Is the intent to provide a CPython-specific interface now and defer a standardized build-details.json field to a future format version?
If so, could we document the intended semantics and stability of extension_compiler_flags in the docs, including how consumers should expect it to relate to a standardized field later?
It may also be worth clarifying whether entries in this field are required, recommended, or potentially both, since the proposed field does not distinguish between them.
5d7e1c3 to
4ad2da4
Compare
Documentation build overview
|
|
CC @rgommers @dnicolodi @mgorny @eli-schwartz @henryiii @davidhewitt @messense @FFY00 @abravalheri @mayeut @sicherha @Viicos, could you chime in on what interface would work best for your build systems and packages for PEP 831? This PR currently exposes the recommended frame-pointer compiler flags through Following up on the work in PyO3/maturin#3323, PyO3/setuptools-rust#621, pydantic/pydantic#13896, mesonbuild/meson-python#829, and the discussion in pypa/manylinux#1970. I'd like to make sure the interface is useful to the tools consuming it before we settle it. Thanks! |
| "arbitrary_data": { | ||
| "extension_compiler_flags": [], | ||
| }, |
There was a problem hiding this comment.
We can just omit if it's known to be empty, surely?
There was a problem hiding this comment.
not opposed but why would that be preferable?
There was a problem hiding this comment.
File size is the most concrete reason, but it'd also help flush out any code that assumes it's going to be there "because it's always there". Users of the value will need a fallback anyway, so they may as well discover it sooner.
I also agree with @mgorny that we should probably do a proper update to add more (optional) fields to the other sections, rather than arbitrary_data. There are gaps, and the PEP is written in a way to require updates (or a new PEP?) to add the things we need (import libraries path and name, IIRC, in addition to the others).
There was a problem hiding this comment.
Thanks! I'm happy to omit the empty field and work through a spec update for the interface.
Just to clarify the motivation: PEP 831 already recommends enabling frame pointers in extensions and native dependencies. Backends can do that today with the appropriate flags for their toolchain, without waiting for this PR. We're trying to make their lives easier by exposing the relevant information without making them extract it from CPython's full CFLAGS. The question here is how best to represent that in build-details.json. I would prefer if this issue doesn't balloon to trying to fix a much bigger problem.
|
Well, first things first: I'm slightly confused as to where we stand with "flags used to compile extensions" here. FWICS setuptools uses As for the change itself, I'd rather avoid introducing custom extensions for it, and instead update the specification. This really sounds like something potentially valid for the That said, we already have a bunch of bugs reported for |
|
Thanks @mgorny. Yes, native and cross builds should have a consistent way to get these recommendations. I'm happy to put this in the specification rather than PEP 831 already recommends enabling frame pointers in extensions and native dependencies; backends can use the appropriate flags today. This PR is meant to make that easier without inheriting all of CPython's For the interface, would flags under |
|
One thing I'd like to keep focused here is getting the frame-pointer recommendation to build backends in a form they can use. I'm happy to make the spec changes needed for that, but I'd prefer to track the other |
|
From my POV, you should just add it to the best section for it (with documentation) and treat the PEP as the original documentation for others to follow and not actually the design specification for ourselves. |
On this, I think putting the verbatim options is best. In the simple case, it gets appended to a Keep as close to |
|
One reservation I have from the build backend side of things is that it's not fully clear to me how to wire this up while at the same time allowing users to choose. My understanding from the PEP is that this is "just" about ensuring that globally distributed binaries (Linux distro packages, conda, cibuildwheel outputs published to PyPI, etc.) don't hinder adoption for users that need to do tracing. These "good defaults" should therefore be getting applied when building redistributable wheels. Ideally we'd have some mechanism for determining when a wheel is intended for redistribution. There's currently no real way to know this, other than I guess "groups doing distribution at scale usually set CFLAGS themselves". Relatedly, what should complex compiled projects do when they have internal libraries of e.g. scientific code that get built as a static library, and then linked into python bindings? The vast bulk of the code isn't a python extension and isn't built with any cpython flags whatsoever. Only the binding code is. So you get frame pointers for the binding code but immediately hit a dead end because the implementation is missing them... In meson it is generally recommended for target cleanliness to logically group compilation units by purpose, and often that will be a "subproject" which you don't even develop yourself, but rather use via a git submodule. None of it uses Python.h and the upstream build definitions (which are directly invoked in a subproject build) don't link to the python dependency (and are probably created by people who don't know python at all). It is "just" a C++ static library. Where should these flags be added? |
|
Thanks @eli-schwartz. PEP 831 enables frame pointers by default in CPython and strongly recommends the same throughout Python's native ecosystem, including extensions and their dependencies. The recommendation also covers local builds. Each build system and project needs to work out how to implement it across its builds; the PEP cannot prescribe that for every setup. We expect this to take time, and that's fine. The goal remains frame pointers throughout the native ecosystem. Your static-library example is a real limitation. The setting needs to reach those libraries and subprojects when they are compiled, since adding flags only to the bindings leaves gaps in stack unwinding. Dependencies supplied as binaries would need to be rebuilt by whoever distributes them. There are examples to follow. Ubuntu and Fedora already adopted frame-pointer defaults. Ubuntu originally exempted Python; upstream CPython 3.15 now enables them. python-build-standalone's change adds them to shared target flags, which dependency builds such as SQLite also consume. Ideally, manylinux and other dependency-build pipelines would apply this to the libraries they build too. That will require coordination and rebuilding over time. For Meson, would a build-wide setting that reaches subprojects be workable? This PR is intended to make the relevant information available to backends, while projects work through how to apply it to their dependencies. |
Uh oh!
There was an error while loading. Please reload this page.