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

python3: backport and fix target musl libc detection #19110

Merged
merged 1 commit into from
Sep 12, 2022

Conversation

autobakterie
Copy link
Contributor

Patch 030:
Backported from Python main branch1 for Python to distinguish between glibc and musl libc SOABI.

Patch 131:
Changes PLATFORM_TRIPLET -gnu/-musl suffix detection (performed by the backported patch)
to be based on the target OS instead of the building OS.

See included patches for more detailed descriptions.

Specifically this fixes cross-compilation for mpc8548 CPUs with SPE instructions2 enabled.

Co-authored-by: Pali Rohár [email protected]
Signed-off-by: Šimon Bořek [email protected]

Maintainer: @jefferyto
Compile and run tested: mpc85xx/p2020, Turris 1.x, OpenWrt 21.02 and master

Footnotes

  1. merged to python:main as https://github.com/python/cpython/pull/24502 'bpo-43112: detect musl as a separate SOABI'

  2. https://www.nxp.com/docs/en/reference-manual/SPEPEM.pdf

@autobakterie
Copy link
Contributor Author

As mentioned in python/cpython#24502 (comment) incorporating this might break loading modules unless they are rebuilt along with the update, therefore this change won't be backported to older versions of upstream cpython. I'm not sure if this is a relevant issue in the context of OpenWrt package.

Could somebody more knowledgeable clarify this?

Comment on lines 38 to 39
+ self.assertTrue(suffix.endswith(expected_suffixes),
+ f'unexpected suffix {suffix!r}')

This comment was marked as resolved.

Copy link
Contributor Author

@autobakterie autobakterie Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder. Should be corrected by the fixup. :)

@BKPepe
Copy link
Member

BKPepe commented Aug 6, 2022

cc: @commodo as well

@commodo
Copy link
Contributor

commodo commented Aug 8, 2022

this probably means that we should re-visit patch lang/python/python3/patches/014-remove-platform-so-suffix.patch ?

@commodo
Copy link
Contributor

commodo commented Aug 8, 2022

this probably means that we should re-visit patch lang/python/python3/patches/014-remove-platform-so-suffix.patch ?

(we may have to also re-visit lang/python/python3/patches/016-adjust-config-paths.patch )

so, to detail my side on this (from memories):

  • i've been meaning to remove patches 014 & 016 for a while, but it requires re-building the entire Python packages (which grew over time)
  • as-far-as-i-remember, in OpenWrt, we just disconsider PLATFORM_TRIPLET for the target build; and we don't really need it, because when we build for a target, the files get put in the target build/staging folder of that target;
  • that being said, it's also a good idea to not keep any OpenWrt specifics, if CPython could handle these elegantly; but it does require some time to re-build everything a couple of times until it successfully compiles (everything with all Python packages)
  • AFAICT: this patch could be a no-op, because patches 014 & 016 will just remove the PLATFORM_TRIPLET anyway? I could be wrong, but I would first start by removing those 2 and then trying to see if these new patches fix anything

@autobakterie
Copy link
Contributor Author

autobakterie commented Aug 10, 2022

Thank you for the comment, @commodo.

this probably means that we should re-visit patch lang/python/python3/patches/014-remove-platform-so-suffix.patch ?
(we may have to also re-visit lang/python/python3/patches/016-adjust-config-paths.patch )

I do not think that is necessary, as the change in PLATFORM_TRIPLET construction (swapping '-gnu' with '-musl' in relevant cases) does not affect the operation of patches making Python ignore PLATFORM_TRIPLET when naming files/directories (if I understand them correctly) - it will still be ignored in this context. (although even with patches 014 and 016 applied, the backport still fixes the problem described below)

i've been meaning to remove patches 014 & 016 for a while, but it requires re-building the entire Python packages (which grew over time)
as-far-as-i-remember, in OpenWrt, we just disconsider PLATFORM_TRIPLET for the target build; and we don't really need it, because when we build for a target, the files get put in the target build/staging folder of that target;
that being said, it's also a good idea to not keep any OpenWrt specifics, if CPython could handle these elegantly; but it does require some time to re-build everything a couple of times until it successfully compiles (everything with all Python packages)

It looks like a good idea to stick to upstream Python build process behaviour where possible, although I don't think I have deep enough insight into OpenWrt's Python packages build process to assess the effects removal of patches 14 and 16 will have.

AFAICT: this patch could be a no-op, because patches 014 & 016 will just remove the PLATFORM_TRIPLET anyway? I could be wrong, but I would first start by removing those 2 and then trying to see if these new patches fix anything

I'm proposing this as a working fix for an existing issue (tested on Turris OS branches based on OpenWrt master and 21.02, Turris 1.x device, as stated in the description). I'm currently in the process of submitting patches allowing OpenWrt to be compiled for mpc85xx targets with Power ISA SPE extension enabled (enables CPU native floating point arithmetic among other things; work already tested and merged in development branches of Turris OS12). Without the backport (plus its fix3), compilation of Python for mpc8548 musl-spe fails.

Footnotes

  1. https://gitlab.nic.cz/turris/os/build/-/merge_requests/542

  2. https://gitlab.nic.cz/turris/os/build/-/merge_requests/544

  3. already suggested upstream https://github.com/python/cpython/issues/95855

@commodo
Copy link
Contributor

commodo commented Aug 16, 2022

@autobakterie

thanks for the info :)

so, from my side these patches are fine if they fix mpc85xx and don't break others (which seems to be the case)

i'll let @jefferyto have the last word on this;

@BKPepe BKPepe requested a review from jefferyto August 20, 2022 12:29
@autobakterie
Copy link
Contributor Author

It's been almost 2 weeks since the last update in this thread. Are further adjustments needed? @jefferyto, do you think this can be merged?

@BKPepe BKPepe requested a review from neheb September 9, 2022 13:10
@neheb
Copy link
Contributor

neheb commented Sep 9, 2022

Formalities need to be fixed.

Patch 030:
Backported from Python main branch[^1] for Python to distinguish between glibc and musl libc SOABI.

Patch 131:
Changes PLATFORM_TRIPLET -gnu/-musl suffix detection (performed by the backported patch)
to be based on the target OS instead of the building OS.

See included patches for more detailed descriptions.

Specifically this fixes cross-compilation for mpc8548 CPUs with SPE instructions[^2] enabled.

[^1]: merged to python:main as python/cpython#24502 'bpo-43112: detect musl as a separate SOABI'
[^2]: https://www.nxp.com/docs/en/reference-manual/SPEPEM.pdf

Co-authored-by: Pali Rohár <[email protected]>
Signed-off-by: Šimon Bořek <[email protected]>
@autobakterie autobakterie force-pushed the python-ppc-crosscompilation-fix branch from 8e4d760 to 79e3a73 Compare September 12, 2022 12:45
@autobakterie
Copy link
Contributor Author

Formalities need to be fixed.

That was a fixup commit left temporarily unsquashed to make the review process cleaner. Has been squashed now and rebased on master.

@neheb neheb merged commit 992fcd1 into openwrt:master Sep 12, 2022
@autobakterie autobakterie deleted the python-ppc-crosscompilation-fix branch January 30, 2023 17:15
@jefferyto
Copy link
Member

Apologies for being absent on this - I would like ask regarding 131-configure_ac-switch-PLATFORM_TRIPLET-suffix-to-musl.patch:

  • Is this an original patch for OpenWrt or was it originally submitted somewhere else?
  • Is this suitable to be upstreamed? If yes, has it been submitted upstream?

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.

6 participants