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

chore: do not systematically build python spk when building python dependent modules #6197

Conversation

smaarn
Copy link
Contributor

@smaarn smaarn commented Aug 18, 2024

Description

Fixes issue where, when trying to update a python depending module (let's say "Bazaar"), Python's spk would systematically be built.

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

  • Includes small framework changes

@smaarn smaarn marked this pull request as ready for review August 18, 2024 18:02
@smaarn smaarn requested a review from th0ma7 August 18, 2024 18:03
@hgy59
Copy link
Contributor

hgy59 commented Aug 18, 2024

@smaarn To build python first is by intention.

All packages that use include ../../mk/spksrc.python.mk are packages that reuse a prebuilt python package.
So the python spk must be built first before building such packages.

@smaarn
Copy link
Contributor Author

smaarn commented Aug 18, 2024

@smaarn To build python first is by intention.

All packages that use include ../../mk/spksrc.python.mk are packages that reuse a prebuilt python package.

So the python spk must be built first before building such packages.

My point here was to avoid building those spks if not needed. Is what you are saying meaning that if I do that no native python install will be made causing python related processes to fail ?

@hgy59
Copy link
Contributor

hgy59 commented Aug 18, 2024

My point here was to avoid building those spks if not needed. Is what you are saying meaning that if I do that no native python install will be made causing python related processes to fail ?

No it works, when no python spk is built in advance. If no prebuilt python spk is found, it will build python as cross/python[310|311] dependency.
And indeed this build will be a little bit faster, since python is built without optimization.
BUT, as soon as more than one python dependent spk is built, the prebuild of the python spk reduces the build time hugely.

The most important benefit with prebuilt python is for local development.
If you build python spk before e.g. bazaar, the build of bazaar will reuse the prebuilt python and spk/python is kept when you call make clean in spk/bazarr (not related to this PR, since you touched the github build action only).

@smaarn smaarn marked this pull request as draft August 18, 2024 19:49
@smaarn smaarn changed the title Chore/do not systematically build python spk when building python dependent modules chore: do not systematically build python spk when building python dependent modules Aug 18, 2024
@smaarn
Copy link
Contributor Author

smaarn commented Aug 18, 2024

My point here was to avoid building those spks if not needed. Is what you are saying meaning that if I do that no native python install will be made causing python related processes to fail ?

No it works, when no python spk is built in advance. If no prebuilt python spk is found, it will build python as cross/python[310|311] dependency. And indeed this build will be a little bit faster, since python is built without optimization. BUT, as soon as more than one python dependent spk is built, the prebuild of the python spk reduces the build time hugely.

The most important benefit with prebuilt python is for local development. If you build python spk before e.g. bazaar, the build of bazaar will reuse the prebuilt python and spk/python is kept when you call make clean in spk/bazarr (not related to this PR, since you touched the github build action only).

Ok I will be rechecking if I see other solutions.
The issue here is that while trying to update bazaar I'm currently facing an issue while trying to build spk of Python 3.11 which fails because of a setuptools unsolicited upgrade being taken into account when building cryptography wheel (see https://github.com/SynoCommunity/spksrc/actions/runs/9945473643/job/28274042817 for example).

@th0ma7
Copy link
Contributor

th0ma7 commented Aug 18, 2024

@smaarn that is part of a much bigger issue that I am trying to solve (yes, old PR, will resurect in the coming days) - we can no longer have a default and unique "fit-all" crossenv anymore... In some circumstance you need the ability to upgrade/downgrade/add packages to your crossenv as otherwise your wheel building simply won't work.

That is what I am trying to solve in PR #6040 whereas I'm including an option to generate custom crossenv so specific wheels can be processed sucessfully.

@smaarn
Copy link
Contributor Author

smaarn commented Sep 15, 2024

@smaarn that is part of a much bigger issue that I am trying to solve (yes, old PR, will resurect in the coming days) - we can no longer have a default and unique "fit-all" crossenv anymore... In some circumstance you need the ability to upgrade/downgrade/add packages to your crossenv as otherwise your wheel building simply won't work.

That is what I am trying to solve in PR #6040 whereas I'm including an option to generate custom crossenv so specific wheels can be processed sucessfully.

@th0ma7 any news on this matter ? I perfectly understand that you have other priorities and may lack bandwidth but my concern with this PR was to ensure that someone working "only on a given python-dependent spk" wouldn't be impacted by Python-wide concerns.

In this particular case I'm "stuck" for updating bazarr (see #6164 ) because of some documented regression introduced in setuptools which is being aggressively upgraded when building python spk.

Would this issue also be tackled by the "separated cross env" idea ?

@th0ma7
Copy link
Contributor

th0ma7 commented Sep 15, 2024

Indeed it would as you can then specify pip and setuptools versions matching your requirements specificities.

I was away for work but now catching up and about to close ffmpeg videodriver pr. Once completed i should be able to resume work on python which is already advanced. But yeah it does require a fair share of cycles and that's gonna take me some time to complete as my availability is limited.

If all goes well as all the prerequisites pr have now been merged, a few more weeks to a month and i should have something ready for you to play with?

@smaarn smaarn closed this Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants