cli: add --profile option and MATURIN_PROFILE var

[davidhewitt]

See PyO3/pyo3#2105

This adds --profile option, which forwards to cargo --profile. In addition, the MATURIN_PROFILE environment variable is added which sets the same value from environment instead of cli. This is helpful when using pip install with the PEP 517 backend to override the default of release build.

The env var may not be necessary - it's possible that we can use the PEP 517 config_settings to take build arguments like --profile, which would be an alternative to passing with an env var. At the moment, however, this dict is unused.

Please let me know how you want this tested and / or documented, and I'll update this PR.

[netlify]

✔️ Deploy Preview for maturin-guide canceled.

🔨 Explore the source changes: b009978

🔍 Inspect the deploy log: https://app.netlify.com/sites/maturin-guide/deploys/61e52e8e954bd3000768f930

[messense]

I like this idea. Since it's not a commonly used option and it simply forwards to cargo --profile, document it in cli help text should be sufficient for now.

[davidhewitt]

Changed to use conflicts_with, and expanded the help text slightly.

[messense]

Odd, it seems that Rust 1.58 increased glibc version requirement?

  process didn't exit successfully: `/github/workspace/test-crates/pyo3-mixed/target/debug/build/pyo3-build-config-4cecb176e201b772/build-script-build` (exit status: 1)
  --- stderr
  /github/workspace/test-crates/pyo3-mixed/target/debug/build/pyo3-build-config-4cecb176e201b772/build-script-build: /lib64/libc.so.6: version `GLIBC_2.15' not found (required by /github/workspace/test-crates/pyo3-mixed/target/debug/build/pyo3-build-config-4cecb176e201b772/build-script-build)
  /github/workspace/test-crates/pyo3-mixed/target/debug/build/pyo3-build-config-4cecb176e201b772/build-script-build: /lib64/libc.so.6: version `GLIBC_2.14' not found (required by /github/workspace/test-crates/pyo3-mixed/target/debug/build/pyo3-build-config-4cecb176e201b772/build-script-build)

[davidhewitt]

Added env = settings.

Hmm, looks like GLIBC wasn't actually increased; there's a note about it here... https://github.com/rust-lang/rust/blob/master/RELEASES.md#compatibility-notes

[messense]

Interesting, other PRs are not failing, for example, #779.

[konstin]

I wonder if a separate --profile make sense vs. just using --cargo-extra-args. cargo build currently has 38 different options and that would make maturin builds cli rather unwieldy. (I've also been thinking about maybe adding -- syntax to replace --cargo-extra-args, like maturin build -i python -- --profile coverage --features abi3 --out-dir my-target-dir, but I've never checked how well this works with clap and rustc args). Currently I've only added those where we parse the value in maturin

The env var may not be necessary - it's possible that we can use the PEP 517 config_settings to take build arguments like --profile, which would be an alternative to passing with an env var. At the moment, however, this dict is unused.

Do you know if there currently is any way to actually use config_settings? Last time I checked it wasn't usable with pip and the general from the python packaging discourse consensus was that it's effectively unspecified so we're not doing anything with it.

For PyO3/pyo3#2105, could you explain what the MATURIN_PROFILE is doing? I didn't understand it from looking at the diff

[messense]

I wonder if a separate --profile make sense vs. just using --cargo-extra-args. cargo build currently has 38 different options and that would make maturin builds cli rather unwieldy.

We certainly shouldn't mirror all of them from cargo build, but I do think that mirroring some of them, for example --features/--no-default-features/--all-features, makes maturin build more convenient for end user.

[davidhewitt]

Do you know if there currently is any way to actually use config_settings? Last time I checked it wasn't usable with pip and the general from the python packaging discourse consensus was that it's effectively unspecified so we're not doing anything with it.

Yes, this is what I've found out too from further reading. It sounds like config_settings doesn't work, so in general we currently don't have a way to pass options to maturin during PEP 517 build. If you don't like having a MATURIN_PROFILE env var an alternative could be a MATURIN_PEP517_ARGS environment variable which adds arguments to the maturin pep517 Rust internal command.

For PyO3/pyo3#2105, could you explain what the MATURIN_PROFILE is doing?

and that loops back to the point of the env var:

• for the coverage to be measured correctly, I need everything to be built in --debug mode (--profile dev)
• there isn't a way to control the maturin build during a PEP 517 build (I'm using pip install -e . with maturin as the backend specified in the pyproject.toml)

... so MATURIN_PROFILE env was a solution to passing exactly the instruction to build in debug.

Now that PyO3 has migrated to nox, I might try a different solution which doesn't use pep 517 build for the coverage, in which case I won't need exactly this solution, however we could leave a follow-up issue to figure out a design for that.

I wonder if a separate --profile make sense vs. just using --cargo-extra-args

This is a fair point. Note that because maturin passes --release automatically in some contexts, the user cannot override the --profile without also figuring out how to invoke maturin to not pass the flag:

$ cargo build --release --profile dev
error: conflicting usage of --profile=dev and --release
The `--release` flag is the same as `--profile=release`.
Remove one flag or the other to continue.

[konstin]

I really like the MATURIN_PEP517_ARGS idea!

This is a fair point. Note that because maturin passes --release automatically in some contexts, the user cannot override the --profile without also figuring out how to invoke maturin to not pass the flag:

Great point, make sense

[messense]

Hmm, looks like GLIBC wasn't actually increased; there's a note about it here... https://github.com/rust-lang/rust/blob/master/RELEASES.md#compatibility-notes

Looks like it's only failing when cargo cache misses, this PR also fails. I suspect that Rust 1.58 did increased GLIBC version requirement somehow. I'll try downgrading to 1.57.

maturin/.github/workflows/test.yml

Line 151 in a38b03a

args: -c "PATH=/github/workspace/.cargo/bin:$PATH tests/manylinux_compliant.sh ${{ matrix.manylinux }}"

It could be that we are using a Rust toolchain installed on Ubuntu 20.04 in old CentOS docker container. 😅

Should be fixed in #783

[ionenwks]

I really like the MATURIN_PEP517_ARGS idea!

Just to say I very much would like that too. I want to use pep517 to ease packaging in e.g. Gentoo ebuilds, but the difficulty in passing arguments been a limiting factor. Would want to be able to pass args to maturin pep517 build-wheel too and not just rust/cargo.

[davidhewitt]

👍 I think for the moment I'm going to close this PR, as I'm stretched quite thin so don't have the time to rebuild this as MATURIN_PEP517_ARGS. I managed to get what I needed into PyO3 thanks to us switching to nox (and then I changed to use maturin develop).

[davidhewitt]

(Anyone is welcome to claim MATURIN_PEP517_ARGS from me and PR it 🙂)