Skip to content

Commit a6282d1

Browse files
chore(build): fix all mypy errors in setup.py (#17963)
## Description Fix a few lint errors in setup.py. Discovered when working on #17959. ## Changes - Add proper type annotations to LibraryDownload class attributes (name, version, url_root, available_releases, translate_suffix), resolving var-annotated and assignment errors in subclasses. - Suppress cmake.CMAKE_BIN_DIR attr-defined errors (runtime-only attribute not declared in the cmake package stubs). - Suppress method-assign errors for intentional debug-instrumentation monkey-patching of CustomBuildExt methods. - Wrap src.suffix in bool() to fix return type mismatch in CMakeExtension.get_sources. ## Testing * commit passed pre-commit hooks * CI Co-authored-by: vlad.scherbich <vlad.scherbich@datadoghq.com>
1 parent f383d81 commit a6282d1

2 files changed

Lines changed: 70 additions & 57 deletions

File tree

mypy.ini

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ no_implicit_reexport = false
4343
ignore_errors = true
4444
no_implicit_reexport = false
4545

46+
[mypy-setup]
47+
check_untyped_defs = true
48+
disallow_untyped_defs = false
49+
disallow_incomplete_defs = false
50+
disallow_untyped_calls = false
51+
disallow_untyped_decorators = false
52+
disallow_subclassing_any = false
53+
warn_return_any = false
54+
no_implicit_reexport = false
55+
disallow_any_generics = false
56+
4657
# docs/conf.py is treated as module "conf" since docs/ has no __init__.py.
4758

4859
[mypy-conf]

setup.py

Lines changed: 59 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
# ORDER MATTERS
3838
# Import this after setuptools or it will fail
3939
from Cython.Build import cythonize
40-
import Cython.Distutils
40+
from Cython.Distutils.extension import Extension as CythonExtension
4141
except ImportError:
4242
raise ImportError(
4343
"Failed to import Cython modules. This can happen under versions of pip older than 18 that don't "
@@ -208,8 +208,6 @@ def wrapper(*args, **kwargs):
208208
# These often indicate temporary network issues
209209
is_retriable = True
210210
error_code = f"subprocess exit code {e.returncode}"
211-
else:
212-
error_code = type(e).__name__
213211

214212
if not is_retriable:
215213
print(f"ERROR: Operation failed (non-retriable {error_code}): {e}")
@@ -299,7 +297,7 @@ def is_64_bit_python():
299297

300298

301299
class PatchedDistribution(Distribution):
302-
def __init__(self, attrs=None):
300+
def __init__(self, attrs: t.Optional[dict[str, t.Any]] = None) -> None:
303301
super().__init__(attrs)
304302
# Tell ext_hashes about your manually-built Rust artifact
305303

@@ -322,7 +320,7 @@ def __init__(self, attrs=None):
322320

323321

324322
class ExtensionHashes(build_ext):
325-
def run(self):
323+
def run(self) -> None:
326324
try:
327325
dist = self.distribution
328326
for ext in chain(dist.ext_modules, getattr(dist, "rust_extensions", [])):
@@ -395,7 +393,7 @@ def run(self):
395393
class CustomBuildRust(build_rust):
396394
"""Custom build_rust command that handles dedup_headers and header copying."""
397395

398-
def initialize_options(self):
396+
def initialize_options(self) -> None:
399397
super().initialize_options()
400398

401399
def is_installed(self, bin_file):
@@ -427,7 +425,7 @@ def cargo_install_with_retry():
427425

428426
cargo_install_with_retry()
429427

430-
def run(self):
428+
def run(self) -> None:
431429
"""Run the build process with additional post-processing."""
432430

433431
has_profiling_feature = False
@@ -466,13 +464,13 @@ class LibraryDownload:
466464
CACHE_DIR = Path(os.getenv("DD_SETUP_CACHE_DIR", HERE / ".download_cache"))
467465
USE_CACHE = os.getenv("DD_SETUP_CACHE_DOWNLOADS", "1").lower() in ("1", "yes", "on", "true")
468466

469-
name = None
470-
download_dir = Path.cwd()
471-
version = None
472-
url_root = None
473-
available_releases = {}
474-
expected_checksums = None
475-
translate_suffix = {}
467+
name: t.Optional[str] = None
468+
download_dir: Path = Path.cwd()
469+
version: t.Optional[str] = None
470+
url_root: t.Optional[str] = None
471+
available_releases: dict[str, list[str]] = {}
472+
expected_checksums: t.Optional[dict[str, dict[str, str]]] = None
473+
translate_suffix: dict[str, tuple[str, ...]] = {}
476474

477475
@classmethod
478476
def download_artifacts(cls):
@@ -565,10 +563,10 @@ def download_file(url, dest):
565563
# Open the tarfile first to get the files needed.
566564
# This could be solved with "r:gz" mode, that allows random access
567565
# but that approach does not work on Windows
568-
with tarfile.open(filename, "r|gz", errorlevel=2) as tar:
566+
with tarfile.open(filename, mode="r|gz", errorlevel=2) as tar:
569567
dynfiles = [c for c in tar.getmembers() if c.name.endswith(suffixes)]
570568

571-
with tarfile.open(filename, "r|gz", errorlevel=2) as tar:
569+
with tarfile.open(filename, mode="r|gz", errorlevel=2) as tar:
572570
tar.extractall(members=dynfiles, path=HERE)
573571
Path(HERE / archive_dir).rename(arch_dir)
574572

@@ -588,7 +586,7 @@ def download_file(url, dest):
588586
(download_dir / ".version").write_text(cls.version)
589587

590588
@classmethod
591-
def run(cls):
589+
def run(cls) -> None:
592590
cls.download_artifacts()
593591

594592
@classmethod
@@ -651,7 +649,7 @@ def get_archive_name(cls, arch, os):
651649

652650

653651
class LibraryDownloader(BuildPyCommand):
654-
def run(self):
652+
def run(self) -> None:
655653
# The setuptools docs indicate the `editable_mode` attribute of the build_py command class
656654
# is set to True when the package is being installed in editable mode, which we need to know
657655
# for some extensions
@@ -699,7 +697,7 @@ def _strip_build_artifacts(self):
699697

700698
class CleanLibraries(CleanCommand):
701699
@staticmethod
702-
def remove_native_extensions():
700+
def remove_native_extensions() -> None:
703701
"""Remove native extensions and shared libraries installed by setup.py."""
704702
for pattern in ("*.so", "*.pyd", "*.dylib", "*.dll"):
705703
for path in DDTRACE_DIR.rglob(pattern):
@@ -711,20 +709,20 @@ def remove_native_extensions():
711709
print(f"WARNING: could not remove {path}: {e}")
712710

713711
@staticmethod
714-
def remove_artifacts():
712+
def remove_artifacts() -> None:
715713
shutil.rmtree(LIBDDWAF_DOWNLOAD_DIR, True)
716714
CleanLibraries.remove_native_extensions()
717715

718716
@staticmethod
719-
def remove_rust_targets():
717+
def remove_rust_targets() -> None:
720718
"""Remove all Rust target dirs (target, target3.9, target3.10, etc.)."""
721719
# rmtree is a superset of `cargo clean`; target* catches plain target and versioned
722720
for target_dir in NATIVE_CRATE.glob("target*"):
723721
if target_dir.is_dir():
724722
shutil.rmtree(target_dir, True)
725723

726724
@staticmethod
727-
def remove_build_artifacts():
725+
def remove_build_artifacts() -> None:
728726
"""Remove egg-info, dist, .eggs, *.egg, and CMake FetchContent cache.
729727
730728
The base distutils clean command does not remove these. They can cause
@@ -744,7 +742,7 @@ def remove_build_artifacts():
744742
shutil.rmtree(cmake_deps, True)
745743

746744
@staticmethod
747-
def remove_build_dir():
745+
def remove_build_dir() -> None:
748746
"""Remove the entire build/ tree for a clean slate.
749747
750748
The base CleanCommand only removes specific subdirs (build_temp, build_lib, etc.)
@@ -754,7 +752,7 @@ def remove_build_dir():
754752
if build_dir.exists():
755753
shutil.rmtree(build_dir, True)
756754

757-
def run(self):
755+
def run(self) -> None:
758756
CleanLibraries.remove_rust_targets()
759757
CleanLibraries.remove_artifacts()
760758
CleanLibraries.remove_build_dir()
@@ -837,7 +835,7 @@ def _absl_should_skip() -> bool:
837835
class CustomBuildExt(build_ext):
838836
INCREMENTAL = os.getenv("DD_CMAKE_INCREMENTAL_BUILD", "1").lower() in ("1", "yes", "on", "true")
839837

840-
def run(self):
838+
def run(self) -> None:
841839
with _time_phase("build_rust"):
842840
self.build_rust()
843841

@@ -855,7 +853,7 @@ def run(self):
855853
with _time_phase("build_extensions"):
856854
super().run()
857855

858-
def build_extensions(self):
856+
def build_extensions(self) -> None:
859857
# Enable parallel extension builds by default. All extensions are
860858
# independent at this point (Rust and libdd_wrapper are already built
861859
# in run()), so they can safely compile concurrently. The user can
@@ -900,14 +898,14 @@ def build_rust(self):
900898
cargo_files = [NATIVE_CRATE / "Cargo.toml", NATIVE_CRATE / "Cargo.lock"]
901899

902900
# Get all source files (including subdirectories)
903-
source_files = []
901+
source_files: list[Path] = []
904902
# Find all .rs files in the crate (including subdirectories)
905903
source_files.extend(NATIVE_CRATE.glob("**/*.rs"))
906904
# Add cargo files
907905
source_files.extend([f for f in cargo_files if f.exists()])
908906

909907
# Check if any source file is newer than the library
910-
newest_source_time = 0
908+
newest_source_time: float = 0.0
911909
for src_file in source_files:
912910
if src_file.exists():
913911
newest_source_time = max(newest_source_time, src_file.stat().st_mtime)
@@ -970,13 +968,13 @@ def build_libdd_wrapper(self):
970968
wrapper_mtime = wrapper_library.stat().st_mtime
971969

972970
# Check dd_wrapper source files
973-
source_files = []
971+
source_files: list[Path] = []
974972
source_files.extend(dd_wrapper_dir.glob("**/*.cpp"))
975973
source_files.extend(dd_wrapper_dir.glob("**/*.hpp"))
976974
source_files.extend([dd_wrapper_dir / "CMakeLists.txt"])
977975

978976
# Check if any source file is newer than the wrapper library
979-
newest_source_time = 0
977+
newest_source_time: float = 0.0
980978
for src_file in source_files:
981979
if src_file.exists():
982980
newest_source_time = max(newest_source_time, src_file.stat().st_mtime)
@@ -999,7 +997,7 @@ def build_libdd_wrapper(self):
999997

1000998
install_args = [f"--config {COMPILE_MODE}"]
1001999

1002-
cmake_command = (Path(cmake.CMAKE_BIN_DIR) / "cmake").resolve()
1000+
cmake_command = (Path(cmake.CMAKE_BIN_DIR) / "cmake").resolve() # type: ignore[attr-defined]
10031001
subprocess.run([cmake_command, *cmake_args], cwd=cmake_build_dir, check=True)
10041002
subprocess.run([cmake_command, "--build", ".", *build_args], cwd=cmake_build_dir, check=True)
10051003
subprocess.run([cmake_command, "--install", ".", *install_args], cwd=cmake_build_dir, check=True)
@@ -1038,7 +1036,7 @@ def _build_shared_dep(self, dep: SharedDep) -> None:
10381036
cmake_build_dir = Path(self.build_lib.replace("lib.", "cmake."), f"{dep.name}_build").resolve()
10391037
cmake_build_dir.mkdir(parents=True, exist_ok=True)
10401038

1041-
cmake_command = (Path(cmake.CMAKE_BIN_DIR) / "cmake").resolve()
1039+
cmake_command = (Path(cmake.CMAKE_BIN_DIR) / "cmake").resolve() # type: ignore[attr-defined]
10421040

10431041
cmake_args = self._base_cmake_args() + [
10441042
f"-S{dep.cmake_dir}",
@@ -1077,7 +1075,7 @@ def try_strip_symbols(so_file):
10771075
"WARNING: An error occurred while stripping the symbols from '{}', ignoring: {}".format(so_file, e)
10781076
)
10791077

1080-
def build_extension(self, ext):
1078+
def build_extension(self, ext: Extension) -> None:
10811079
if isinstance(ext, CMakeExtension):
10821080
try:
10831081
self.build_extension_cmake(ext)
@@ -1189,7 +1187,7 @@ def _get_common_cmake_args(self, source_dir, build_dir, output_dir, extension_na
11891187
# Forward the install path of every successfully pre-built shared dep so
11901188
# each consuming extension's CMakeLists.txt can use find_package() instead
11911189
# of running its own FetchContent build.
1192-
built = getattr(self, "_built_shared_deps", set())
1190+
built: set[str] = getattr(self, "_built_shared_deps", set())
11931191
for dep in SHARED_DEPS:
11941192
if dep.name in built:
11951193
cmake_args += [f"-D{dep.cmake_var}={dep.install_dir}"]
@@ -1334,7 +1332,7 @@ def build_extension_cmake(self, ext: "CMakeExtension") -> None:
13341332
"-DCMAKE_CXX_FLAGS_%s=-O0" % ext.build_type.upper(),
13351333
]
13361334
cmake_command = (
1337-
Path(cmake.CMAKE_BIN_DIR) / "cmake"
1335+
Path(cmake.CMAKE_BIN_DIR) / "cmake" # type: ignore[attr-defined]
13381336
).resolve() # explicitly use the cmake provided by the cmake package
13391337
subprocess.run([cmake_command, *cmake_args], cwd=cmake_build_dir, check=True)
13401338
subprocess.run([cmake_command, "--build", ".", *build_args], cwd=cmake_build_dir, check=True)
@@ -1391,7 +1389,7 @@ def dump_metadata(cls):
13911389
for ext, elapsed_ns in sorted(cls.build_times.items(), key=lambda x: x[1], reverse=True):
13921390
elapsed_s = elapsed_ns / 1e9
13931391
ext_percent = (elapsed_ns / total_ns) * 100.0
1394-
f.write(f"\t{ext.name}: {elapsed_s:0.2f}s ({ext_percent:0.2f}%)\n")
1392+
f.write(f"\t{ext}: {elapsed_s:0.2f}s ({ext_percent:0.2f}%)\n")
13951393

13961394
if cls.shared_dep_times:
13971395
f.write("Shared dependency build times:\n")
@@ -1447,25 +1445,29 @@ def _time_phase(name: str):
14471445

14481446
if DebugMetadata.enabled:
14491447
DebugMetadata.start_ns = time.time_ns()
1450-
CustomBuildExt.build_extension = debug_build_extension(CustomBuildExt.build_extension)
1451-
CustomBuildExt._build_shared_dep = debug_build_shared_dep(CustomBuildExt._build_shared_dep)
1448+
CustomBuildExt.build_extension = ( # type: ignore[method-assign]
1449+
debug_build_extension(CustomBuildExt.build_extension)
1450+
)
1451+
CustomBuildExt._build_shared_dep = ( # type: ignore[method-assign]
1452+
debug_build_shared_dep(CustomBuildExt._build_shared_dep)
1453+
)
14521454
build_rust.build_extension = debug_build_extension(CustomBuildRust.build_extension)
14531455
atexit.register(DebugMetadata.dump_metadata)
14541456

14551457

14561458
class CMakeExtension(Extension):
14571459
def __init__(
14581460
self,
1459-
name,
1460-
source_dir=Path.cwd(),
1461-
extra_source_dirs=[],
1462-
cmake_args=[],
1463-
build_args=[],
1464-
install_args=[],
1465-
build_type=None,
1466-
optional=True, # By default, extensions are optional
1467-
dependencies=[],
1468-
):
1461+
name: str,
1462+
source_dir: Path = Path.cwd(),
1463+
extra_source_dirs: list[Path] = [],
1464+
cmake_args: list[str] = [],
1465+
build_args: list[str] = [],
1466+
install_args: list[str] = [],
1467+
build_type: t.Optional[str] = None,
1468+
optional: bool = True, # By default, extensions are optional
1469+
dependencies: list[Path] = [],
1470+
) -> None:
14691471
super().__init__(name, sources=[])
14701472
self.source_dir = source_dir
14711473
self.extra_source_dirs = extra_source_dirs # extra source dirs to include when computing extension hash
@@ -1488,7 +1490,7 @@ def get_sources(self) -> list[Path]:
14881490
def is_valid_source(src: Path) -> bool:
14891491
return (
14901492
src.is_file()
1491-
and src.suffix
1493+
and bool(src.suffix)
14921494
# Exclude compiled/generated artifacts that are not real sources:
14931495
# .so/.dylib/.dll/.pyd — compiled native extensions (from co-located Cython exts)
14941496
# .c — Cython-generated C files (present in shared source directories)
@@ -1571,7 +1573,7 @@ def get_exts_for(name):
15711573

15721574

15731575
if not IS_PYSTON:
1574-
ext_modules: list[t.Union[Extension, Cython.Distutils.Extension, RustExtension]] = [
1576+
ext_modules: list[t.Union[Extension, CythonExtension, RustExtension]] = [
15751577
Extension(
15761578
"ddtrace.internal._threads",
15771579
sources=["ddtrace/internal/_threads.cpp"],
@@ -1654,7 +1656,7 @@ def get_exts_for(name):
16541656
if os.getenv("DD_CYTHONIZE", "1").lower() in ("1", "yes", "on", "true"):
16551657
with _time_phase("cythonize"):
16561658
_cython_sources = [
1657-
Cython.Distutils.Extension(
1659+
CythonExtension(
16581660
"ddtrace.internal._tagset",
16591661
sources=["ddtrace/internal/_tagset.pyx"],
16601662
language="c",
@@ -1675,32 +1677,32 @@ def get_exts_for(name):
16751677

16761678
if sys.version_info < (3, 15):
16771679
_cython_sources += [
1678-
Cython.Distutils.Extension(
1680+
CythonExtension(
16791681
"ddtrace.profiling._threading",
16801682
sources=["ddtrace/profiling/_threading.pyx"],
16811683
language="c",
16821684
),
1683-
Cython.Distutils.Extension(
1685+
CythonExtension(
16841686
"ddtrace.profiling.collector._task",
16851687
sources=["ddtrace/profiling/collector/_task.pyx"],
16861688
language="c",
16871689
),
1688-
Cython.Distutils.Extension(
1690+
CythonExtension(
16891691
"ddtrace.profiling.collector._exception",
16901692
sources=["ddtrace/profiling/collector/_exception.pyx"],
16911693
language="c",
16921694
),
1693-
Cython.Distutils.Extension(
1695+
CythonExtension(
16941696
"ddtrace.profiling.collector._fast_poisson",
16951697
sources=["ddtrace/profiling/collector/_fast_poisson.pyx"],
16961698
language="c",
16971699
),
1698-
Cython.Distutils.Extension(
1700+
CythonExtension(
16991701
"ddtrace.profiling.collector._sampler",
17001702
sources=["ddtrace/profiling/collector/_sampler.pyx"],
17011703
language="c",
17021704
),
1703-
Cython.Distutils.Extension(
1705+
CythonExtension(
17041706
"ddtrace.profiling.collector._lock",
17051707
sources=["ddtrace/profiling/collector/_lock.pyx"],
17061708
language="c",

0 commit comments

Comments
 (0)