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

gh-90656: Add platform triplets for 64-bit LoongArch (LA64) #30939

Merged
merged 5 commits into from May 9, 2023

Conversation

loongson-zn
Copy link
Contributor

@loongson-zn loongson-zn commented Jan 27, 2022

Add new triplets for loongarch64, please review, thanks!
https://bugs.python.org/issue46498

https://bugs.python.org/issue46498

Signed-off-by: Zhang Na zhangna@loongson.cn
Reviewed-by: Wang Xuerui git@xen0n.name
Reviewed-by: Wu Xiaotian wuxiaotian@loongson.cn
Reviewed-by: Zhu Yaliang zhuyaliang@loongson.cn

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@loongson-zn

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@bedevere-bot
Copy link

Every change to Python requires a NEWS entry.

Please, add it using the blurb_it Web app or the blurb command-line tool.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@loongson-zn
Copy link
Contributor Author

Okay, I'm working on it now.

Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

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

Please rebase on top of main branch so that no merge commits remain in this PR.

configure.ac Outdated Show resolved Hide resolved
@loongson-zn loongson-zn changed the title bpo-46498: Add new triplets for loongarch64 bpo-46498: Add platform triplets for LoongArch64 Mar 23, 2023
@erlend-aasland erlend-aasland changed the title bpo-46498: Add platform triplets for LoongArch64 gh-90656: Add platform triplets for LoongArch64 Mar 23, 2023
@loongson-zn
Copy link
Contributor Author

LoongArch toolchain conventions -- official documentation instructions, please review, thanks! @corona10 @erlend-aasland

@corona10 corona10 self-assigned this Mar 28, 2023
@corona10
Copy link
Member

corona10 commented Mar 28, 2023

@loongson-zn I will take a look, but it may takes some times to review from the view of supporting this platform is proper.
Please keep patient

@loongson-zn
Copy link
Contributor Author

@loongson-zn I will take a look, but it may takes some times to review from the view of supporting this platform is proper. Please keep patient

Thanks,this is debian documentation about LoongArch, we need cpython support LoongArch.

@loongson-zn
Copy link
Contributor Author

llvm 16.0.0 kernel 5.19gcc 13.1binutils 2.40glibc 2.36 and many other projects already support LoongArch.
On the other hand, I find python in the Gentoo , . /python3.11-config --extension-suffix output is .cpython-311.so, but to maintain consistency with other architectures, the output should be .cpython-311-loongarch64-linux-gnu.so. So, I think this commit should be merged as soon as possible to avoid more new questions continue to arise.

@arhadthedev arhadthedev added the build The build process and cross-build label May 6, 2023
@loongson-zn
Copy link
Contributor Author

rebase update

Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

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

The commit message tags are malformed. Email addresses must be attached:

Co-authored-by: WANG Xuerui <git@xen0n.name>
Signed-off-by: Zhang Na <zhangna@loongson.cn>

Also I don't know if CPython uses DCO tags (i.e. Signed-off-by) by convention. If it's not the case you may safely drop that line. I can confirm the other part (the bulk of #ifdef-ery) was suggested and written by me though.

Signed-off-by: Zhang Na <zhangna@loongson.cn>
Co-authored-by: WANG Xuerui <git@xen0n.name>
@loongson-zn
Copy link
Contributor Author

Yeah, you are right. I will make corrections.

@erlend-aasland
Copy link
Contributor

rebase update

BTW, for the CPython repo, please don't force push; use git merge --no-ff main instead :) Force-pushing does not play well with the GitHub UI. Also, we squash merge all PRs into a single commit, so it does not matter if the PR commit history is cluttered. See also the devguide.

@xen0n
Copy link
Contributor

xen0n commented May 8, 2023

Aside from the Loongson teams, there is an active community around LoongArch (also Telegram user group) with multiple porters and power users willing to help. I'm heavily involved in and contributed to many upstream projects' LoongArch support, if it helps.

Yeah, I understand how you try to support the LoongArch for various projects. But maintaining from the CPython is a different story, so what I want to confirm about accepting the patch is that all you have to do is adding a triplet, or do you need more patches in the future that include modifying CPython code with preprocessor?

Thanks for clarifying. AFAICT, so far this is the only patch necessary for future binary compatibility between multiarch-aware and -unaware distributions (i.e. Debian & derivatives vs. the others). CPython itself builds and tests fine without this change; only the native extensions have different filename suffixes which is what this patch is trying to reconcile. Hence I don't think more #ifdef-ery should be necessary in the future.

Some more backgrounds though: LoongArch might need more treatment in the future if we'd want to provide perfect UX. There's two incompatible ABIs of LoongArch userland, so native libraries of different ABIs can't be interlinked without invoking UB, but Loongson wants the two ABIs (really two worlds) to share the same multiarch tuple. I expect some friction in the future if some user on one world uploads wheels to PyPI, only to be downloaded by someone on the other world, then failing at import time. It can be said to be a purely downstream problem though; most other upstream projects simply don't consider the "old world" / "ABI v1.0" to exist at all. And even if we do want to care, the changes here still apply: we've already undergone debates and this patch can be seen as the result. (This is kinda orthogonal to the issue at hand; I should probably post this elsewhere for further discussion.)

@loongson-zn
Copy link
Contributor Author

@erlend-aasland cc @loongson-zn

But on the other hand, if this is the only patch that they will need, I am okay to accept the patch anyway.

@corona10 Currently, this is the only one patch. Because it affects binary compatibility, I believe it is necessary to merge as soon as possible.

@corona10
Copy link
Member

corona10 commented May 9, 2023

@corona10 Currently, this is the only one patch. Because it affects binary compatibility, I believe it is necessary to merge as soon as possible.

Okay, let's pass the CI first, I am +1 with accepting this patch.

@loongson-zn
Copy link
Contributor Author

loongson-zn commented May 9, 2023

Okay, let's pass the CI first, I am +1 with accepting this patch.

@corona10 Thank you for your support. I am debugging errors in CI Docs. I have asked Erlend Aasland about the reason and wating for his reply. If you could tell me the reason, I would be very grateful

@loongson-zn
Copy link
Contributor Author

This error disappeared after I executed it twice on my PC.

@erlend-aasland erlend-aasland linked an issue May 9, 2023 that may be closed by this pull request
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Thanks, Zhang & WÁNG! I'll wait for Dong-hee's approval before landing this.

@erlend-aasland erlend-aasland changed the title gh-90656: Add platform triplets for LoongArch64 gh-90656: Add platform triplets for 64-bit LoongArch (LA64) May 9, 2023
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@erlend-aasland
Copy link
Contributor

@xen0n, should we add your attributions to the NEWS and What's New entry also?

@erlend-aasland erlend-aasland merged commit 03029ac into python:main May 9, 2023
25 of 26 checks passed
@xen0n
Copy link
Contributor

xen0n commented May 9, 2023

@xen0n, should we add your attributions to the NEWS and What's New entry also?

Either is OK, I'm unaffiliated with Loongson and in general just provides drive-by code reviews for everything LoongArch ;-)

In this case IMO let's see what @loongson-zn thinks though.

EDIT: Actually my involvement can be seen here -- while the change was substantive compared to the original (that was just a single __loongarch64 -> loongarch64-linux-gnu check), it's still kinda trivial given the patch's absolute size, so I'm not sure if I deserve the attribution ;-)

carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
* main:
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
* main: (29 commits)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)
  pythongh-103193: Improve `getattr_static` test coverage (python#104286)
  Trim trailing whitespace and test on CI (python#104275)
  pythongh-102500: Remove mention of bytes shorthand (python#104281)
  pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)
  pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
* main: (156 commits)
  pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)
  pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)
  pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)
  pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)
  pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)
  pythongh-104240: return code unit metadata from codegen (python#104300)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  ...
carljm added a commit to carljm/cpython that referenced this pull request May 9, 2023
* main: (35 commits)
  pythongh-97696 Add documentation for get_coro() behavior with eager tasks (python#104304)
  pythongh-97933: (PEP 709) inline list/dict/set comprehensions (python#101441)
  pythongh-99889: Fix directory traversal security flaw in uu.decode() (python#104096)
  pythongh-104184: fix building --with-pydebug --enable-pystats (python#104217)
  pythongh-104139: Add itms-services to uses_netloc urllib.parse. (python#104312)
  pythongh-104240: return code unit metadata from codegen (python#104300)
  pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)
  pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)
  pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)
  require-pr-label.yml: Add missing "permissions:" (python#104309)
  pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)
  pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)
  pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)
  pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)
  pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)
  pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)
  pythonGH-104284: Fix documentation gettext build (python#104296)
  pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)
  pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)
  pythongh-99108: fix typo in Modules/Setup (python#104293)
  ...
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 new triplets for 64-bit LoongArch
8 participants