-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
gh-123681: Check NORMALIZE_CENTURY behavior at runtime; require C99 #136022
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
Conversation
…at the compile time It is needed to support cross-compiling. Remove macros Py_NORMALIZE_CENTURY and Py_STRFTIME_C99_SUPPORT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If remove the C99 check in the C code, we should also remove _can_support_c99()
in Lib/_pydatetime.py
.
Look in #122272 for reasons for adding this check at first place.
self.check_strftime_y2k('Y') | ||
self.check_strftime_y2k('G') | ||
|
||
def test_strftime_y2k_c99(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not it be decorated with cpython_only
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! The meaning of cpython_only
is not very clear.
This test might be usable for other implementations as well, so I think it's best for other implementations to skip the test if it doesn't work for them. IMO, cpython_only
is best for things like bytecode details or sys.getsizeof
.
But, that's just my opinion of course.
🤖 New build scheduled with the buildbot fleet by @encukou for commit a52be23 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136022%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
The buildbot failures are unrelated. |
@serhiy-storchaka, are you OK with merging this? If you think #128444 is preferable then I'll defer to you. |
I think #128444 is preferable for backporting. In 3.15 we can remove checks for the C99 features. We will have a lot of time to test this and restore workarounds if this does not work on some obscure platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On other hand, support for non-c99 strftime was removed in 3.14 a while ago, and there were no complains during beta testing. So, this LGTM if you plan to backport it to 3.14. We should fix 3.14 in one way or another.
Misc/NEWS.d/next/Build/2025-01-03-13-02-06.gh-issue-123681.gQ67nK.rst
Outdated
Show resolved
Hide resolved
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
… C99 (pythonGH-136022) A runtime check is needed to support cross-compiling. Remove the _Py_NORMALIZE_CENTURY macro. Remove _pydatetime.py's _can_support_c99. (cherry picked from commit 719e5c3) Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-137947 is a backport of this pull request to the 3.14 branch. |
|
This is a continuation of #128444.
CPython requires C11, so all C99 features should work.
We might need workaround if a supported platform misbehaves (hence this PR -- I want to feed the buildbots). But, IMO, any workarounds for unsupported platforms best left to the respective forks.
Serhiy's runtime check for
normalize_century
is left in; I don't know about the details there.test_strftime_y2k
fails on embedded Linux #123681