-
Notifications
You must be signed in to change notification settings - Fork 940
Description
Related:
- Move env! to env::var_os for buck determinism #4578
- Lookup cross compile config paths with runtime OUT_DIR vs compile time #4555
The current implementation of pyo3_build_config::get() relies on (in order):
- looking at
DEP_PYTHON_PYO3_CONFIGenvironment variable (only set by cargo forbuild.rsfiles of crates which are direct dependents ofpyo3-ffi) - looking at
TARGETand reading a cross-compile config from a corresponding subpath ofpyo3-build-config'sOUT_DIR - use contents of
PYO3_CONFIG_FILE, if set (inlined intopyo3-build-configas part of itsbuild.rs) - use the automatically located Python installation (inlined into
pyo3-build-configas part of itsbuild.rs)
In step 2, the cross-compile config is generated by pyo3-ffi is written to pyo3-build-config's OUT_DIR. Using pyo3-build-config's OUT_DIR was chosen because we wanted to share this resolved information across all downstream uses, and only pyo3-build-config is the common dependency for pyo3_build_config::get(). However, this has problems:
- It breaks reproducible builds when doing distributed builds (root of the buck issue above)
- It breaks when the
OUT_DIRis a temporary sandbox; it's not really well specified that we should ever be abusingOUT_DIRin this way (root of the bazel issue above)
Really, all we need is a mechanism to generate a cross-compile configuration and re-use it across builds. At the moment we can't do this at the same time we inline options 3 and 4 because pyo3-build-config's build.rs doesn't know the final compilation target (pyo3-build-config is always built for the host target as a build dependency).
Unless we can think of a better mechanism to pass data from pyo3-ffi's build.rs to downstream uses in the proc macros and other crates, the only option I can think of is to add extra PYO3_CROSS_ environment variables (maybe PYO3_CROSS_TARGET) which can be used by pyo3-build-config to inline a complete config. maturin and setuptools-rust might at least be able to set these automatically.
Possibly there's an upstream feature request for cargo here which could solve the problem more cleanly in the future. But we need a solution for existing MSRV as well :(
Minor aside, Step 1 is also probably a redundant optimization because it's likely rarely hit and when present should resolve to whichever of 2, 3, or 4 was selected by pyo3-ffi's build.