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

Feat: support sdists #32

Closed
3 of 4 tasks
baszalmstra opened this issue Sep 29, 2023 · 15 comments
Closed
3 of 4 tasks

Feat: support sdists #32

baszalmstra opened this issue Sep 29, 2023 · 15 comments
Labels
enhancement New feature or request

Comments

@baszalmstra
Copy link
Contributor

baszalmstra commented Sep 29, 2023

Currently we only support wheels. It would be nice if we could retrieve metadata from sdists.

Remaining:

@wolfv
Copy link
Member

wolfv commented Sep 29, 2023

I believe the way to go is to compile the sdist to a wheel, and then extract metadata from there. We could also check how pip does it.

@notatallshaw
Copy link

notatallshaw commented Sep 29, 2023

Pip definitely has to compile sdists to extract their metadata.

I would make the recommendation that in any situation you can don't try to resolve sdists until you know for sure you can't use wheels to find a solution that matches the spec given.

This can be in terms of backtracking on different packages but also installation version of a given package. Pip has an option called "--prefer-binary" which has been discussed for a long time to make default but will cause backwards compatibility issues for them.

Compiling sdists is where a lot of the trouble around Python packaging comes from, as they can run arbitrary code and be heavily dependent on a users environment.

Pip also immediately exits resolving requirements if an sdists compilation fails, this is on the assumption that the user does not have the prerequisites in their environment to compile.

@notatallshaw
Copy link

P.s. I think it's within spec to serve PEP 658 metadata files for sdists but I don't believe anyone is doing it yet or has a proposed solution for how that would work. But something to be aware of.

@pradyunsg
Copy link

I think it's within spec to serve PEP 658 metadata files for sdists but I don't believe anyone is doing it yet or has a proposed solution for how that would work.

The way that would work is:

  • Core Metadata 2.2 gets support on build-backends and PyPI (https://peps.python.org/pep-0643/)
  • PyPI starts extracting that metadata and publishing those via the metadata mechanism from PEP 658 (and PEP 714).

@pradyunsg
Copy link

We could also check how pip does it.

There's an easier way for y'all, theoretically: Use python -m build or implement PEP 517 + PEP 518 on your end in Rust (I don't think there's prior art for this?).

Either of those still needs to use multiple subprocesses though, so that's going to slow down the overall resolve (it's a partial or complete build of the package). There's also the prepare_metadata hooks which allow short-curcuit builds, if the backend implements that and allows the resolver to get the dependency metadata eagerly.

@charliermarsh
Copy link

FWIW, Posy has some support for building source distributions in Rust without a dependency on pypa/build or another frontend. (See the hooks around PEP 517, along with its minimal build frontend.)

@tdejager
Copy link
Contributor

tdejager commented Oct 30, 2023

Yes, so I will want to give a stab at implementing this in the coming week. What I see from posy is that it indeed uses it's own minimal build front-end and the following things need to be implemented.

Let's say we are mainly interested in getting the metadata from the sdist for now.

  • Need to save the actual sdists and not skip them as we are doing now.
  • Extract the sdists, should be pretty easy.
  • We need to have a python to be able to actually use the build frontend. Also determine what python and version we need.
  • We need to create an environment with possible dependencies that can potentially build the wheel.
  • We need to actually call the script.
  • Cache artifacts and such which posy also does .

This would also depend on wheel installs to create the environment #6

@tdejager
Copy link
Contributor

Just an update @baszalmstra @wolfv @ruben-arts and others.

So I've been working on this some. Currently, have PKG-INFO loading #71 and reading the correct pyproject.toml data #72 to start building sdists.

Now getting to the fun part, currently working on setting up the venv for building in a branch on my forked repo feat/venv-creation. Will work on actually using the posy frontend after, and a content adressable wheel cache.

Also found out, that although PEP643 is widely implemented, the requirements do not always match-up. E.g apache airflow sdist has no depencies, while the wheel does, I think this is erroneous.

@pradyunsg
Copy link

You're missing an important caveat in PEP 643: it bumped the metadata version to 2.2 and your implementation is not filtering out projects that don't have that metadata version.

@tdejager
Copy link
Contributor

@pradyunsg thanks will change that!

@tdejager
Copy link
Contributor

tdejager commented Nov 14, 2023

@pradyunsg I created a PR that adresses the issue you noticed. #76

A question, in a lot of my testing I don't really see version 2.2 being used a lot. Do you have any idea how widespread the support is.

I suppose, even though some projects like Flask seem to list the correct requirements even though still at an older Metadata-Version, we cannot rely on this as we cannot be sure this is correct. This potentially blocks out quite usable PKG-INFO's but I guees there is no other way.

Also poetry/masonry for example, from a cursory glance at the source code, do seem to support the use of Dynamic, but is still at Metadata-Version: 2.1, PEP643 in Poetry.
Do you have any more thoughts on this?

@tdejager
Copy link
Contributor

Will continue on the sdist building and leaving the PEP643 handling as is, as we would need this for resolution anyways.

@tdejager
Copy link
Contributor

To keep better track I've split this into three issues

When we want to integrate this into pixi we would also need functions that expose the installation of an sdist.

@pradyunsg
Copy link

A question, in a lot of my testing I don't really see version 2.2 being used a lot. Do you have any idea how widespread the support is.

There shouldn't be any. pypi/warehouse#9660 -- PyPI does not support it yet, and hence backends shouldn't be producing that.

I expect we'd have stricter validation of metadata on PyPI in a bit as well, with https://packaging.pypa.io/en/stable/metadata.html#packaging.metadata.Metadata implemented and available for use within Warehouse and pip.

baszalmstra pushed a commit that referenced this issue Nov 20, 2023
This is the first iteration in rip to add support for #78.

Small test was added to see if the wheel install works, this will mainly
be used for #32 but we can also integrate this into the `rip` binary to
be able to add the install path there as well. @baszalmstra let me know
if you want me to add this in a separate PR once this is merged.
@tdejager
Copy link
Contributor

tdejager commented Dec 5, 2023

Initial support is integrated as of now into main 👍

@tdejager tdejager closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants