bpo-35389: platform.libc_ver() uses os.confstr() #10891
Conversation
Lib/platform.py
Outdated
There was a problem hiding this comment.
On some systems (IIRC using musl libc) os.confstr('CS_GNU_LIBC_VERSION') fails with OSError, so that should be caught too.
There was a problem hiding this comment.
Oh, I didn't know. It's now fixed.
There was a problem hiding this comment.
Maybe I should add that it's only used if the executable parameter is not set.
|
Travis CI failed because of https://bugs.python.org/issue35411 :-( I just fixed this issue, this PR should be rebase (or merged with master). |
platform.libc_ver() now uses os.confstr('CS_GNU_LIBC_VERSION') if
available and the *executable* parameter is not set. The default
value of the libc_ver() *executable* parameter becomes None.
Quick benchmark on Fedora 29:
python3 -m perf command ./python -S -c 'import platform; platform.libc_ver()'
94.9 ms +- 4.3 ms -> 33.2 ms +- 1.4 ms: 2.86x faster (-65%)
|
I squashed my commits and rebased my PR. |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
The test overrides the existing test test_libc_ver.
|
When you're done making the requested changes, leave the comment: And if you don't make the requested changes, you will be put in the comfy chair! |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
Oops, I missed these tests! It's now fixed. |
platform.libc_ver() now uses os.confstr('CS_GNU_LIBC_VERSION') if
available.
Quick benchmark on Fedora 29:
python3 -m perf command ./python -S -c 'import platform; platform.libc_ver()'
94.9 ms +- 4.3 ms -> 33.2 ms +- 1.4 ms: 2.86x faster (-65%)
https://bugs.python.org/issue35389