[Python-checkins] bpo-42212: smelly.py also checks the dynamic library (GH-23423)
vstinner
webhook-mailer at python.org
Tue Nov 24 07:38:17 EST 2020
https://github.com/python/cpython/commit/ac7d0169d2bce2021b8d88973e649889d7dc1b03
commit: ac7d0169d2bce2021b8d88973e649889d7dc1b03
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2020-11-24T13:38:08+01:00
summary:
bpo-42212: smelly.py also checks the dynamic library (GH-23423)
The smelly.py script now also checks the Python dynamic library and
extension modules, not only the Python static library. Make also the
script more verbose: explain what it does.
The GitHub Action job now builds Python with the libpython dynamic
library.
files:
A Misc/NEWS.d/next/Tools-Demos/2020-11-20-15-11-05.bpo-42212.sjzgOf.rst
M .github/workflows/build.yml
M Tools/scripts/smelly.py
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index c1a017165665f..f543a94af363b 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -60,7 +60,8 @@ jobs:
run: sudo ./.github/workflows/posix-deps-apt.sh
- name: Build CPython
run: |
- ./configure --with-pydebug
+ # Build Python with the libpython dynamic library
+ ./configure --with-pydebug --enable-shared
make -j4 regen-all
- name: Check for changes
run: |
diff --git a/Misc/NEWS.d/next/Tools-Demos/2020-11-20-15-11-05.bpo-42212.sjzgOf.rst b/Misc/NEWS.d/next/Tools-Demos/2020-11-20-15-11-05.bpo-42212.sjzgOf.rst
new file mode 100644
index 0000000000000..d2cbe3de6fe92
--- /dev/null
+++ b/Misc/NEWS.d/next/Tools-Demos/2020-11-20-15-11-05.bpo-42212.sjzgOf.rst
@@ -0,0 +1,3 @@
+The smelly.py script now also checks the Python dynamic library and extension
+modules, not only the Python static library. Make also the script more verbose:
+explain what it does.
diff --git a/Tools/scripts/smelly.py b/Tools/scripts/smelly.py
index 43d091654d2dc..e8a375c808cda 100755
--- a/Tools/scripts/smelly.py
+++ b/Tools/scripts/smelly.py
@@ -1,17 +1,47 @@
#!/usr/bin/env python
# Script checking that all symbols exported by libpython start with Py or _Py
+import os.path
import subprocess
import sys
import sysconfig
-def get_exported_symbols():
- LIBRARY = sysconfig.get_config_var('LIBRARY')
- if not LIBRARY:
- raise Exception("failed to get LIBRARY")
+ALLOWED_PREFIXES = ('Py', '_Py')
+if sys.platform == 'darwin':
+ ALLOWED_PREFIXES += ('__Py',)
+
+IGNORED_EXTENSION = "_ctypes_test"
+# Ignore constructor and destructor functions
+IGNORED_SYMBOLS = {'_init', '_fini'}
+
+
+def is_local_symbol_type(symtype):
+ # Ignore local symbols.
+
+ # If lowercase, the symbol is usually local; if uppercase, the symbol
+ # is global (external). There are however a few lowercase symbols that
+ # are shown for special global symbols ("u", "v" and "w").
+ if symtype.islower() and symtype not in "uvw":
+ return True
+
+ # Ignore the initialized data section (d and D) and the BSS data
+ # section. For example, ignore "__bss_start (type: B)"
+ # and "_edata (type: D)".
+ if symtype in "bBdD":
+ return True
+
+ return False
- args = ('nm', '-p', LIBRARY)
+
+def get_exported_symbols(library, dynamic=False):
+ print(f"Check that {library} only exports symbols starting with Py or _Py")
+
+ # Only look at dynamic symbols
+ args = ['nm', '--no-sort']
+ if dynamic:
+ args.append('--dynamic')
+ args.append(library)
print("+ %s" % ' '.join(args))
proc = subprocess.run(args, stdout=subprocess.PIPE, universal_newlines=True)
if proc.returncode:
@@ -25,12 +55,9 @@ def get_exported_symbols():
def get_smelly_symbols(stdout):
- symbols = []
- ignored_symtypes = set()
-
- allowed_prefixes = ('Py', '_Py')
- if sys.platform == 'darwin':
- allowed_prefixes += ('__Py',)
+ smelly_symbols = []
+ python_symbols = []
+ local_symbols = []
for line in stdout.splitlines():
# Split line '0000000000001b80 D PyTextIOWrapper_Type'
@@ -42,41 +69,98 @@ def get_smelly_symbols(stdout):
continue
symtype = parts[1].strip()
- # Ignore private symbols.
- #
- # If lowercase, the symbol is usually local; if uppercase, the symbol
- # is global (external). There are however a few lowercase symbols that
- # are shown for special global symbols ("u", "v" and "w").
- if symtype.islower() and symtype not in "uvw":
- ignored_symtypes.add(symtype)
+ symbol = parts[-1]
+ result = '%s (type: %s)' % (symbol, symtype)
+
+ if symbol.startswith(ALLOWED_PREFIXES):
+ python_symbols.append(result)
continue
- symbol = parts[-1]
- if symbol.startswith(allowed_prefixes):
+ if is_local_symbol_type(symtype):
+ local_symbols.append(result)
+ elif symbol in IGNORED_SYMBOLS:
+ local_symbols.append(result)
+ else:
+ smelly_symbols.append(result)
+
+ if local_symbols:
+ print(f"Ignore {len(local_symbols)} local symbols")
+ return smelly_symbols, python_symbols
+
+
+def check_library(library, dynamic=False):
+ nm_output = get_exported_symbols(library, dynamic)
+ smelly_symbols, python_symbols = get_smelly_symbols(nm_output)
+
+ if not smelly_symbols:
+ print(f"OK: no smelly symbol found ({len(python_symbols)} Python symbols)")
+ return 0
+
+ print()
+ smelly_symbols.sort()
+ for symbol in smelly_symbols:
+ print("Smelly symbol: %s" % symbol)
+
+ print()
+ print("ERROR: Found %s smelly symbols!" % len(smelly_symbols))
+ return len(smelly_symbols)
+
+
+def check_extensions():
+ print(__file__)
+ srcdir = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+ filename = os.path.join(srcdir, "pybuilddir.txt")
+ try:
+ with open(filename, encoding="utf-8") as fp:
+ pybuilddir = fp.readline()
+ except FileNotFoundError:
+ print(f"Cannot check extensions because {filename} does not exist")
+ return True
+
+ print(f"Check extension modules from {pybuilddir} directory")
+ builddir = os.path.join(srcdir, pybuilddir)
+ nsymbol = 0
+ for name in os.listdir(builddir):
+ if not name.endswith(".so"):
+ continue
+ if IGNORED_EXTENSION in name:
+ print()
+ print(f"Ignore extension: {name}")
continue
- symbol = '%s (type: %s)' % (symbol, symtype)
- symbols.append(symbol)
- if ignored_symtypes:
- print("Ignored symbol types: %s" % ', '.join(sorted(ignored_symtypes)))
print()
- return symbols
+ filename = os.path.join(builddir, name)
+ nsymbol += check_library(filename, dynamic=True)
+
+ return nsymbol
def main():
- nm_output = get_exported_symbols()
- symbols = get_smelly_symbols(nm_output)
+ # static library
+ LIBRARY = sysconfig.get_config_var('LIBRARY')
+ if not LIBRARY:
+ raise Exception("failed to get LIBRARY variable from sysconfig")
+ nsymbol = check_library(LIBRARY)
+
+ # dynamic library
+ LDLIBRARY = sysconfig.get_config_var('LDLIBRARY')
+ if not LDLIBRARY:
+ raise Exception("failed to get LDLIBRARY variable from sysconfig")
+ if LDLIBRARY != LIBRARY:
+ print()
+ nsymbol += check_library(LDLIBRARY, dynamic=True)
- if not symbols:
- print("OK: no smelly symbol found")
- sys.exit(0)
+ # Check extension modules like _ssl.cpython-310d-x86_64-linux-gnu.so
+ nsymbol += check_extensions()
+
+ if nsymbol:
+ print()
+ print(f"ERROR: Found {nsymbol} smelly symbols in total!")
+ sys.exit(1)
- symbols.sort()
- for symbol in symbols:
- print("Smelly symbol: %s" % symbol)
print()
- print("ERROR: Found %s smelly symbols!" % len(symbols))
- sys.exit(1)
+ print(f"OK: all exported symbols of all libraries "
+ f"are prefixed with {' or '.join(map(repr, ALLOWED_PREFIXES))}")
if __name__ == "__main__":
More information about the Python-checkins
mailing list