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

Use wheels instead of file_packager #2027

Merged
merged 237 commits into from
Jan 24, 2022
Merged

Use wheels instead of file_packager #2027

merged 237 commits into from
Jan 24, 2022

Conversation

hoodmane
Copy link
Member

@hoodmane hoodmane commented Dec 10, 2021

Use wheels. Resolves #655.

In the process, I rewrote most of load-pyodide, eliminating all the remaining black magic and confusing control flow.

It is worth pointing out that it is perfectly possible now to make package.json point at pypi, and by doing this get automatic installation into the repl for a preselected collection of pypi packages. The only difference between loadPackage and micropip is the dependency resolution. loadPackage is designed for "static" dependency resolution, whereas micropip is designed for "dynamic" dependency resolution. For this reason, it might be good to encourage use of loadPackage. I think ideally what we could do is make micropip.freeze which dumps a new packages.json with the currently loaded set of packages added in. Then a user can get the setup they want and freeze it so they won't have to redo dependency resolution every time their app starts.

@hoodmane
Copy link
Member Author

In particular, I think loadPackage should be changed to only load packages in package.json. For loading anything else, we should use micropip. This will lead to a clear distinction in purposes between these two tools.

@hoodmane hoodmane marked this pull request as draft December 10, 2021 02:37
@hoodmane
Copy link
Member Author

Probably we need an RFC for this of some sort.

@hoodmane
Copy link
Member Author

Screenshot from 2021-12-10 08-54-24
Most of the packages are building correctly.

@hoodmane
Copy link
Member Author

I don't think the needs_rebuild command is working correctly. It seems to rebuild stuff a lot even when it isn't needed.

@hoodmane
Copy link
Member Author

Okay at least locally all 89 package builds are working.

@hoodmane
Copy link
Member Author

@rth @ryanking13 Okay, tests pass again. Any further comments? How do you feel about merging this?

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "src/tests/data/test-scipy.{whl,tar.bz2}" files could be removed if they are not used?

Otherwise LGTM. Thanks!

Copy link
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your huge effort! I have some minor comments, otherwise LGTM. And please update the changelog for this.

conftest.py Outdated Show resolved Hide resolved
packages/test_packages_common.py Outdated Show resolved Hide resolved
packages/distlib/meta.yaml Show resolved Hide resolved
@ryanking13
Copy link
Member

We should also update the documentation which refers .data, but I think we could do that in a separate PR since this PR is already big enough.

@hoodmane hoodmane merged commit 8323987 into pyodide:main Jan 24, 2022
@hoodmane hoodmane deleted the wheels branch January 24, 2022 01:47
@rth
Copy link
Member

rth commented Jan 26, 2022

I really like that one can now just call less to list files in a wheel or .tar file, and zcat to get wheels contents. It makes debugging so much easier. Thanks again @hoodmane !

@henryiii
Copy link
Contributor

henryiii commented Feb 11, 2022

Is there a documented process somewhere for a user to build wheels? If I have a simple C/C++ package, could I produce one of these wheels now? (And ideally, the emscripten_wasm32 tag should be allowed in PyPI, is that being pursued? Edit: Ahh, I see the compatibility issue, makes sense, probably needs some sort of ABI numbering or something in the wheel, like manylinux. Still very interested in the wheel production process!)

Edit: Having a few issues following it, but https://pyodide.org/en/stable/development/new-packages.html was what I was looking for. pip install doesn't work inside the docker container, for example. ruamel.yaml is a dependency but not listed in pyodide-build. Now I get it hanging, and it's always stuck here:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/src/pyodide-build/pyodide_build/__main__.py", line 62, in <module>
    main()
  File "/src/pyodide-build/pyodide_build/__main__.py", line 56, in main
    args.func(args)
  File "/src/pyodide-build/pyodide_build/buildpkg.py", line 821, in main
    build_package(
  File "/src/pyodide-build/pyodide_build/buildpkg.py", line 641, in build_package
    with chdir(pkg_root), get_bash_runner() as bash_runner:
  File "/usr/local/lib/python3.9/contextlib.py", line 117, in __enter__
    return next(self.gen)
  File "/src/pyodide-build/pyodide_build/buildpkg.py", line 109, in get_bash_runner
    b.run(f"source {PYODIDE_ROOT}/emsdk/emsdk/emsdk_env.sh", stderr=subprocess.DEVNULL)
  File "/src/pyodide-build/pyodide_build/buildpkg.py", line 80, in run
    self.env = json.loads(self._reader.readline())
KeyboardInterrupt

@rth
Copy link
Member

rth commented Feb 11, 2022

Is there a documented process somewhere for a user to build wheels?

For pure Python packages, it's just regular universal Python wheels.
For packages with C extensions, you can run inside the pyodide Docker image,

PYODIDE_PACKAGES="<package-name>" make

see the dev docs https://pyodide.org/en/latest/development/building-from-sources.html and https://pyodide.org/en/latest/development/new-packages.html. We are trying to migrate to a better build setup & CLI. latest vs stable in the URL to get the dev docs, since this feature is not yet included in a release.
This assumes though that you have a meta.yaml package defining the package and meta.yaml for its dependencies.

And ideally, the emscripten_wasm32 tag should be allowed in PyPI, is that being pursued?

Yes, see pypi/warehouse#10416 . Though it's likely a multi-year project. The feedback we got from core devs is that first CPython needs to officially support WASM (which hopefully would happen for the next release), then there need to be a PEP and enough traction/usage for Python with WASM from the community. So the short term goal is just to deploy wheels to some other wheel services (or keep our custom deployment). A bit similar to what is done for https://github.com/antocuni/pypy-wheels

Having the ability to build wheels for WASM as part of cibuildwheel would be indeed ideal. I'll comment more on that in pypa/cibuildwheel#1002

@henryiii
Copy link
Contributor

I'd be quite fine with that - calling this still exploratory, keeping the ABI from gaining requirements quite yet. I think it still could be "standalone" via cibuildwheel & probably docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Binary wheels for pyodide?
4 participants