Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions src/fromager/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from . import (
build_environment,
external_commands,
hooks,
overrides,
packagesettings,
requirements_file,
Expand Down Expand Up @@ -65,6 +66,13 @@ def get_build_system_dependencies(
sdist_root_dir=sdist_root_dir,
build_dir=pbi.build_dir(sdist_root_dir),
)
orig_deps = hooks.run_get_build_system_dependencies_hooks(
ctx=ctx,
req=req,
sdist_root_dir=sdist_root_dir,
build_dir=pbi.build_dir(sdist_root_dir),
requirements=list(orig_deps),
)
deps = _filter_requirements(req, orig_deps)

_write_requirements_file(
Expand Down
26 changes: 26 additions & 0 deletions src/fromager/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

# Event callbacks that run for every package (e.g., after build, after bootstrap).
GLOBAL_HOOK_NAMES: tuple[str, ...] = (
"get_build_system_dependencies",
"post_bootstrap",
"post_build",
"prebuilt_wheel",
Expand Down Expand Up @@ -71,6 +72,31 @@ def _die_on_plugin_load_failure(
raise RuntimeError(f"failed to load overrides for {ep.name}") from err


def run_get_build_system_dependencies_hooks(
ctx: context.WorkContext,
req: Requirement,
sdist_root_dir: pathlib.Path,
build_dir: pathlib.Path,
requirements: list[str],
) -> list[str]:
"""Run global hooks that post-process build-system dependencies.

Each hook receives the current requirements list and returns a
(possibly modified) list. Hooks are chained: the output of one
becomes the input of the next.
"""
hook_mgr = _get_hooks("get_build_system_dependencies")
for ext in hook_mgr:
requirements = ext.plugin(
ctx=ctx,
req=req,
sdist_root_dir=sdist_root_dir,
build_dir=build_dir,
requirements=requirements,
)
return requirements


def run_post_build_hooks(
ctx: context.WorkContext,
req: Requirement,
Expand Down
40 changes: 40 additions & 0 deletions tests/test_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,46 @@ def test_validate_dist_name_version(
validate()


@patch("fromager.dependencies._write_requirements_file")
@patch("fromager.hooks._get_hooks")
def test_get_build_system_dependencies_runs_global_hooks(
mock_get_hooks: Mock,
_: Mock,
tmp_context: context.WorkContext,
tmp_path: pathlib.Path,
) -> None:
pyproject_content = textwrap.dedent("""\
[build-system]
requires = ["setuptools>=40.0"]
build-backend = "setuptools.build_meta"
""")
(tmp_path / "pyproject.toml").write_text(pyproject_content)

called_with: dict[str, typing.Any] = {}

def fake_hook(**kwargs: typing.Any) -> list[str]:
called_with.update(kwargs)
return kwargs["requirements"] + ["setuptools<82"]

from unittest.mock import MagicMock

fake_mgr = MagicMock()
fake_mgr.names.return_value = ["fake_hook"]
fake_mgr.__iter__ = lambda self: iter([MagicMock(plugin=fake_hook)])
mock_get_hooks.return_value = fake_mgr

results = dependencies.get_build_system_dependencies(
ctx=tmp_context,
req=Requirement("testpkg"),
version=Version("1.0.0"),
sdist_root_dir=tmp_path,
)

names = set(r.name for r in results)
assert "setuptools" in names
assert called_with["requirements"] == ["setuptools>=40.0"]
Comment on lines +432 to +434

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that the hook’s returned constraint affects the result.

"setuptools" is already present before the hook runs, so this passes even if setuptools<82 is discarded. Assert that at least one returned setuptools requirement excludes version 82.

Suggested assertion
     names = set(r.name for r in results)
     assert "setuptools" in names
+    assert any(
+        r.name == "setuptools" and Version("82") not in r.specifier
+        for r in results
+    )
     assert called_with["requirements"] == ["setuptools>=40.0"]

As per path instructions, “Verify test actually tests the intended behavior.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
names = set(r.name for r in results)
assert "setuptools" in names
assert called_with["requirements"] == ["setuptools>=40.0"]
names = set(r.name for r in results)
assert "setuptools" in names
assert any(
r.name == "setuptools" and Version("82") not in r.specifier
for r in results
)
assert called_with["requirements"] == ["setuptools>=40.0"]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_dependencies.py` around lines 432 - 434, Strengthen the assertions
in the dependency hook test around the returned results so they verify a
setuptools requirement excludes version 82, rather than only checking that
setuptools is present. Preserve the existing requirements argument assertion and
ensure the new check would fail if the hook discarded the returned setuptools
constraint.

Source: Path instructions



def test_get_install_dependencies_of_wheel(tmp_path: pathlib.Path) -> None:
"""Test extracting install dependencies from a wheel file built with real tools."""
# Arrange: Build a real wheel with dependencies
Expand Down
95 changes: 95 additions & 0 deletions tests/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,101 @@ def fake_plugin(**kwargs: typing.Any) -> None:
assert "sdist_filename" not in called_with


@patch("fromager.hooks._get_hooks")
def test_run_get_build_system_dependencies_hooks_calls_plugin(
mock_get: Mock,
) -> None:
called_with: dict[str, typing.Any] = {}

def fake_plugin(**kwargs: typing.Any) -> list[str]:
called_with.update(kwargs)
return kwargs["requirements"]

mock_get.return_value = _make_fake_mgr([fake_plugin])

ctx = Mock()
req = Requirement("numpy>=1.0")
sdist = pathlib.Path("/tmp/numpy-1.0")
build = pathlib.Path("/tmp/numpy-1.0/build")
reqs = ["setuptools>=40.0", "wheel"]

result = hooks.run_get_build_system_dependencies_hooks(
ctx=ctx,
req=req,
sdist_root_dir=sdist,
build_dir=build,
requirements=reqs,
)

mock_get.assert_called_once_with("get_build_system_dependencies")
assert called_with["ctx"] is ctx
assert called_with["req"] is req
assert called_with["sdist_root_dir"] is sdist
assert called_with["build_dir"] is build
assert called_with["requirements"] == reqs
assert result == reqs


@patch("fromager.hooks._get_hooks")
def test_run_get_build_system_dependencies_hooks_chains(
mock_get: Mock,
) -> None:
def hook_a(**kwargs: typing.Any) -> list[str]:
return kwargs["requirements"] + ["extra-a"]

def hook_b(**kwargs: typing.Any) -> list[str]:
return kwargs["requirements"] + ["extra-b"]

mock_get.return_value = _make_fake_mgr([hook_a, hook_b])

result = hooks.run_get_build_system_dependencies_hooks(
ctx=Mock(),
req=Requirement("pkg"),
sdist_root_dir=pathlib.Path("/tmp/pkg"),
build_dir=pathlib.Path("/tmp/pkg/build"),
requirements=["setuptools"],
)

assert result == ["setuptools", "extra-a", "extra-b"]


@patch("fromager.hooks._get_hooks")
def test_run_get_build_system_dependencies_hooks_no_hooks(
mock_get: Mock,
) -> None:
mock_get.return_value = _make_fake_mgr([])

reqs = ["setuptools>=40.0", "wheel"]
result = hooks.run_get_build_system_dependencies_hooks(
ctx=Mock(),
req=Requirement("pkg"),
sdist_root_dir=pathlib.Path("/tmp/pkg"),
build_dir=pathlib.Path("/tmp/pkg/build"),
requirements=reqs,
)

assert result == reqs


@patch("fromager.hooks._get_hooks")
def test_run_get_build_system_dependencies_hooks_exception_propagates(
mock_get: Mock,
) -> None:
def bad_plugin(**kwargs: typing.Any) -> list[str]:
raise ValueError("hook failed")

mock_get.return_value = _make_fake_mgr([bad_plugin])

with pytest.raises(ValueError, match="hook failed"):
hooks.run_get_build_system_dependencies_hooks(
ctx=Mock(),
req=Requirement("pkg"),
sdist_root_dir=pathlib.Path("/tmp/pkg"),
build_dir=pathlib.Path("/tmp/pkg/build"),
requirements=["setuptools"],
)


@patch("fromager.hooks.overrides._get_dist_info", return_value=("mypkg", "1.0.0"))
@patch("fromager.hooks.extension.ExtensionManager")
def test_log_hooks_logs_each_extension(
Expand Down
Loading