examples: split maturin and setuptools-rust examples

[davidhewitt]

This PR makes two copies of the existing rustapi_module example, called maturin_extension and setuptools_rust_extension.

Each example is currently a full copy of the other, except for different build configuration.

To make the output structure the same and importing work the same way, I found a solution to #759 - see the register_submodules bits of the code.

This needs cleaning up and documenting better. We might also want to reduce the amount of code duplication (into a new maturin module, perhaps)?

Currently I think I'm hitting a bug in maturin failing to take cargo workspaces into account. I'm going to see if there's a fix for that I can contribute to.

Closes #807

[kngwyu]

So we keep two copies of the same example?
If we need to test something specific to maturin, it would be better to create a small example for that purpose.

[davidhewitt]

So we keep two copies of the same example?
If we need to test something specific to maturin, it would be better to create a small example for that purpose.

Agreed; the way I see this WIP PR evolving is:

1. I first want to make sure that the full example works exactly the same with setuptools-rust and maturin
2. I then trim down a lot of the duplicated code (probably make these two examples super simple) and put most of the tests in a new location (perhaps tests/?)

[davidhewitt]

Looks like I have the following issues still:

• pypy support 😢
• windows 32-bit - maturin not respecting CARGO_BUILD_TARGET for some reason 🤔
• MSRV not supported - I think looking at the error we require at least rust 1.40 for #[cfg(doctest)].

I will continue to work through these.

Looks like RHEL 8.3 is released and includes rust 1.45, so actually it might be possible to do a bit of upgrading and cleanup! https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/8.3_release_notes/overview

[Progdrasil]

I've also just tested the solution you gave on my current project. I noticed here you were using add_wrapped(wrap_pymodule!) and not the more recent add_submodule which is the recommended way in the docs now.

I mention this because using the add_wrapped works beautifully, but add_submodule gives me the following error if i were to change misc in your maturin example.

import maturin_extension.misc
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: dynamic module does not define module export function (PyInit_misc)

changes made for this:

examples/maturin_extension/src/misc.rs

pub fn misc( m: &PyModule) -> PyResult<()> {
    m.add_function(wrap_pyfunction!(issue_219, m)?)?;
    Ok(())
}

examples/maturin_extension/src/lib.rs

#[pymodule]
fn maturin_extension(_py: Python, m: &PyModule) -> PyResult<()> {
   /*... */
//    m.add_wrapped(wrap_pymodule!(misc))?;

    let misc_mod = PyModule::new(_py, "misc")?;
    misc(misc_mod)?;
    m.add_submodule(misc_mod)?;

    Ok(())
}

[konstin]

huh what's that doing? I wasn't aware that maturin needs that kind of workarounds

[davidhewitt]

It's to enable importing directly from a submodule.

E.g. this always works:

import maturin_extension
maturin_extension.datetime.foo()

But the following only works with the SubmoduleFinder

import maturin_extension.datetime
datetime.foo()
# or
from maturin_extension.datetime import foo
foo()

However I'm beginning to think this was perhaps not a good idea: it re-runs the #[pymodule] init code which means that it's possible to create two separate modules:

>>> import maturin_extension
>>> dt = maturin_extension.datetime
>>> import maturin_extension.datetime
>>> dt == maturin_extension.datetime
False

That it's possible to create two different copies of the submodule is a bit... horrible. So I'm going to remove this from the example code.

[davidhewitt]

I mention this because using the add_wrapped works beautifully, but add_submodule gives me the following error if i were to change misc in your maturin example.

@Progdrasil interesting - I think add_submodule will also work if you keep #[pymodule] on the submodule functions. However see my other comment #1269 (comment) - I'm not so sure that allowing importing in this way is a good idea...

[davidhewitt]

😭 I can't reproduce this pypy failure locally at all.

Looks like it is tox which is removing the CARGO_BUILD_TARGET environment variable, so I'm unsure what to do about that also.

[alex]

tox has a an option called `passenv` which tells it to let the env var from outside be passed in.

…

On Sun, Dec 20, 2020 at 9:14 AM David Hewitt ***@***.***> wrote: 😭 I can't reproduce this pypy failure locally at all. Looks like it is tox which is removing the CARGO_BUILD_TARGET environment variable, so I'm unsure what to do about that also. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBDMK7QBVRX46BFEU43SVYBDPANCNFSM4TQZMGTA> .

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

[davidhewitt]

I'm going to be force-pushing to this branch a few times today to try to diagnose what this PyPy failure that I can't reproduce locally is. Sorry for any spam.

[davidhewitt]

Replaced with #1537