-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 support for converting pyproject.toml-based Python3 packages. #1982
base: main
Are you sure you want to change the base?
Conversation
After some reflections I've had a second thought.
Going to revise my solution accordingly. |
419d841
to
9e16f11
Compare
It seems to be working a little bit better now. Things to do:
Any suggestions/comments/guidelines are still welcome. |
Thanks for your work on this! I’ll take a look as soon as I can :) |
Bottom line:
|
After a while, I hack the Wheel metadata to get information if package is pure or not. |
It reached "works for me" stage. |
lib/fpm/package/python.rb
Outdated
"#{Shellwords.escape(toml_metadata_code)}" | ||
|
||
# @todo FIXME! | ||
# if attributes[:python_obey_requirements_txt?] |
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.
Doesn't this break backward compatibility?
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.
It can.
Do you know any cmd option for PEP517 builds to do the same thing as --load-requirements-txt does for setup.py?
lib/fpm/package/python.rb
Outdated
::Dir.chdir(project_dir) do | ||
flags = [ "--root", staging_path ] | ||
|
||
# if !attributes[:python_install_lib].nil? |
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.
Lot's of commented code. Is it necessary?
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.
Seems there is just no alternative for such option (--prefix) in setup.py for PEP517 builds...
a9a1434
to
43fefae
Compare
Amy update on this? A lot of packages no longer ship setup.py files so if we need to use fpm then we're stuck on old versions |
I don't understand what this does? It still requires a legacy And if you have a source python project that has the option to generate a legeacy I am a bit confused what this change is supposed to do. What is the intention of this change? |
No, it doesn't. There is an option to use |
Thanks for the clarification. I'll need to read through the patch again then. 😁 |
Although in this MR I made attempt to solve the issue via |
That's excellent. I will need to test out your PR and report the results. Specifically, I will need to test with this PEP-517 project ... https://github.com/openai/openai-python |
Just an enthusiastic vote for more eyeballs on this merge request ... I would love ❤️ to see the project catch up to this shift in the python community. |
Quick feedback from my initial testing of this patch: Testing with
My main issue that I'm looking into at the moment is the boot-strapping of the PEP 517 build environments for I think the main concern so far is the new dependency on the But a lot of wheels will now write 2.3 Metadata file formats. Therefore, injecting or vendoring an up-to-date version of |
@cwegener could you please elaborate, what is your issue with |
I think it's actually a different issue that is unrelated to the PEP-517 build support. The issue is that all dependencies are being built from source due to a change introduced here: 77e0cf7 The The dependencies will be thrown away at the end. They don't end up in the released package that fpm creates. So there is really no benefit in building them from source. Nevertheless, I still have not managed to build an |
Just checked - built seamlessly. Could you please try to build it with more verbose output and share the result? (you may want to adjust |
I had another look. And I think I figured out where the issue with this approach is. Example package:
This will try to immediately build a wheel from the This is a References: |
Could you please elaborate, do you able to offer different approach? May be you know the way to download and use pre-build binary packages, for all possible supported OS/architectures? |
@amdei This branch worked for me on ubuntu 24.04! On ubuntu 22.04, ubuntu 20.04, rockylinux 8 and rockylinux 9, I had to both upgrade the distro-provided pip as well as upgrade the above modules before the creation of a .deb from python/pip would work. So, I conclude that the core of your patch works great, but there may be some dependency auto-foo that is lacking? Thank you for submitting this MR! |
d722b81
to
c5af917
Compare
Rebased on top of main. |
@cwegener Guess I finally get what you've meant here:
I asked:
But the answer appears quite obvious: in wast majority of cases all these packages ARE on PyPi already. Will try to figure it out. |
@@ -0,0 +1,106 @@ | |||
import os | |||
import sys | |||
import pkg_resources |
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 usage of pkg_resources is DEPRECATED:
https://setuptools.pypa.io/en/latest/pkg_resources.html
Users should refrain from new usage of pkg_resources and should work to port to importlib-based solutions.
@cwegener
Can not properly check p. 3 though at the moment... Could you please try it now? |
TL;DR:
It works! (With some limitations at the moment though, as this is work in progress currently)
Limitations:
Temporary limitations (TODOs):
1. Do not support automatic dependencies extraction to the moment- supports2. Do not support license extraction to the moment- supports3. Checked only on Debian 11/12 amd64/arm64 only so far
4. Not sure about applicability to 'native' packages- supports native package detection5. A lot of @todo's in codeOnly a few of them left...6. Code quality is poormade it better7. Code-style is poortried to made it betterI had no previous experience with ruby, so any suggestions/comments/guidelines are welcome.
@ObjatieGroba - could you please check Python part of code, and give your suggestions on how to improve it?
Fixes #1780
Fixes #1873
Fixes #1860
Fixes magma/magma#13847