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

Add test files unvendoring functionality #41

Open
agriyakhetarpal opened this issue Jan 23, 2025 · 4 comments
Open

Add test files unvendoring functionality #41

agriyakhetarpal opened this issue Jan 23, 2025 · 4 comments

Comments

@agriyakhetarpal
Copy link
Member

Recently, I provided pyodide-build's unvendor_tests() functionality as a separate unvendor_tests_from_wheel.py CLI script as a part of sympy/live#26, to reduce the size of the SymPy wheel downloaded from PyPI.

Would it be a good idea for pyodide-pack to include such functionality as well, so that it can be used as a CLI tool for downstream projects like this?

As a further step, would it make sense to remove this functionality from pyodide-build, and make it call pyodide-pack as a part of the wheel build process in https://github.com/pyodide/pyodide-recipes?

@ryanking13
Copy link
Member

Thanks for the proposal @agriyakhetarpal!

I made a similar issue in the pyodide-lock repository: pyodide/pyodide-lock#30, because I thought the test unvendoring feature was related to the lockfile.

But if you think it can be used not only in the lockfile, but also in general purpose (reducing the size of the wheel file), I think it is a good idea to put it under pyodide-pack as pyodide-pack is designed for various optimizations.

We've considered moving the pyodide py-compile CLI to pyodide-pack as well.

@ryanking13
Copy link
Member

This issue is also worth reading: pyodide/pyodide#3092. Of course, this is a separate discussion from moving the test unvendoring feature, but it would be helpful to be able to generalize the "unvendoring" in the future.

@agriyakhetarpal
Copy link
Member Author

I've been thinking about what a proper interface would be for unvendoring test files through pyodide-pack, as the functionality in itself isn't at all hard for us to wrap up. That is, I'd like to ask what the CLI should look like? I present some options:

  • pyodide unvendor mywheel.whl – this is too general and doesn't reflect what "unvendor" means (whether that's meant to unvendor tests, or unused private/public modules, etc.)
  • pyodide unvendor-tests mywheel.whl – does the job well, but it isn't pretty to type out
  • pyodide unvendor tests mywheel.whl – this makes it clear that we are unvendoring tests, while leaving scope to include other things than tests
  • pyodide minify mywheel.whl – currently minify works on Python files, and not archives, so we would need to support them
  • add pyodide wheel unvendor-tests mywheel.whl and then add pyodide wheel compress mywheel.whl (by adding https://github.com/pyodide/pyodide-pack/blob/36da5e7ece0415d1dd0508f68d784599b9497ace/compress_wheel.py)

I think point 3 sounds best to me, considering the issue about generalising the unvendoring, but I am open to more ideas.

P.S. Thanks for linking the issue about general unvendoring!

@agriyakhetarpal agriyakhetarpal mentioned this issue Mar 14, 2025
6 tasks
@ryanking13
Copy link
Member

Yeah, I also like the third option. Thanks for listing up the possible interfaces.

ryanking13 added a commit to pyodide/pyodide-build that referenced this issue Mar 22, 2025
This PR changes when test codes are unvendored.

Before:

- it is unvendored right after the wheel file is built (== at the same
step when `build.post` script runs).
- when generating a lockfile, we check if the test codes are unvendored
and update the lockfile entry.

After:

- it is unvendored when we are creating a lockfile.

__Why?__

I would like to move the unvendoring logic to somewhere else like
`pyodide-pack` or `pyodide-lock`
(pyodide/pyodide-lock#30,
pyodide/pyodide-pack#41), so package
maintainers can use that feature when they are not using a recipe.



---

Related issues:

- pyodide/pyodide-lock#30
- pyodide/pyodide-pack#41
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

No branches or pull requests

2 participants