Possible breaking change in pyo3-build-config from 0.20.0 0.20.1?

[max-sixty]

I haven't looked into this deeply, and possibly dependabot has done something screwy with the lockfile — but this patch upgrade seems to raise an error in PRQL/prql#4033

The error is at https://github.com/PRQL/prql/actions/runs/7378972354/job/20074705518?pr=4033

[alex]

You'll want to upgrade pyo3 and pyo3-ffi as well

…

On Mon, Jan 1, 2024 at 12:58 PM Maximilian Roos ***@***.***> wrote: I haven't looked into this deeply, and possibly dependabot has done something screwy with the lockfile — but this patch upgrade seems to raise an error in PRQL/prql#4033 <PRQL/prql#4033> The error is at https://github.com/PRQL/prql/actions/runs/7378972354/job/20074705518?pr=4033 — Reply to this email directly, view it on GitHub <#3719>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBFLQQ6ON3PTC5QCFFDYML2NLAVCNFSM6AAAAABBJF72WWVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA3DCNRZGA3DGMY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

[max-sixty]

OK, we'll do that.

Though it does seem like a breaking change fwiw

(feel free to close)

[adamreichold]

I don't think we want to commit to full semver compatibility for these internal API, but I do think we should use more specific version in our dependencies, e.g. pyo3-ffi should depend on

version = "=0.20.1"

instead of

version = "0.20.1"

as it does now, so that these helper crates will always be updated in lock step.

[max-sixty]

internal API

How are you thinking about what's internal? AFAIU, the common meaning is "internal to the crate"...

Should we not be specifying this crate explicitly here?

[adamreichold]

Should we not be specifying this crate explicitly here?

You need to as you make use of the add_extension_module_link_args workaround to support building without Maturin/setuptools-rust on macOS.

How are you thinking about what's internal? AFAIU, the common meaning is "internal to the crate"...

This is conventionally true but PyO3 is in a bit of a difficult situation as we have multiple API which are strictly speaking public, but which are only so because they are internal support structures for the code generated by our macros within downstream crates. We usually mark these items as #[doc(hidden)] to indicate that we do not adhere to semver compatibility for them (which after is all is highly regarded and useful convention, but not a functional requirement).

Similarly, the pyo3-build-config crate is primarily a means to centralize information on the build configuration so that both the pyo3 library crate (in its build script) as well as the proc macros in pyo3-macros (when generating code) can access it in any uniform manner. That downstream code sometimes needs to call into a directly to workaround integration issues is unfortunate, but getting around this for the sake of semver purity is not always worth it even though the approach is complicated by the absence of a #[doc(hidden)] equivalent for a whole crate.

[max-sixty]

OK thanks!

(not sure if you're looking for feedback on your comment — if you are — totally empathize with not investing in semver purity, but then you might want to think about doing more minor releases. There are lots of crates that only do minor releases. My understanding is that the core difference is signifying whether there are breaking changes, which in this case there do seem to be, based on the general definition of breaking change...)

[ribeaud]

Hi. Not 100% whether my problem relates to this issue, or whether my observation is relevant to you, but we have following CI job on our end: cargo +nightly -Zdirect-minimal-versions update && cargo build. Recently it started to throw following error:

   Compiling pyo3-ffi v0.20.0
error[E0599]: no method named `emit_pyo3_cfgs` found for struct `InterpreterConfig` in the current scope
  --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pyo3-ffi-0.20.0/build.rs:96:24
   |
96 |     interpreter_config.emit_pyo3_cfgs();
   |                        ^^^^^^^^^^^^^^ method not found in `InterpreterConfig`
For more information about this error, try `rustc --explain E0599`.

The solution on our side is to specify the patch version for pyo3: 0.20.1. But I think, the preferred way to go would be to continue to not specify the last number. What do you think?

I tried to put all necessary information here.

[davidhewitt]

It probably is; I suspect with -Zdirect-minimal-versions you're having pyo3 and pyo3-ffi downgraded but not pyo3-build-config.

(not sure if you're looking for feedback on your comment — if you are — totally empathize with not investing in semver purity, but then you might want to think about doing more minor releases. There are lots of crates that only do minor releases. My understanding is that the core difference is signifying whether there are breaking changes, which in this case there do seem to be, based on the general definition of breaking change...)

Feedback is welcome. We indeed try to use minor vs patch releases to signify breaking changes, even though semver before 1.0 has no standard rules. However, to clarify, while this was technically breaking, we made this in a hidden API and the Rust community has generally agreed that hidden APIs are semver exempt.

And to be clear, the fact that we made a change to this API broke you is straight up a bug. I think we're looking at doing a 0.20.2 release so I'll see if there's a way that this can be fixed.

[davidhewitt]

Will reopen this just to remind me to think of there's a way to backport for the patch.

[davidhewitt]

Ok I think this will be fixed by #3724 which we'll ship shortly as a pyo3-build-config 0.20.2.

[adamreichold]

Ok I think this will be fixed by #3724 which we'll ship shortly as a pyo3-build-config 0.20.2.

I think we should backport the definite version specifications as well, as there should be no mixing and matching of the pyo3-* helper crates.

[max-sixty]

Thanks!