feat: add settings-driven build_system_dependencies_hook

[vshawrh]

Summary

Adds a configurable build_system_dependencies_hook in the global
settings.yaml that post-processes build-system dependencies for all
packages. The hook is specified as a dotted import path and loaded via
Pydantic ImportString, following the same pattern as the proposed
build_tag_hook.

Motivation: In downstream projects, many identical per-package
plugins exist solely to cap setuptools for packages whose setup.py
uses removed APIs (pkg_resources, dry_run). With this hook, they
can all be replaced by a single callable.

Changes

• src/fromager/packagesettings/_settings.py: Added
  build_system_dependencies_hook field to SettingsFile (Pydantic
  ImportString | None) and a @property on Settings to expose it.
• src/fromager/dependencies.py: After overrides.find_and_invoke()
  returns and before _filter_requirements(), the hook is called if
  configured.
• tests/test_dependencies.py: Tests for the hook integration,
  settings field defaults, and property access.

Configuration

build_system_dependencies_hook: "my_package.hooks:fix_build_deps"

Hook signature

def fix_build_deps(
    *,
    ctx: context.WorkContext,
    req: Requirement,
    sdist_root_dir: pathlib.Path,
    build_dir: pathlib.Path,
    requirements: list[str],
) -> list[str]:
    ...

How it fits

The hook runs after per-package overrides (overrides.find_and_invoke)
and before marker filtering (_filter_requirements). Per-package
plugins still take full precedence, and the hook only augments the result.

Proposal: #1272
Closes #1263

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds an optional build_system_dependencies_hook setting. Settings exposes the configured callable. get_build_system_dependencies invokes the hook after initial resolution and before filtering and persistence. Tests cover configured hooks, default behavior, and settings exposure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: 🔵 Low · up to 3d7b5

The PR adds a global hook for post-processing build-system dependencies. It is mergeable with owner awareness that the integration tests should use the same source directory for the build environment and dependency resolution to avoid masking localized regressions.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR adds a settings hook but does not implement the required fromager.hooks registration, execution, or hook chaining from issue #1263.	Add get_build_system_dependencies to GLOBAL_HOOK_NAMES and implement chained execution through fromager.hooks before marker filtering.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The changes are limited to build-dependency hook configuration, integration, and tests, with no unrelated code changes.
Title check	✅ Passed	The title clearly identifies the addition of a settings-driven build-system dependency hook, which is the main change.
Description check	✅ Passed	The description explains the configurable hook, its execution point, configuration, motivation, and related tests.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/test_dependencies.py`:
- Around line 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.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4fdc1ee4-6f73-4d81-b707-e30713142a59

📥 Commits

Reviewing files that changed from the base of the PR and between d249228 and 2ffbe2d.

📒 Files selected for processing (4)

• src/fromager/dependencies.py
• src/fromager/hooks.py
• tests/test_dependencies.py
• tests/test_hooks.py

[coderabbitai]

🎯 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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/test_dependencies.py (1)

193-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the same sdist root for BuildEnvironment and dependency resolution.

Both tests create BuildEnvironment with an empty temporary directory, but resolve dependencies from _fromager_root. This can hide regressions that depend on the build environment source tree.

Proposed fix

-    sdist_root = tmp_path / "fromager-1.0.0"
-    sdist_root.mkdir()
+    sdist_root = _fromager_root
...
-        sdist_root_dir=_fromager_root,
+        sdist_root_dir=sdist_root,

Apply the same change in both tests. As per path instructions, “Verify test actually tests the intended behavior.”

Also applies to: 257-269

🤖 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 193 - 205, Update both tests around
BuildEnvironment and get_build_system_dependencies to use the same sdist root
path for construction and dependency resolution. Replace the empty temporary
sdist_root directory passed to BuildEnvironment with the existing _fromager_root
value, while preserving the dependency-resolution arguments and verifying both
tests exercise the source-tree behavior.

Source: Path instructions

🤖 Prompt for all review comments with 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.

Outside diff comments:
In `@tests/test_dependencies.py`:
- Around line 193-205: Update both tests around BuildEnvironment and
get_build_system_dependencies to use the same sdist root path for construction
and dependency resolution. Replace the empty temporary sdist_root directory
passed to BuildEnvironment with the existing _fromager_root value, while
preserving the dependency-resolution arguments and verifying both tests exercise
the source-tree behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 856d72f8-574a-4c53-9def-0cff8df0c872

📥 Commits

Reviewing files that changed from the base of the PR and between 2ffbe2d and 3d7b57b.

📒 Files selected for processing (3)

• src/fromager/dependencies.py
• src/fromager/packagesettings/_settings.py
• tests/test_dependencies.py