-
Notifications
You must be signed in to change notification settings - Fork 11
[strawman] Add UvPipCompile #40
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
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for the PoC @bollwyvl. Overall, I am +1 for the direction to provide an easy way to create a lockflie with custom dependencies, and it is great that we can leverage In the long term, I would prefer to create a lockfile from scratch instead of passing We are also considering adopting |
|
In terms (ha!) of schedule: sure, it certainly has taken a long time for all of the upstream features and specs to line up for:
But great as the enemy of the good and all: I'm very motivated to do whatever it takes to get this landed here.
Yep, that and many other great PEPs (e.g. PEP-794) are coming soon to an ecosystem near you... in the long term. Perhaps pessimistically, I'm assuming "use anything from PyPI" will need to be tweaked for a while yet to work properly in the harsh climate of the browser. Further, I don't know if PyPI really wants to be signing up for it to be easier for potentially hundreds of megabytes of downloads per-page-view. Developers are already pretty cavalier about caching in e.g. CI; browser users might not even know they are using Python. As a side effect of having to backfill PEP-764 by inspection, the proposed API encourages self-hosted wheels. In previous attempts at these features, I actually wired up a local caching proxy to intercept all wheel and API calls to avoid the second hit to download the file, as
Right: TOML not being part of the web "standard library", I'm confident a JS-based parser can be found, workarounds for bespoke, not-wheel files engineered, etc... in the long term.
Assuming a hard switch that breaks downstream workflows is being considered, the proposed API could be adapted to make that easier. Indeed, there's probably value in this repository providing a path (ha!) from an existing |
|
Seems like pypi is interested in serving our packages. The main blocker on pep 783 is the Donald Stufft problem -- he's supposed to decide on whether to approve the pep or not but he does not read it. I have a meeting with the steering council in a few weeks and hopefully they'll come up with an alternative for me. |
Great! But this doesn't change the fact that there are currently zero wheels on PyPI or any Pyodide distribution that would be marked properly. The use case of:
... is still an enormous pain for downstream users, and therefore downstream maintainers, such as myself. Perhaps #10, or a brand-new issue, would be a better place to discuss future PEPs and future breaking changes. In the meantime, what would take for the capability proposed here to land in |
|
Sure, even after the PEP 783 is landed, it will take a lot of time for the package maintainers to adopt it and publish wheels. So having a feature to generate a lockfile from local wheels, wheels hosted from other places would still be useful. Let me take a look at this PR soon and see what kind of changes we want before landing, and sorry for the delay. I've been busy these days and didn't have time to think about the lockfile. |
| return | ||
| yield self.fetch_wheel(wheel_dir, archive["url"]) | ||
| elif wheels: | ||
| wheel = wheels[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add a comment when there can be multiple wheels and why we are picking the first one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f4b2355 added an exception and comment; I don't know how it would be possible, given --python-platform and --python-version to get multiple wheels, but the spec is the spec, and wheels is a list.
| work: Path, | ||
| wheel_dir: Path, | ||
| lock_spec: PyodideLockSpec, | ||
| ) -> Iterator[tuple[Path, str | None]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this function returning an interator, regarding that there is no iteration at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The iterator allows for having a single return type, with {0,1} findings: if the source is already in the destination place, nothing needs to be copied, but should not fail.
| continue | ||
| if Path(self.input_path.parent, file_name).is_file(): | ||
| continue | ||
| url = f"{self.base_url_for_missing.removesuffix('/')}/{file_name}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about preprocessing the base_url_for_missing at the constructor, so that we don't need to remove the suffix here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess my goal is to change the input lock as little as possible, and isolate all rewriting in the post-processing step.
README.md
Outdated
| lock_spec.to_json(lock_path) | ||
| ``` | ||
|
|
||
| ### Adding pre-built wheels and dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to hide the Python API and only expose it through the CLI (pyodide lock) so that we can safely change the API more easily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will just take this out at present, and focus on the code. The state of the existing API/CLI docs is probably out of scope.
As a downstream, I want a documented Python API, and don't want a click/typer dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in f4b2355
References
Changes
pyodide-lock[uv]UvPipCompileand helper classesFuture work