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

Publish wheel for macos-arm64, manylinux-x86_64 and manylinux-arm64 #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

getzze
Copy link

@getzze getzze commented Jul 22, 2024

fixes #127, #128

Taking inspiration from:

https://github.com/shakfu/cyfaust/blob/main/docs/devnotes/universal2.md

https://stackoverflow.com/questions/76450587/python-wheel-that-includes-shared-library-is-built-as-pure-python-platform-indep

I checked manylinux-x86_64 was correct with auditwheel, maybe checking with auditwheel could be added to the tox config or to the appveyor config.

@sbraz
Copy link
Owner

sbraz commented Jul 23, 2024

Thanks a lot for doing this. I'll look into it as soon as I have a bit of free time as I know little about wheels so I want to properly understand what this does.

appveyor.yml Outdated Show resolved Hide resolved
@getzze
Copy link
Author

getzze commented Jul 23, 2024 via email

@getzze
Copy link
Author

getzze commented Sep 6, 2024

Hello, did you have time to test this PR? Thanks!

@sbraz
Copy link
Owner

sbraz commented Sep 6, 2024

Hi @getzze I'm sorry I haven't taken time to review this. I will try to do it soon.

@sbraz
Copy link
Owner

sbraz commented Oct 19, 2024

Hi, and once again sorry for the really long delay in my review. I've started reviewing the PR and I think I understand most of it. I left a few comments on the code.
I also noticed that there are a lot of warnings (not added by your changes, they're just because of the way the build is done).
If you have experience with other build backends like Hatchling, please feel free to completely rewrite the sdist/wheel code to avoid this:

/usr/lib/python3.12/site-packages/setuptools/__init__.py:94: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
!!

        ********************************************************************************
        Requirements should be satisfied by a PEP 517 installer.
        If you are using pip, you can try `pip install --use-pep517`.
        ********************************************************************************

!!
  dist.fetch_build_eggs(dist.setup_requires)

I know sooner or later I'll have to fix the build so maybe now is the right time?

@sbraz
Copy link
Owner

sbraz commented Oct 19, 2024

After reading the warnings, I stumbled on https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary. It looks like installing the build package and running python -m build gets rid of these warnings.

@getzze
Copy link
Author

getzze commented Oct 19, 2024

I think it makes sense to switch to Hatch, I find it easier to use than setuptools.

I can open another PR with the change to hatch 🙂. But it's better to keep it separate from this PR, right?

@sbraz
Copy link
Owner

sbraz commented Oct 19, 2024

Or we can just drop this PR entirely, whatever is easiest for you. If it's too much work, we can merge this first, I'd just like to understand what the try/except block does.

@getzze
Copy link
Author

getzze commented Oct 19, 2024

Ah I had forgotten about the modifications in setup.py, I only remembered the appveyor part.
It makes more sense to make a new PR with everything then!

@Tohrusky
Copy link

use pdm as build backend https://github.com/TensoRaws/pymediainfo-tensoraws

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.

Add support for macOS ARM
3 participants