bpo-35348: Fix platform.architecture() on macOS by vstinner · Pull Request #11186 · python/cpython · GitHub
Skip to content

bpo-35348: Fix platform.architecture() on macOS #11186

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.

Sign up for GitHub

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

Closed
vstinner wants to merge 3 commits into from
Closed

bpo-35348: Fix platform.architecture() on macOS #11186

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
778dfb5
bpo-35348: Fix platform.architecture() on macOS
vstinner Dec 17, 2018
db1cc3f
Fix test_architecture()
vstinner Dec 17, 2018
1928d12
Add platform._DEFAULT_LINKAGE
vstinner Dec 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions Doc/library/platform.rst
View file
Open in desktop
38 changes: 11 additions & 27 deletions Lib/platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def libc_ver(executable=None, lib='', version='', chunksize=16384):
The file is read and scanned in chunks of chunksize bytes.

"""
if executable is None:
if executable is None or executable == sys.executable:
try:
ver = os.confstr('CS_GNU_LIBC_VERSION')
# parse 'glibc 2.28' as ('glibc', '2.28')
Expand Down Expand Up @@ -618,15 +618,7 @@ def _syscmd_file(target, default=''):

### Information about the used architecture

# Default values for architecture; non-empty strings override the
# defaults given as parameters
_default_architecture = {
'win32': ('', 'WindowsPE'),
'win16': ('', 'Windows'),
'dos': ('', 'MSDOS'),
}

def architecture(executable=sys.executable, bits='', linkage=''):
def architecture(executable=None, bits='', linkage=''):

""" Queries the given executable (defaults to the Python interpreter
binary) for various architecture information.
Expand All @@ -640,11 +632,9 @@ def architecture(executable=sys.executable, bits='', linkage=''):
(or sizeof(long) on Python version < 1.5.2) is used as
indicator for the supported pointer size.

The function relies on the system's "file" command to do the
actual work. This is available on most if not all Unix
platforms. On some non-Unix platforms where the "file" command
does not exist and the executable is set to the Python interpreter
binary defaults from _default_architecture are used.
If the executable parameter is set, the function relies on the system's
"file" command to do the actual work. This is available on most Unix
platforms.

"""
# Use the sizeof(pointer) as default number of bits if nothing
Expand All @@ -654,24 +644,18 @@ def architecture(executable=sys.executable, bits='', linkage=''):
size = struct.calcsize('P')
bits = str(size * 8) + 'bit'

# bpo-35348: For Python executable, don't use the file command
if executable is None or executable == sys.executable:
if sys.platform == 'win32':

@serhiy-storchaka serhiy-storchaka Dec 17, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use _default_architecture?

@vstinner vstinner Dec 17, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think that it's time to drop MS-DOS and 16-bit Windows. I don't see the point of keeping a _default_architecture = {'win32': ('', 'WindowsPE')} dictionary.

@serhiy-storchaka serhiy-storchaka Dec 17, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using _default_architecture can make supporting new non-standard architectures easier.

But if you think than it is unlikely that new special cases will be added in future, perhaps remove _default_architecture?

@serhiy-storchaka serhiy-storchaka Dec 17, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please ignore the last sentence. I missed that the definition of _default_architecture was removed in the original commit.

@vstinner vstinner Dec 17, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I pushed a new commit "Add platform._DEFAULT_LINKAGE". I dislike _default_architecture: it was documented whereas it's a private attribute, it's lower-case whereas PEP 8 suggests upper-case for constants, and it requires to check if bits or linkage are empty.

linkage = 'WindowsPE'
return bits, linkage

# Get data from the 'file' system command
if executable:
fileout = _syscmd_file(executable, '')
else:
fileout = ''

if not fileout and \
executable == sys.executable:
# "file" command did not return anything; we'll try to provide
# some sensible defaults then...
if sys.platform in _default_architecture:
b, l = _default_architecture[sys.platform]
if b:
bits = b
if l:
linkage = l
return bits, linkage

if 'executable' not in fileout:
# Format not supported
return bits, linkage
Expand Down
9 changes: 8 additions & 1 deletion Lib/test/test_platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import platform
import struct
import subprocess
import sys
import sysconfig
Expand All @@ -16,7 +17,13 @@ def clear_caches(self):
platform._uname_cache = None

def test_architecture(self):
res = platform.architecture()
bits, linkage = platform.architecture()
pointer_bits = struct.calcsize('P') * 8
self.assertEqual(bits, f"{pointer_bits}bit")
if sys.platform == 'win32':
self.assertEqual(linkage, 'WindowsPE')
else:
self.assertEqual(linkage, '')

@support.skip_unless_symlink
def test_architecture_via_symlink(self): # issue3762
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2018-12-17-10-01-15.bpo-35348.rUPsJI.rst
View file
Open in desktop