Skip to content

Commit 2fa412e

Browse files
refactor(hooks): centralize hook name constants and add sync test
Move hardcoded hook names from list_overrides.py into GLOBAL_HOOK_NAMES (hooks.py) and OVERRIDE_HOOK_NAMES (overrides.py). Add AST-based tests that verify these constants match actual usage in the source tree. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
1 parent 7e784fb commit 2fa412e

4 files changed

Lines changed: 115 additions & 21 deletions

File tree

src/fromager/commands/list_overrides.py

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
from rich.table import Table
1010

1111
from fromager import clickext, context
12+
from fromager.hooks import GLOBAL_HOOK_NAMES
13+
from fromager.overrides import OVERRIDE_HOOK_NAMES
1214
from fromager.packagesettings import PatchMap
1315

1416

@@ -63,34 +65,15 @@ def list_overrides(
6365
variants = sorted(wkctx.settings.all_variants())
6466
variant_names = [str(v) for v in variants]
6567
export_data = []
68+
all_hook_names = GLOBAL_HOOK_NAMES + OVERRIDE_HOOK_NAMES
6669

6770
for name in overridden_packages:
6871
pbi = wkctx.settings.package_build_info(name)
6972
ps = wkctx.settings.package_setting(name)
7073

7174
plugin_hooks: list[str] = []
7275
if pbi.plugin:
73-
for hook in [
74-
# from hooks.py
75-
"post_build",
76-
"post_bootstrap",
77-
"prebuilt_wheel",
78-
# from overrides.py, found by searching for find_override_method
79-
"download_source",
80-
"get_resolver_provider",
81-
"prepare_source",
82-
"build_sdist",
83-
"build_wheel",
84-
"get_build_system_dependencies",
85-
"get_build_backend_dependencies",
86-
"get_build_sdist_dependencies",
87-
"get_install_dependencies_of_sdist",
88-
"expected_source_archive_name",
89-
"expected_source_directory_name",
90-
"add_extra_metadata_to_wheels",
91-
# from packagesettings/_hooks.py
92-
"update_extra_environ",
93-
]:
76+
for hook in all_hook_names:
9477
if hasattr(pbi.plugin, hook):
9578
plugin_hooks.append(hook)
9679
plugin_hooks_str = ", ".join(plugin_hooks)

src/fromager/hooks.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@
1616

1717
_mgrs: dict[str, hook.HookManager] = {}
1818

19+
# Event callbacks that run for every package (e.g., after build, after bootstrap).
20+
GLOBAL_HOOK_NAMES: tuple[str, ...] = (
21+
"post_bootstrap",
22+
"post_build",
23+
"prebuilt_wheel",
24+
)
25+
1926
logger = logging.getLogger(__name__)
2027

2128

src/fromager/overrides.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,23 @@
1717

1818
_mgr: extension.ExtensionManager | None = None
1919

20+
# Hooks that per-package plugins can implement to override default build behavior.
21+
OVERRIDE_HOOK_NAMES: tuple[str, ...] = (
22+
"add_extra_metadata_to_wheels",
23+
"build_sdist",
24+
"build_wheel",
25+
"download_source",
26+
"expected_source_archive_name",
27+
"expected_source_directory_name",
28+
"get_build_backend_dependencies",
29+
"get_build_sdist_dependencies",
30+
"get_build_system_dependencies",
31+
"get_install_dependencies_of_sdist",
32+
"get_resolver_provider",
33+
"prepare_source",
34+
"update_extra_environ",
35+
)
36+
2037

2138
def _get_extensions() -> extension.ExtensionManager:
2239
global _mgr

tests/test_override_hook_names.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
"""Verify that hook name constants stay in sync with actual usage.
2+
3+
Uses Python's Abstract Syntax Tree (AST) module to parse source files and
4+
find every string literal passed to hook-dispatch functions, then checks
5+
that the declared constants match.
6+
"""
7+
8+
import ast
9+
import pathlib
10+
11+
from fromager.hooks import GLOBAL_HOOK_NAMES
12+
from fromager.overrides import OVERRIDE_HOOK_NAMES
13+
14+
SRC_DIR = pathlib.Path(__file__).parent.parent / "src" / "fromager"
15+
16+
17+
def _called_function_name(node: ast.Call) -> str | None:
18+
"""Return the simple name of the called function, or None."""
19+
if isinstance(node.func, ast.Name):
20+
return node.func.id
21+
if isinstance(node.func, ast.Attribute):
22+
return node.func.attr
23+
return None
24+
25+
26+
def _collect_string_arg(
27+
source_files: list[pathlib.Path],
28+
func_names: set[str],
29+
arg_index: int,
30+
) -> set[str]:
31+
"""Find every string literal passed at ``arg_index`` to calls of ``func_names``.
32+
33+
Scans the AST of each file for calls like ``func("hook_name", ...)``
34+
and returns the set of string values found at the given position.
35+
"""
36+
found: set[str] = set()
37+
for path in source_files:
38+
tree = ast.parse(path.read_text(), filename=str(path))
39+
for node in ast.walk(tree):
40+
if not isinstance(node, ast.Call):
41+
continue
42+
if _called_function_name(node) not in func_names:
43+
continue
44+
if len(node.args) <= arg_index:
45+
continue
46+
arg = node.args[arg_index]
47+
if isinstance(arg, ast.Constant) and isinstance(arg.value, str):
48+
found.add(arg.value)
49+
return found
50+
51+
52+
def test_override_hook_names_match_usage() -> None:
53+
"""OVERRIDE_HOOK_NAMES must list every hook passed to
54+
find_override_method / find_and_invoke across the source tree."""
55+
source_files = [
56+
p
57+
for p in SRC_DIR.rglob("*.py")
58+
if p.name != "overrides.py" # skip the forwarding call (uses a variable)
59+
]
60+
used = _collect_string_arg(
61+
source_files,
62+
{"find_and_invoke", "find_override_method"},
63+
arg_index=1,
64+
)
65+
registered = set(OVERRIDE_HOOK_NAMES)
66+
missing = used - registered
67+
extra = registered - used
68+
assert not missing, (
69+
f"Hooks used in source but missing from OVERRIDE_HOOK_NAMES: {missing}"
70+
)
71+
assert not extra, f"Hooks in OVERRIDE_HOOK_NAMES but not used in source: {extra}"
72+
73+
74+
def test_global_hook_names_match_usage() -> None:
75+
"""GLOBAL_HOOK_NAMES must list every hook passed to _get_hooks in hooks.py."""
76+
used = _collect_string_arg(
77+
[SRC_DIR / "hooks.py"],
78+
{"_get_hooks"},
79+
arg_index=0,
80+
)
81+
registered = set(GLOBAL_HOOK_NAMES)
82+
missing = used - registered
83+
extra = registered - used
84+
assert not missing, (
85+
f"Hooks used in hooks.py but missing from GLOBAL_HOOK_NAMES: {missing}"
86+
)
87+
assert not extra, f"Hooks in GLOBAL_HOOK_NAMES but not used in hooks.py: {extra}"

0 commit comments

Comments
 (0)