packages icon indicating copy to clipboard operation
packages copied to clipboard

python3: backport and fix target musl libc detection

Open autobakterie opened this issue 3 years ago • 6 comments

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 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

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

autobakterie avatar Aug 04 '22 17:08 autobakterie

As mentioned in https://github.com/python/cpython/pull/24502#issuecomment-1021290819 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?

autobakterie avatar Aug 04 '22 18:08 autobakterie

cc: @commodo as well

BKPepe avatar Aug 06 '22 18:08 BKPepe

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

commodo avatar Aug 08 '22 08:08 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 )

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

commodo avatar Aug 08 '22 08:08 commodo

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 OS^1). Without the backport (plus its fix[^3]), compilation of Python for mpc8548 musl-spe fails.

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

autobakterie avatar Aug 10 '22 17:08 autobakterie

@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;

commodo avatar Aug 16 '22 06:08 commodo

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?

autobakterie avatar Sep 02 '22 12:09 autobakterie

Formalities need to be fixed.

neheb avatar Sep 09 '22 17:09 neheb

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.

autobakterie avatar Sep 12 '22 12:09 autobakterie

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?

jefferyto avatar May 08 '23 08:05 jefferyto