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

Specifying the minimum supported python version #1195

Closed
konstin opened this issue Sep 16, 2020 · 12 comments
Closed

Specifying the minimum supported python version #1195

konstin opened this issue Sep 16, 2020 · 12 comments

Comments

@konstin
Copy link
Member

konstin commented Sep 16, 2020

Currently, there is no way to specify minimal supported python version, and compiling against a too low version or fail with an error about missing symbols (or worse, at runtime due to some failed import). With maturin, I'd like to build abi3 wheels for the lowest supported python version, so there's only one wheel and that supports all versions. I'm currently thinking about how to best specify this. Ideally, pyo3's build script or at least setuptools-rust should also check for this version and exit with an understandable error message about the wrong python version (I can write the pull request).

I currently have three different ideas:

  • In the cargo metadata section. The big plus is that user already configure a Cargo.toml.
  • In pyproject.toml. We could start in the tools section and switch over the official python-requires when PEP 621 is accepted.
  • Feature flags. Add py36-abi3, py37-abi3, etc. features

@kngwyu @davidhewitt What do you think?

@davidhewitt
Copy link
Member

Is it possible to use metadata from the package itself in pyo3's build.rs? Even if it is I'm not enthusiastic about doing that. OTOH Maturin could definitely check the cargo metadata, and setuptools-rust the pyproject.toml.

I wonder if we can add macros like rustversion ?

@kngwyu
Copy link
Member

kngwyu commented Sep 17, 2020

I think the combination of feature flags (for build scripts) + toml config (for maturin or setuptools) is good, though we have to invert unstable-api flags to do so.

I wonder if we can add macros like rustversion ?

We already have #[cfg(Py_3_7)] or so for that purpose.

@kngwyu kngwyu mentioned this issue Sep 19, 2020
6 tasks
@davidhewitt
Copy link
Member

We already have #[cfg(Py_3_7)] or so for that purpose.

#[cfg(Py_3_7)] etc. only works inside of the pyo3 code though; downstream crates don't see these flags unless we give them a way to add them in their own build scripts.

@konstin
Copy link
Member Author

konstin commented Oct 4, 2020

I checked that only setting the cfg's in build.rs works nicely with abi3. I've just checked with this hacky snippet:

fn main() -> Result<()> {
    if env::var_os("CARGO_FEATURE_ABI3").is_some() {
        let msg = r#"cargo:rustc-cfg=Py_3_5
cargo:rustc-cfg=Py_3_6
cargo:rustc-cfg=Py_3_7
cargo:rustc-cfg=Py_3
cargo:rustc-cfg=py_sys_config="WITH_THREAD"
cargo:python_flags=FLAG_WITH_THREAD=1,CFG_Py_3_5,CFG_Py_3_6,CFG_Py_3_7"#;
        println!("{}", msg);
        return Ok(());
    }

The really cool part about this is that the targeted python version doesn't need to be installed on the build machine anymore, which means e.g. faster and easier CI.

I also realized that one big disadvantage of the toml solutions would be that we would need to add a toml parser as build dependency.

@kngwyu kngwyu added this to the 0.13 milestone Oct 30, 2020
@kngwyu
Copy link
Member

kngwyu commented Oct 30, 2020

I think we can employ basically the same strategy as @konstin 's snippet, but what I'm not sure is whether we should support the combination of abi3 and minimum required Python version (e.g., abi3 build, but does not support Python 3.6).
Any idea?
cc: @alex

@alex
Copy link
Contributor

alex commented Oct 30, 2020

I'm not sure I understand the question. Can you try rephrasing it?

@kngwyu
Copy link
Member

kngwyu commented Oct 30, 2020

Ah, sorrry, but I noticed that my original question meant nothing so please ignore it.

@kngwyu
Copy link
Member

kngwyu commented Oct 30, 2020

@konstin

The really cool part about this is that the targeted python version doesn't need to be installed on the build machine anymore, which means e.g. faster and easier CI.

You mean we can build py35-abi3 using Python 3.6 or 3.7?

@alex
Copy link
Contributor

alex commented Oct 30, 2020

Being able to build an ABI3 wheel which targets a lower ABI than the Python you're building with would be cool. (e.g. Using Python 3.8 to build an abi3 wheel with 3.5 as the minimum version) However, it's not a hard requirement for me. Not sure what the best API for it would be.

@kngwyu kngwyu mentioned this issue Oct 31, 2020
1 task
@konstin
Copy link
Member Author

konstin commented Nov 10, 2020

Being able to build an ABI3 wheel which targets a lower ABI than the Python you're building with would be cool. (e.g. Using Python 3.8 to build an abi3 wheel with 3.5 as the minimum version) However, it's not a hard requirement for me. Not sure what the best API for it would be.

Even better, with abi3 you should be able to build without any python version present! That's what I tried to do with the snippet I posted above, where I only set rust config and don't read or link any python.

@davidhewitt
Copy link
Member

@konstin now that #1263 is merged, would be interesting to hear how this works for you in maturin!

@davidhewitt davidhewitt modified the milestones: 0.13, 0.14 Dec 22, 2020
@konstin
Copy link
Member Author

konstin commented Jan 14, 2021

Thank you @kngwyu and @davidhewitt for all the help and work!

Being able to build an ABI3 wheel which targets a lower ABI than the Python you're building with would be cool. (e.g. Using Python 3.8 to build an abi3 wheel with 3.5 as the minimum version) However, it's not a hard requirement for me. Not sure what the best API for it would be.

As a follow-up, on linux/mac you can build now without any python version, while on windows you need a python interpreter for linking with its python3.lib, where the python version shouldn't matter.

@konstin konstin closed this as completed Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants