Skip to content

Commit b8ac8d9

Browse files
encukouencukou
committed
Refactor smelly.py
First gather data, then present the results.
1 parent b62d565 commit b8ac8d9

File tree

1 file changed

+105
-115
lines changed

1 file changed

+105
-115
lines changed

‎Tools/build/smelly.py‎

Lines changed: 105 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
#!/usr/bin/env python
22
# Script checking that all symbols exported by libpython start with Py or _Py
33

4-
import os.path
4+
import dataclasses
5+
import functools
6+
import pathlib
57
import subprocess
68
import sys
79
import sysconfig
8-
import _colorize
910

1011
ALLOWED_PREFIXES = ('Py', '_Py')
1112
if sys.platform == 'darwin':
@@ -22,169 +23,158 @@
2223
})
2324

2425
IGNORED_EXTENSION = "_ctypes_test"
25-
# Ignore constructor and destructor functions
26-
IGNORED_SYMBOLS = {'_init', '_fini'}
2726

2827

29-
def is_local_symbol_type(symtype):
30-
# Ignore local symbols.
31-
32-
# If lowercase, the symbol is usually local; if uppercase, the symbol
33-
# is global (external). There are however a few lowercase symbols that
34-
# are shown for special global symbols ("u", "v" and "w").
35-
if symtype.islower() and symtype not in "uvw":
36-
return True
37-
38-
# Ignore the initialized data section (d and D) and the BSS data
39-
# section. For example, ignore "__bss_start (type: B)"
40-
# and "_edata (type: D)".
41-
if symtype in "bBdD":
28+
@dataclasses.dataclass
29+
class Library:
30+
path: pathlib.Path
31+
is_dynamic: bool
32+
33+
@functools.cached_property
34+
def is_ignored(self):
35+
name_without_extemnsions = self.path.name.partition('.')[0]
36+
return name_without_extemnsions == IGNORED_EXTENSION
37+
38+
39+
@dataclasses.dataclass
40+
class Symbol:
41+
name: str
42+
type: str
43+
library: str
44+
45+
def __str__(self):
46+
return f"{self.name!r} (type {self.type}) from {self.library.path}"
47+
48+
@functools.cached_property
49+
def is_local(self):
50+
# If lowercase, the symbol is usually local; if uppercase, the symbol
51+
# is global (external). There are however a few lowercase symbols that
52+
# are shown for special global symbols ("u", "v" and "w").
53+
if self.type.islower() and self.type not in "uvw":
54+
return True
55+
56+
return False
57+
58+
@functools.cached_property
59+
def is_smelly(self):
60+
if self.is_local:
61+
return False
62+
if self.name.startswith(ALLOWED_PREFIXES):
63+
return False
64+
if self.name in EXCEPTIONS:
65+
return False
66+
if not self.library.is_dynamic and self.name.startswith(
67+
ALLOWED_STATIC_PREFIXES):
68+
return False
69+
if self.library.is_ignored:
70+
return False
4271
return True
4372

44-
return False
73+
@functools.cached_property
74+
def _sort_key(self):
75+
return self.name, self.library.path
4576

77+
def __lt__(self, other_symbol):
78+
return self._sort_key < other_symbol._sort_key
4679

47-
def get_exported_symbols(library, dynamic=False, *, colors):
48-
name, dot, ext = os.path.basename(library).partition('.')
49-
print(f"Check {colors.INTENSE_WHITE}{name}{colors.RESET}{dot}{ext}")
5080

81+
def get_exported_symbols(library):
5182
# Only look at dynamic symbols
5283
args = ['nm', '--no-sort']
53-
if dynamic:
84+
if library.is_dynamic:
5485
args.append('--dynamic')
55-
args.append(library)
56-
print(f"+ {' '.join(args)}")
86+
args.append(library.path)
5787
proc = subprocess.run(args, stdout=subprocess.PIPE, encoding='utf-8')
5888
if proc.returncode:
89+
print("+", args)
5990
sys.stdout.write(proc.stdout)
6091
sys.exit(proc.returncode)
6192

6293
stdout = proc.stdout.rstrip()
6394
if not stdout:
6495
raise Exception("command output is empty")
65-
return stdout
66-
67-
68-
def get_smelly_symbols(stdout, dynamic=False):
69-
smelly_symbols = []
70-
python_symbols = []
71-
local_symbols = []
7296

97+
symbols = []
7398
for line in stdout.splitlines():
74-
# Split line '0000000000001b80 D PyTextIOWrapper_Type'
7599
if not line:
76100
continue
77101

102+
# Split lines like '0000000000001b80 D PyTextIOWrapper_Type'
78103
parts = line.split(maxsplit=2)
104+
# Ignore lines like ' U PyDict_SetItemString'
105+
# and headers like 'pystrtod.o:'
79106
if len(parts) < 3:
80107
continue
81108

82-
symtype = parts[1].strip()
83-
symbol = parts[-1]
84-
result = f'{symbol} (type: {symtype})'
85-
86-
if (symbol.startswith(ALLOWED_PREFIXES) or
87-
symbol in EXCEPTIONS or
88-
(not dynamic and symbol.startswith(ALLOWED_STATIC_PREFIXES))):
89-
python_symbols.append(result)
90-
continue
91-
92-
if is_local_symbol_type(symtype):
93-
local_symbols.append(result)
94-
elif symbol in IGNORED_SYMBOLS:
95-
local_symbols.append(result)
96-
else:
97-
smelly_symbols.append(result)
98-
99-
if local_symbols:
100-
print(f"Ignore {len(local_symbols)} local symbols")
101-
return smelly_symbols, python_symbols
102-
103-
104-
def check_library(library, dynamic=False, *, colors):
105-
nm_output = get_exported_symbols(library, dynamic, colors=colors)
106-
smelly_symbols, python_symbols = get_smelly_symbols(nm_output, dynamic)
109+
symbol = Symbol(name=parts[-1], type=parts[1], library=library)
110+
if not symbol.is_local:
111+
symbols.append(symbol)
107112

108-
if not smelly_symbols:
109-
print(f"{colors.GREEN}OK{colors.RESET}:",
110-
f"no smelly symbol found ({len(python_symbols)} Python symbols)")
111-
return 0
113+
return symbols
112114

113-
print()
114-
smelly_symbols.sort()
115-
for symbol in smelly_symbols:
116-
print(f"Smelly symbol: {symbol}")
117115

118-
print()
119-
print(f"{colors.RED}ERROR{colors.RESET}:",
120-
f"Found {len(smelly_symbols)} smelly symbols!")
121-
return len(smelly_symbols)
122-
123-
124-
def check_extensions(colors):
125-
print(__file__)
116+
def get_extension_libraries():
126117
# This assumes pybuilddir.txt is in same directory as pyconfig.h.
127118
# In the case of out-of-tree builds, we can't assume pybuilddir.txt is
128119
# in the source folder.
129-
config_dir = os.path.dirname(sysconfig.get_config_h_filename())
130-
filename = os.path.join(config_dir, "pybuilddir.txt")
120+
config_dir = pathlib.Path(sysconfig.get_config_h_filename()).parent
131121
try:
132-
with open(filename, encoding="utf-8") as fp:
133-
pybuilddir = fp.readline()
134-
except FileNotFoundError:
135-
print(f"Cannot check extensions because {filename} does not exist")
136-
return True
137-
138-
print(f"Check extension modules from {pybuilddir} directory")
139-
builddir = os.path.join(config_dir, pybuilddir)
140-
nsymbol = 0
141-
for name in os.listdir(builddir):
142-
if not name.endswith(".so"):
143-
continue
144-
if IGNORED_EXTENSION in name:
145-
print()
146-
print(f"Ignore extension: {name}")
122+
config_dir = config_dir.relative_to(pathlib.Path.cwd(), walk_up=True)
123+
except ValueError:
124+
pass
125+
filename = config_dir / "pybuilddir.txt"
126+
pybuilddir = filename.read_text().strip()
127+
128+
builddir = config_dir / pybuilddir
129+
result = []
130+
for path in builddir.glob('**/*.so'):
131+
if path.stem == IGNORED_EXTENSION:
147132
continue
133+
result.append(Library(path, is_dynamic=True))
148134

149-
print()
150-
filename = os.path.join(builddir, name)
151-
nsymbol += check_library(filename, dynamic=True, colors=colors)
152-
153-
return nsymbol
135+
return result
154136

155137

156138
def main():
157-
colors = _colorize.get_colors()
158-
159-
nsymbol = 0
139+
libraries = []
160140

161141
# static library
162-
LIBRARY = sysconfig.get_config_var('LIBRARY')
163-
if not LIBRARY:
164-
raise Exception("failed to get LIBRARY variable from sysconfig")
165-
if os.path.exists(LIBRARY):
166-
nsymbol += check_library(LIBRARY, colors=colors)
142+
try:
143+
LIBRARY = pathlib.Path(sysconfig.get_config_var('LIBRARY'))
144+
except TypeError as exc:
145+
raise Exception("failed to get LIBRARY sysconfig variable") from exc
146+
LIBRARY = pathlib.Path(LIBRARY)
147+
if LIBRARY.exists():
148+
libraries.append(Library(LIBRARY, is_dynamic=False))
167149

168150
# dynamic library
169-
LDLIBRARY = sysconfig.get_config_var('LDLIBRARY')
170-
if not LDLIBRARY:
171-
raise Exception("failed to get LDLIBRARY variable from sysconfig")
151+
try:
152+
LDLIBRARY = pathlib.Path(sysconfig.get_config_var('LDLIBRARY'))
153+
except TypeError as exc:
154+
raise Exception("failed to get LDLIBRARY sysconfig variable") from exc
172155
if LDLIBRARY != LIBRARY:
173-
print()
174-
nsymbol += check_library(LDLIBRARY, dynamic=True, colors=colors)
156+
libraries.append(Library(LDLIBRARY, is_dynamic=True))
175157

176158
# Check extension modules like _ssl.cpython-310d-x86_64-linux-gnu.so
177-
nsymbol += check_extensions(colors=colors)
159+
libraries.extend(get_extension_libraries())
178160

179-
if nsymbol:
161+
smelly_symbols = []
162+
for library in libraries:
163+
symbols = get_exported_symbols(library)
164+
print(f"{library.path}: {len(symbols)} symbol(s) found")
165+
for symbol in symbols:
166+
print(" -", symbol.name)
167+
if symbol.is_smelly:
168+
smelly_symbols.append(symbol)
169+
170+
if smelly_symbols:
180171
print()
181-
print(f"{colors.RED}ERROR{colors.RESET}:",
182-
f"Found {nsymbol} smelly symbols in total!")
172+
print(f"Found {len(smelly_symbols)} smelly symbols in total!")
173+
for symbol in sorted(smelly_symbols):
174+
print(f" - {symbol.name} from {symbol.library.path}")
183175
sys.exit(1)
184176

185-
print()
186-
print(f"{colors.GREEN}OK{colors.RESET}:",
187-
f"all exported symbols of all libraries",
177+
print(f"OK: all exported symbols of all libraries",
188178
f"are prefixed with {' or '.join(map(repr, ALLOWED_PREFIXES))}",
189179
f"or are covered by exceptions")
190180

0 commit comments

Comments
 (0)