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

Maturin isn't packaging submodule stubs along with the rest of the code #771

Closed
1 of 2 tasks
oyarsa opened this issue Jan 11, 2022 · 3 comments · Fixed by #772
Closed
1 of 2 tasks

Maturin isn't packaging submodule stubs along with the rest of the code #771

oyarsa opened this issue Jan 11, 2022 · 3 comments · Fixed by #772
Labels
bug Something isn't working

Comments

@oyarsa
Copy link

oyarsa commented Jan 11, 2022

Bug Description

Maturin isn't packaging submodule stubs along with the rest of the code when using the default configuration.

To reproduce this, take a look at https://github.com/oyarsa/maturin-submodule-stub/ and run the run_test.sh file at the repository root. It will:

  1. Build the maturin module from rust, then test the module and type checking from there.
    This will work, since the module was installed in editable module and the script will go
    directly to the original directory.
  2. Install the maturin module as a dependency from python. The script will successfully
    execute because the module was installed correctly, but type-checking will fail. Listing
    the files in the site-packages/test_module directory shows that the .pyi files were
    not moved there.

This is the file structure:

.
├── python
│  ├── requirements.txt
│  ├── run_test.sh
│  └── test_stub.py
├── run_test.sh
└── rust
   ├── Cargo.lock
   ├── Cargo.toml
   ├── pyproject.toml
   ├── run_test.sh
   ├── src
   │  ├── lib.rs
   │  └── python.rs
   ├── test_module
   │  ├── __init__.py
   │  ├── test_module
   │  │  ├── __init__.pyi
   │  │  └── sub_module.pyi
   │  └── test_module.cpython-38-x86_64-linux-gnu.so
   └── test_stub.py

This is the full output:

+ python3 -m venv .venv
+ set +x
+ echo '# Building module'
# Building module
+ maturin develop
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.8 at /home/italo/dev/random/maturin-submodule-stub/rust/.venv/bin/python
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
📦 Built wheel for CPython 3.8 to /tmp/.tmpcQNhU2/test_module-0.1.0-cp38-cp38-linux_x86_64.whl
🛠  Installed test-module-0.1.0
+ echo -e '\n# Testing module'

# Testing module
+ python test_stub.py
Hello world
+ echo -e '\n# Testing mypy'

# Testing mypy
+ mypy test_stub.py
test_stub.py:7: note: Revealed type is "def (x: Union[builtins.str, None]) -> Union[builtins.str, None]"

----------------------


+ echo '# Installing module'
# Installing module
+ python3 -m venv .venv
+ set +x
+ pip install -r requirements.txt
Processing /home/italo/dev/random/maturin-submodule-stub/rust
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
  Preparing metadata (pyproject.toml): started
  Preparing metadata (pyproject.toml): finished with status 'done'
Requirement already satisfied: mypy in ./.venv/lib/python3.8/site-packages (from -r requirements.txt (line 2)) (0.931)
Requirement already satisfied: tomli>=1.1.0 in ./.venv/lib/python3.8/site-packages (from mypy->-r requirements.txt (line 2)) (2.0.0)
Requirement already satisfied: typing-extensions>=3.10 in ./.venv/lib/python3.8/site-packages (from mypy->-r requirements.txt (line 2)) (4.0.1)
Requirement already satisfied: mypy-extensions>=0.4.3 in ./.venv/lib/python3.8/site-packages (from mypy->-r requirements.txt (line 2)) (0.4.3)
Building wheels for collected packages: test-module
  Building wheel for test-module (pyproject.toml): started
  Building wheel for test-module (pyproject.toml): finished with status 'done'
  Created wheel for test-module: filename=test_module-0.1.0-cp38-cp38-manylinux_2_24_x86_64.whl size=937036 sha256=ea96b9423adc8c28a99aac96fafb6011a45d08af2d9cbcb232c53ca5976a04bd
  Stored in directory: /tmp/pip-ephem-wheel-cache-9_38_hw5/wheels/e7/02/84/5da1a60d7e59ceac2da0c110bbc0cb76b261b720422e644932
Successfully built test-module
Installing collected packages: test-module
  Attempting uninstall: test-module
    Found existing installation: test-module 0.1.0
    Uninstalling test-module-0.1.0:
      Successfully uninstalled test-module-0.1.0
Successfully installed test-module-0.1.0
+ echo -e '\n# Testing module'

# Testing module
+ python test_stub.py
Hello world
+ echo -e '\n# Testing mypy'

# Testing mypy
+ mypy test_stub.py
test_stub.py:2: error: Skipping analyzing "test_module": module is installed, but missing library stubs or py.typed marker
test_stub.py:2: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
test_stub.py:7: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)
+ echo -e '\n# Content of site-pacakges/test_module:'

# Content of site-pacakges/test_module:
+ ls .venv/lib/python3.8/site-packages/test_module
__init__.py
__pycache__
test_module.cpython-38-x86_64-linux-gnu.so


Your Python version (python -V)

Python 3.8.10

Your pip version (pip -V)

20.0.2

What bindings you're using

pyo3

Does cargo build work?

  • Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

  • Yes

Steps to Reproduce

Reproduction steps on https://github.com/oyarsa/maturin-submodule-stub/

@oyarsa oyarsa added the bug Something isn't working label Jan 11, 2022
@messense
Copy link
Member

Thanks for a great bug report.

Running maturin build and list the files in the built wheel show that the test_module/test_module isn't packaged:

❯ unzip -l dist/test_module-0.1.0-cp39-cp39-macosx_10_7_x86_64.whl
Archive:  dist/test_module-0.1.0-cp39-cp39-macosx_10_7_x86_64.whl
  Length      Date    Time    Name
---------  ---------- -----   ----
       78  01-12-2022 13:21   test_module-0.1.0.dist-info/METADATA
      104  01-12-2022 13:21   test_module-0.1.0.dist-info/WHEEL
       45  01-12-2022 13:21   test_module/__init__.py
  1231064  01-12-2022 13:21   test_module/test_module.cpython-39-darwin.so
      399  01-12-2022 13:21   test_module-0.1.0.dist-info/RECORD
---------                     -------
  1231690                     5 files

@messense
Copy link
Member

// Ignore the cffi folder from develop, if any
if relative.starts_with(module_name.as_ref().join(&module_name)) {
continue;
}

It was skipped by the code above, @konstin can you explain more about it? I'm not sure how to fix it without breaking the previous behavior for cffi.

@konstin
Copy link
Member

konstin commented Jan 12, 2022

I think this was from the old implementation of develop where the files were added to the source directory instead of the venv, I think you can just delete that code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants