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

[WIP] Review content of crossenv to respect build vs cross. #6437

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

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Feb 2, 2025

Description

Review content of crossenv to respect build vs cross.

  • spksrc.requirement.mk: Unified method to process requirements files

Closes #6430

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@th0ma7 th0ma7 changed the title Review content of crossenv to respect build vs cross. [WIP] Review content of crossenv to respect build vs cross. Feb 2, 2025
@th0ma7 th0ma7 mentioned this pull request Jan 30, 2025
5 tasks
@hgy59
Copy link
Contributor

hgy59 commented Feb 2, 2025

@th0ma7 I was hoping this would fix numpy for python312, but it doen't (still getting ../meson.build:37:2: ERROR: Problem encountered: NumPy requires Cython >= 0.29.34)

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 2, 2025

@hgy59 thnx for testing but it's not ready yet, I've only laid-down the foundation to have a reusable requirement mk file. I still have to:

  1. Update the crossenv mk file to use the new requirement mk file
  2. convert crossenv definitions to use cross|build
  3. Migrate environment PATH to use build only (and stop relying on native/python3* maturin)

Only once that is complete I should be ready to start looking further into numpy in particular.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 5, 2025

@hgy59 still a bit of testing todo but I should be iches away from being ready to look in particular at the numpy issue.

I'll let things run but changes done so far in this PR to the PATH and most importantly the crossenv cross vs build contents should now address many of the remaining issues found 🤞

@mreid-tt
Copy link
Contributor

mreid-tt commented Feb 5, 2025

hey @th0ma7, looking forward to this PR. Looking at the build errors it seems that based on this python-greenlet/greenlet#392 (comment), they are caused by:

That's because you're trying to install greenlet < 3.1, which doesn't support Python 3.13. You must use greenlet >= 3.1.x.

As such, I believe if you comment out this line your build should succeed:

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 7, 2025

I believe it is now complete and ready for testing. Only remaining item (unless github-action tells me otherwise): numpy.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 7, 2025

Suggested plan of actions:

  1. Merge this pr
  2. Update pythons (updates got out this week)
  3. Fix numpy

Thoughts?

@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

@th0ma7 I have updated the comment in python-wheels packages related to the include of spksrc.python.mk.

Please feel free to revert it, if it is not correct...

@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

@th0ma7 one thing I am missing

when building single wheels with make WHEEL="{name==version}" wheel-{arch}-{tcversion} it would be handy to have the build logs collected in a file.

I guess we only need to add something like | tee --append build-wheel-$*.log in the make wheel command of the wheel-% target in spksrc.wheel.mk.

@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

2. Update pythons (updates got out this week)

Python 3.13.2 is already released.

EDIT:
and Python 3.12.9 too

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 7, 2025

when building single wheels with make WHEEL="{name==version}" wheel-{arch}-{tcversion} it would be handy to have the build logs collected in a file.

I guess we only need to add something like | tee --append build-wheel-$*.log in the make wheel command of the wheel-% target in spksrc.wheel.mk.

Yeah, I thought of that too. By default it gest collected when invoking a full standard build but not for neither crossenv-* or wheel-*. I can look into that. I wonder, would it make sense to have a per-wheel log in a sub-directory? or having a single wheel-arch-version.log and similarly a crossenv-arch-version.log created? maybe even only when invoking them directly (not sure i can do that)?

That I'd add to this PR.

@mreid-tt
Copy link
Contributor

mreid-tt commented Feb 7, 2025

I believe it is now complete and ready for testing. Only remaining item (unless github-action tells me otherwise): numpy.

Great work, @th0ma7! You might consider re-enabling llfuse based on the fix I mentioned here. I'm not sure if there are any other commented-out items (besides numpy) that could be re-enabled, but it’s worth a check.

@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

would it make sense to have a per-wheel log in a sub-directory? or having a single wheel-arch-version.log

For me, a single build-wheel-arch-version.log was enough.
if it starts with build-* it is already handled in the clean* targets
no need for sub-dir (we also have other build*.log at the same level as the work-* folders)

no need for wheel-name or -version in the log file name
If I want to keep log files for detailed analysis, I always rename it before running make again,

hgy59 added 3 commits February 7, 2025 15:08
- add llfuse to python312-wheels and python313-wheels
- use -std=gnu11 for gcc < 5
@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

@th0ma7 hopefully you don't mind my latests pushes.

Just updated python312 and python313 and added llfuse wheel with fix provided by @mreid-tt

EDIT:
locally tested for x64-6.2.4 and x64-7.1

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 7, 2025

Bah, i would have rather having the separated due to the already lengthy pr, but fine by me.

- llfuse wheel fails to build with python313
@hgy59
Copy link
Contributor

hgy59 commented Feb 7, 2025

I believe it is now complete and ready for testing. Only remaining item (unless github-action tells me otherwise): numpy.

Great work, @th0ma7! You might consider re-enabling llfuse based on the fix I mentioned here. I'm not sure if there are any other commented-out items (besides numpy) that could be re-enabled, but it’s worth a check.

llfuse fails to build for python313:

      src/llfuse.c: In function ‘__Pyx_PyInt_As_gid_t’:
      src/llfuse.c:60451:23: error: too few arguments to function ‘_PyLong_AsByteArray’
                       ret = _PyLong_AsByteArray((PyLongObject *)v,
                             ^~~~~~~~~~~~~~~~~~~
      In file included from /spksrc/spk/python313/work-x64-7.1/install/var/packages/python313/target/include/python3.13/longobject.h:107,
                       from /spksrc/spk/python313/work-x64-7.1/install/var/packages/python313/target/include/python3.13/Python.h:81,
                       from src/llfuse.c:16:
      /spksrc/spk/python313/work-x64-7.1/install/var/packages/python313/target/include/python3.13/cpython/longobject.h:111:17: note: declared here
       PyAPI_FUNC(int) _PyLong_AsByteArray(PyLongObject* v,
                       ^~~~~~~~~~~~~~~~~~~

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 8, 2025

@hgy59 @mreid-tt heads-up, far from out of the woods but I was able to cross-compile numpy 2.2.2 on x86_64, fails on armv8 but i may have a recipe in the work...

@hgy59
Copy link
Contributor

hgy59 commented Feb 8, 2025

I alredy reported an issue with octoprint and python312

On this branch I found another issue.
The weel for zeroconf 0.143.0 in a package for x64-7.1 was named zeroconf-0.143.0-cp312-cp312-manylinux2014.whl and the install log showed

2025/02/08 00:03:05     ERROR: zeroconf-0.143.0-cp312-cp312-manylinux2014.whl is not a supported wheel on this platform.

I already have seen other wheels with manylinux2014 instead of linux_x86_64 in the name.
currently I have no idea what went wrong.

@hgy59
Copy link
Contributor

hgy59 commented Feb 8, 2025

Another open question:

Do we really need the requirements-abi3.txt file`?

In python312-wheels we only have pycryptodome==3.21.0 and pycryptodomex==3.21.0 in this file.
But the package contains two other wheels with abi in the name:

  • cryptography-44.0.0-cp37-abi3-linux_x86_64.whl
  • psutil-6.1.1-cp36-abi3-linux_x86_64.whl

Both come from the regular requirements-crossenv.txt

So my guess is, that the wheel build can autodetect the abi.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Feb 8, 2025

Thnx for further testing its exactly the type of feedback i need. I have a good idea of the reason, I'll look at it in the coming days. Pretty sure the issue is with how the crossenv is being built using manylinux which now impact pure python wheels which previously were built using native

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.

3 participants