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.

Sign up for GitHub

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

Jump to bottom

Pyodide: test packages may be installed off-sys.path #1853

Closed
QuLogic opened this issue Jun 5, 2024 · 9 comments · Fixed by pyodide/pyodide#4844
Closed

Pyodide: test packages may be installed off-sys.path #1853

QuLogic opened this issue Jun 5, 2024 · 9 comments · Fixed by pyodide/pyodide#4844

Comments

@QuLogic
Copy link

QuLogic commented Jun 5, 2024

Description

As #1456 was recently merged, I've been testing out whether we can use it with Matplotlib. In order to do so, I've set CIBW_BEFORE_TEST='pip install pytest' CIBW_TEST=pytest .... This correctly installs pytest, the just-built matplotlib, and all of its dependencies.

However, matplotlib and all its archful dependencies cannot be found by python in the test venv. If I check sys.path, it ends with a path like /tmp/cibw-run-yt4_gwiv/cp312-pyodide_wasm32/venv-test/lib/python3.12/site-packages only. However, all the archful dependencies were installed in /tmp/cibw-run-yt4_gwiv/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages.

This is possibly due to use of the system Python in some way; on Fedora which I use, pure-Python packages go in .../lib/... and archful in .../lib64/.... I'm not sure though if the bug is in cibuildwheel, pyodide venv, or somewhere else.

Build log

No response

CI config

No response
@henryiii
Copy link
Contributor

henryiii commented Jun 5, 2024

In general, you should not be installing with pip in before-test, but you should use test-requires or test-extras instead.

Pyodide is 32-bit, so not sure why lib64 would be used? Also, I would expect we are not modding pip to support pyodide?

@QuLogic
Copy link
Author

QuLogic commented Jun 5, 2024

In general, you should not be installing with pip in before-test, but you should use test-requires or test-extras instead.

Ah, good point; it's not too important here though as the package-to-be-tested is the one that's in the wrong spot. The pytest package is installed in the right place and available (if I were to run it on something pure Python.)

Pyodide is 32-bit, so not sure why lib64 would be used? Also, I would expect we are not modding pip to support pyodide?

I wouldn't expect it either, but I guess somewhere in setting up the venv, something from the system environment is leaking through? From what I understood of the implementation in #1456, the test command should setup a venv with pyodide and install the built-package there, and I do see this from the log.

For example, if I change test command to just python -c "import matplotlib" (and drop any before-test), I get:

Testing wheel...

+ pyodide venv /tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/venv-test
Creating Pyodide virtualenv at /tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/venv-test                                                                                                                               
... Configuring virtualenv                                                                                                                                                                                         
... Installing standard library                                                                                                                                                                                    
Successfully created Pyodide virtual environment!                                                                                                                                                                  
+ which python
/tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/venv-test/bin/python
+ pip install /tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/repaired_wheel/matplotlib-3.10.0.dev245+g3a09643649.d20240605-cp312-cp312-pyodide_2024_0_wasm32.whl
Looking in indexes: https://pypi.org/simple, file:///home/elliott/.cache/cibuildwheel/.pyodide-xbuildenv-0.26.0/0.26.0/xbuildenv/pyodide-root/package_index
Processing /tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/repaired_wheel/matplotlib-3.10.0.dev245+g3a09643649.d20240605-cp312-cp312-pyodide_2024_0_wasm32.whl
Collecting contourpy>=1.0.1 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/contourpy-1.2.1-cp312-cp312-pyodide_2024_0_wasm32.whl
Collecting cycler>=0.10 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/cycler-0.12.1-py3-none-any.whl
Collecting fonttools>=4.22.0 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached fonttools-4.53.0-py3-none-any.whl.metadata (162 kB)
Collecting kiwisolver>=1.3.1 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/kiwisolver-1.4.5-cp312-cp312-pyodide_2024_0_wasm32.whl
Collecting numpy>=1.23 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/numpy-1.26.4-cp312-cp312-pyodide_2024_0_wasm32.whl
Requirement already satisfied: packaging>=20.0 in /tmp/cibw-run-_iasczwc/cp312-pyodide_wasm32/venv-test/lib/python3.12/site-packages (from matplotlib==3.10.0.dev245+g3a09643649.d20240605) (23.2)
Collecting pillow>=8 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/pillow-10.2.0-cp312-cp312-pyodide_2024_0_wasm32.whl
Collecting pyparsing>=2.3.1 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/pyparsing-3.1.2-py3-none-any.whl
Collecting python-dateutil>=2.7 (from matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/python_dateutil-2.9.0.post0-py2.py3-none-any.whl
Collecting six>=1.5 (from python-dateutil>=2.7->matplotlib==3.10.0.dev245+g3a09643649.d20240605)
  Using cached https://cdn.jsdelivr.net/pyodide/v0.26.0/full/six-1.16.0-py2.py3-none-any.whl
Using cached fonttools-4.53.0-py3-none-any.whl (1.1 MB)
Installing collected packages: six, pyparsing, pillow, numpy, kiwisolver, fonttools, cycler, python-dateutil, contourpy, matplotlib
Successfully installed contourpy-1.2.1 cycler-0.12.1 fonttools-4.53.0 kiwisolver-1.4.5 matplotlib-3.10.0.dev245+g3a09643649.d20240605 numpy-1.26.4 pillow-10.2.0 pyparsing-3.1.2 python-dateutil-2.9.0.post0 six-1.16.0
+ python -c "import matplotlib"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'matplotlib'

If I check the venv directory noted above, only the pure Python packages are there; archful went into lib64.

@QuLogic
Copy link
Author

QuLogic commented Jun 5, 2024

Also, I just remembered, on a native Python, the venv is created with a lib64 -> lib symlink so that everything ends up in the same directory. Maybe that's what's missing?

@henryiii
Copy link
Contributor

henryiii commented Jun 5, 2024

Might be a quick and easy thing to check? You can make a symlink in before-pytest? If that fixes it, maybe we/pyodide (@hoodmane) should add it? Though I expect it's an issue with the trickery done to get pip to target pyodide.

@QuLogic
Copy link
Author

QuLogic commented Jun 5, 2024

Yes, I've added CIBW_BEFORE_TEST='cd $(dirname $(dirname $(which python))); ln -s lib lib64' and then CIBW_TEST_COMMAND='python -c "import matplotlib"' works, so this might be an easy workaround.

@hoodmane hoodmane mentioned this issue Jun 7, 2024
Closed
@hoodmane
Copy link
Contributor

hoodmane commented Jun 7, 2024

I grabbed a fedora docker image and am trying this out.

hoodmane added a commit to hoodmane/pyodide that referenced this issue Jun 7, 2024
@hoodmane
Set platlibdir in venv pip
71adab0 
Previously we'd read it from the host environment, which could cause trouble in
environments like Fedora where it would have an unusual value.
Resolves pypa/cibuildwheel#1853
hoodmane added a commit to hoodmane/pyodide that referenced this issue Jun 7, 2024
@hoodmane
Set platlibdir in venv pip
4d0f1e9 
Previously we'd read it from the host environment, which could cause trouble in
environments like Fedora where it would have an unusual value.
Resolves pypa/cibuildwheel#1853
@hoodmane hoodmane mentioned this issue Jun 7, 2024
Merged
2 tasks
hoodmane added a commit to pyodide/pyodide that referenced this issue Jun 7, 2024
@hoodmane
Set platlibdir in venv pip (#4844)
0f2ee22 
Previously we'd read it from the host environment, which could cause trouble in environments
like Fedora where it would have an unusual value. Resolves pypa/cibuildwheel#1853
@hoodmane
Copy link
Contributor

hoodmane commented Jun 7, 2024

Okay this will be fixed once we roll out Pyodide 0.26.1 and cibuildwheel switches to it. Thanks so much for the report @QuLogic!

hoodmane added a commit to hoodmane/pyodide that referenced this issue Jun 7, 2024
@hoodmane
Set platlibdir in venv pip (pyodide#4844)
52d7d5c 
Previously we'd read it from the host environment, which could cause trouble in environments
like Fedora where it would have an unusual value. Resolves pypa/cibuildwheel#1853
@henryiii
Copy link
Contributor

henryiii commented Jun 8, 2024

In main now.

ryanking13 pushed a commit to pyodide/pyodide-build that referenced this issue Jun 10, 2024
@hoodmane
Set platlibdir in venv pip (#4844)
aabd2bb 
Previously we'd read it from the host environment, which could cause trouble in environments
like Fedora where it would have an unusual value. Resolves pypa/cibuildwheel#1853
@henryiii
Copy link
Contributor

Should be in 2.19!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set platlibdir in venv pip hoodmane/pyodide
3 participants
@QuLogic @henryiii @hoodmane