Skip to content

bpo-35344: platform.platform() uses mac_ver() on macOS#10780

Merged
vstinner merged 1 commit intopython:masterfrom
vstinner:platform_macos
Dec 5, 2018
Merged

bpo-35344: platform.platform() uses mac_ver() on macOS#10780
vstinner merged 1 commit intopython:masterfrom
vstinner:platform_macos

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Nov 28, 2018 •
edited by bedevere-bot
Loading

On macOS, platform.platform() now uses mac_ver(), if it returns a
non-empty release string, to get the macOS version rather than darwin
version.

https://bugs.python.org/issue35344

@vstinner
bpo-35344: platform.platform() uses mac_ver() on macOS
f074c81 
On macOS, platform.platform() now uses mac_ver(), if it returns a
non-empty release string, to get the macOS version rather than darwin
version.
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Nov 30, 2018

I rebased my PR and squashed all commits into a single commit. The test should now pass on Windows, Linux and macOS ;-)

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Nov 30, 2018

@malemburg, @ned-deily, @ronaldoussoren: So, what do you think of:

$ python3.8 -m platform
macOS-10.13.6-x86_64-i386-64bit

instead of:

$ python3.8 -m platform
Darwin-17.7.0-x86_64-i386-64bit

?

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Dec 3, 2018

Is there any reason to not change platform.platform() in Python 3.8 on macOS?

ronaldoussoren
ronaldoussoren reviewed Dec 5, 2018
View reviewed changes
Comment thread Lib/platform.py
# macOS (darwin kernel)
macos_release = mac_ver()[0]
if macos_release:
# note: 'macOS' is different than 'MacOS' used below
Copy link
Copy Markdown
Contributor

@ronaldoussoren ronaldoussoren Dec 5, 2018

Choose a reason for hiding this comment

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

This is something for another pull request, but the 'MacOS' bits you mention here are a leftover from the long removed MacOS 9 port and should be removed.

@ronaldoussoren
Copy link
Copy Markdown
Contributor

ronaldoussoren commented Dec 5, 2018

@malemburg, @ned-deily, @ronaldoussoren: So, what do you think of:

$ python3.8 -m platform
macOS-10.13.6-x86_64-i386-64bit

instead of:

$ python3.8 -m platform
Darwin-17.7.0-x86_64-i386-64bit

?

I like this, information about macOS is more interesting than information about the kernel version.

I'm not very happy with the processor information at the end, but that's unrelated to the change you are proposing:

  1. I don't understand why both 'uname -m' and 'uname -p' are present in this string. Does that ever add useful information?

  2. The information does not include data about fat binaries, resulting amongst others in the following inconsistency:

ronald@Menegroth[0]$ arch -i386 python3.6 -m platform
Darwin-18.2.0-x86_64-i386-64bit

ronald@Menegroth[0]$ arch -i386 python3.6 -c 'import sys; print(sys.maxsize)'
2147483647

This platform output includes "64bit" because the binary for python3.6 includes support for both i386 and x86_64, and doesn't show that the command is using i386 instructions.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Dec 5, 2018

This platform output includes "64bit" because the binary for python3.6 includes support for both i386 and x86_64, and doesn't show that the command is using i386 instructions.

To be honest, I don't understand the purpose of the platform.architecture() function :-) For me, sys.maxsize > 2**31 should be enough to distinguish 32 and 64 bits platforms, no?

@vstinner vstinner merged commit ea0ca21 into python:master Dec 5, 2018
@vstinner vstinner deleted the platform_macos branch December 5, 2018 21:41
@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Dec 5, 2018

Ronald:

This is something for another pull request, but the 'MacOS' bits you mention here are a leftover from the long removed MacOS 9 port and should be removed.

I wrote PR #10959 for that.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Dec 5, 2018

I'm not very happy with the processor information at the end, but that's unrelated to the change you are proposing: (...)

@ronaldoussoren: Please open a new issue if you would like to change that. I have no idea why we have the current info nor how to change them.

@vstinner vstinner mentioned this pull request Dec 6, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ronaldoussoren ronaldoussoren ronaldoussoren left review comments

@malemburg malemburg Awaiting requested review from malemburg

@ned-deily ned-deily Awaiting requested review from ned-deily

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vstinner @ronaldoussoren @the-knights-who-say-ni @bedevere-bot