Skip to content

Handle trailing % in strftime#23045

Merged
sbc100 merged 2 commits intoemscripten-core:mainfrom
hoodmane:strftime-trailing-percent
Dec 4, 2024
Merged

Handle trailing % in strftime#23045
sbc100 merged 2 commits intoemscripten-core:mainfrom
hoodmane:strftime-trailing-percent

Conversation

@hoodmane
Copy link
Collaborator

@hoodmane hoodmane commented Dec 2, 2024

"In glibc a trailing % in strftime() acts like printf, ie it's a literal %". This is breaking some Python tests. I've applied the patch suggested here: https://www.openwall.com/lists/musl/2022/12/19/2

"In glibc a trailing % in strftime() acts like printf, ie it's a literal %".
This is breaking some Python tests. I've applied the patch suggested here:
https://www.openwall.com/lists/musl/2022/12/19/2
@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 2, 2024

I guess this would have to be addressed upstream with musl though.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

lgtm % comment

@sbc100
Copy link
Collaborator

sbc100 commented Dec 3, 2024

Perhaps the python tests themselves should be updated? Otherwise they cannot run on any musl-based platform (e.g. alpine linux).

@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 3, 2024

Generally I try to fix both sides =)
python/cpython#127528

@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 3, 2024

I think it works on other musl systems as long as they support wide characters. The problem affects systems that use musl or bsd libc and where HAVE_WCSFTIME is false.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 3, 2024

Ah.. In that case we can/should also be including wcsftime.. i don't know why we don't already include that.

@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 3, 2024

I looked into turning that on too but I couldn't figure out how it was determined. But I think this change is an improvement in either case.

sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 3, 2024
@sbc100
Copy link
Collaborator

sbc100 commented Dec 3, 2024

This change lgtm with comments addressed.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 3, 2024

wcsftime being added in #23061

sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 3, 2024
sbc100 added a commit that referenced this pull request Dec 3, 2024
@hoodmane
Copy link
Collaborator Author

hoodmane commented Dec 4, 2024

Okay I addressed the comments I think.

@sbc100 sbc100 merged commit f526386 into emscripten-core:main Dec 4, 2024
@hoodmane hoodmane deleted the strftime-trailing-percent branch December 5, 2024 09:04
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
hedwigz pushed a commit to hedwigz/emscripten that referenced this pull request Dec 18, 2024
"In glibc a trailing % in strftime() acts like printf, ie it's a literal
%". This is breaking some Python tests. I've applied the patch suggested
here: https://www.openwall.com/lists/musl/2022/12/19/2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants