-
Notifications
You must be signed in to change notification settings - Fork 53
docs(proposals): global hook for build-system dependency post-processing #1272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,234 @@ | ||
| # Build-system dependency post-processing for cross-cutting concerns | ||
|
|
||
| - Author: Vikash Shaw | ||
| - Created: 2026-07-24 | ||
| - Status: Proposed | ||
| - GitHub issue: [#1263](https://github.com/python-wheel-build/fromager/issues/1263) | ||
|
|
||
| ## What | ||
|
|
||
| This proposal describes two approaches for handling cross-cutting | ||
| build-system dependency concerns (such as setuptools version capping) | ||
| that affect many packages. Both aim to eliminate the need for identical | ||
| per-package plugins in downstream projects. | ||
|
|
||
| ## Why | ||
|
|
||
| Fromager currently provides two extension mechanisms: | ||
|
|
||
| 1. **Per-package plugins** (`fromager.project_overrides`): Override a | ||
| hook for a single package. When present, the plugin replaces the | ||
| default implementation entirely. | ||
|
|
||
| 2. **Global hooks** (`fromager.hooks`): Run for every package. Currently | ||
| support `post_build`, `post_bootstrap`, and `prebuilt_wheel`, which | ||
| are event callbacks that fire after an action has completed. | ||
|
|
||
| Currently, no global hook runs during dependency resolution. When a | ||
| cross-cutting concern affects build dependencies for many packages, the | ||
| only option today is to write identical per-package plugins for each one. | ||
|
|
||
| ### Motivating example | ||
|
|
||
| setuptools 81 removed `distutils.spawn(dry_run=...)` and | ||
| `remove_tree(dry_run=...)`. setuptools 82 removed `pkg_resources` | ||
| entirely. Many PyPI packages still reference these removed APIs in their | ||
| `setup.py`, causing build failures when Fromager resolves an uncapped | ||
| setuptools. | ||
|
|
||
| In one downstream project, this led to **22 identical per-package | ||
| plugins**, each scanning `setup.py` to detect removed API usage and | ||
| appending a setuptools version cap. Every time a new package hits the | ||
| same incompatibility, another identical plugin must be added. This does | ||
| not scale well. | ||
|
|
||
| ### Why not `update_build_requires`? | ||
|
|
||
| Fromager's YAML settings support `update_build_requires` for statically | ||
| adding build dependencies. However, the setuptools cap is conditional | ||
| and depends on what APIs a given `setup.py` actually uses. A static YAML | ||
| entry would either over-constrain all packages or require per-package | ||
| entries, which has the same maintenance burden as plugins. | ||
|
|
||
| ## Proposed approaches | ||
|
|
||
| ### Option A: Core logic in Fromager (PR [#1264](https://github.com/python-wheel-build/fromager/pull/1264)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we already rejected option A right? Can we just document why we rejected it? |
||
|
|
||
| Add the setuptools detection logic directly into | ||
| `default_get_build_system_dependencies`. Fromager would automatically | ||
| scan `setup.py` for `pkg_resources` imports and `dry_run` keyword | ||
| arguments, and append the appropriate setuptools version cap to the | ||
| build dependencies. | ||
|
|
||
| **Changes in Fromager:** | ||
|
|
||
| - Add `_get_setuptools_constraint()` to `dependencies.py` using | ||
| `ast.parse` to detect removed API usage | ||
| - Call it from `default_get_build_system_dependencies()` and append | ||
| the constraint if needed | ||
|
|
||
| **Changes in downstream projects:** | ||
|
|
||
| - None. Delete the 22 identical plugins and their entry points. The | ||
| packages build correctly without any downstream configuration, | ||
| plugin, or hook registration. | ||
|
|
||
| ### Option B: Global hooks (PR [#1271](https://github.com/python-wheel-build/fromager/pull/1271)) | ||
|
|
||
| Add `get_build_system_dependencies` as a new global hook point under | ||
| `fromager.hooks`. Downstream projects register a hook that post-processes | ||
| the build-system dependencies list for all packages. Fromager stays | ||
| generic and does not include any setuptools-specific logic. | ||
|
|
||
| **Changes in Fromager:** | ||
|
|
||
| - Add `get_build_system_dependencies` to `GLOBAL_HOOK_NAMES` in | ||
| `hooks.py` | ||
| - Add `run_get_build_system_dependencies_hooks()` that chains hooks | ||
| so each receives the previous hook's output | ||
| - Call the hooks in `dependencies.get_build_system_dependencies()` | ||
| after per-package overrides return, before marker filtering | ||
|
|
||
| **Changes in downstream projects:** | ||
|
|
||
| - Add a hook implementation (one file with the setuptools detection | ||
| logic) | ||
| - Register it as an entry point under `fromager.hooks` | ||
| - Delete the 22 identical per-package plugins and their entry points | ||
| - The packages still need to be listed in the downstream project's | ||
| requirements/collections | ||
|
|
||
| Alternatively, the hook could be released as a standalone installable | ||
| package (e.g. `fromager-setuptools-hook`) so any Fromager user can | ||
| opt in by simply installing it, without writing any hook code themselves. | ||
|
|
||
| ## Comparison | ||
|
|
||
| | | Option A (core) | Option B (global hooks) | | ||
| | -- | -- | -- | | ||
| | Downstream work needed | None | Hook registration or pip install | | ||
| | Fromager stays generic | No, includes setuptools-specific logic | Yes | | ||
| | Other Fromager users benefit | Automatically | Only if they install the hook | | ||
| | Reusable for other concerns | No, only solves setuptools | Yes, hook point is general-purpose | | ||
| | Configurable / opt-out | Would need a config flag | Opt-in by design | | ||
|
|
||
| ## How (Option A details) | ||
|
|
||
| ### Execution order | ||
|
|
||
| The constraint logic runs inside | ||
| `default_get_build_system_dependencies()`, after reading `[build-system] requires` from `pyproject.toml`: | ||
|
|
||
| ``` | ||
| 1. Check for cached requirements file (early return if exists) | ||
| 2. overrides.find_and_invoke() <-- per-package plugin or default | ||
| 2a. default reads [build-system] requires from pyproject.toml | ||
| 2b. _get_setuptools_constraint() scans setup.py <-- NEW | ||
| 2c. Append constraint if needed | ||
| 3. _filter_requirements() <-- marker evaluation | ||
| 4. Write requirements cache file | ||
| ``` | ||
|
|
||
| ### Detection logic | ||
|
|
||
| `_get_setuptools_constraint(sdist_root_dir)` uses `ast.parse` to walk | ||
| the `setup.py` AST and detect: | ||
|
|
||
| - `import pkg_resources` or `from pkg_resources import ...` results in | ||
| `setuptools<82` | ||
| - `dry_run` keyword argument results in `setuptools<81` (tighter cap | ||
| takes precedence when both are present) | ||
| - Returns `None` if neither is found, or if `setup.py` does not exist | ||
| - Gracefully returns `None` on `SyntaxError` | ||
|
|
||
| ### Example | ||
|
|
||
| For a package with this `setup.py`: | ||
|
|
||
| ```python | ||
| from pkg_resources import get_distribution | ||
| from setuptools import setup | ||
| setup(name="example", version="1.0") | ||
| ``` | ||
|
|
||
| Fromager would automatically append `setuptools<82` to its build | ||
| dependencies. No downstream configuration needed. | ||
|
|
||
| ## How (Option B details) | ||
|
|
||
| ### Execution order | ||
|
|
||
| The hook would run inside `dependencies.get_build_system_dependencies()`, | ||
| after the per-package override (or default) returns and before marker | ||
| filtering: | ||
|
|
||
| ``` | ||
| 1. Check for cached requirements file (early return if exists) | ||
| 2. overrides.find_and_invoke() <-- per-package plugin or default | ||
| 3. hooks.run_get_build_system_dependencies_hooks() <-- NEW | ||
| 4. _filter_requirements() <-- marker evaluation | ||
| 5. Write requirements cache file | ||
| ``` | ||
|
|
||
| This means per-package plugins still produce the initial dependency | ||
| list, global hooks can then augment it, and marker filtering happens | ||
| last so hooks do not need to handle markers themselves. The result is | ||
| cached, so hooks run only once per package per build. | ||
|
Comment on lines
+124
to
+134
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Define cache invalidation for global hook changes. The resolver returns Document a required cache clear, or include the active hook configuration/version in the cache identity. As per path instructions, this comment addresses a concrete dependency-resolution contract issue. Also applies to: 191-192 🤖 Prompt for AI AgentsSource: Path instructions |
||
|
|
||
| ### Hook signature | ||
|
|
||
| ```python | ||
| def get_build_system_dependencies( | ||
| *, | ||
| ctx: context.WorkContext, | ||
| req: Requirement, | ||
| sdist_root_dir: pathlib.Path, | ||
| build_dir: pathlib.Path, | ||
| requirements: list[str], | ||
| ) -> list[str]: | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ... | ||
| ``` | ||
|
|
||
| The hook receives the current requirements list and must return a | ||
| (possibly modified) `list[str]`. When multiple hooks are registered, | ||
| they chain: each receives the previous hook's output. Execution order | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If one hook in the chain raises an exception, does the entire chain fail? Does the build fail? Are subsequent hooks skipped? The existing hooks don't need to answer this because they're independent but chained hooks do. |
||
| follows stevedore's `HookManager` iteration order. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HookManager doesn't guarantee order — it depends on entry point discovery, which varies across installations and Python versions. For side-effect hooks this is fine. For chained hooks where hook_A(hook_B(deps)) may differ from hook_B(hook_A(deps)), it's a problem. How should users control or reason about order? |
||
|
|
||
| ### Registration | ||
|
|
||
| Hooks are registered as entry points under the `fromager.hooks` | ||
| namespace, the same way `post_build` and other existing hooks work: | ||
|
|
||
| ```toml | ||
| [project.entry-points."fromager.hooks"] | ||
| get_build_system_dependencies = "my_package.hooks:get_build_system_dependencies" | ||
| ``` | ||
|
|
||
| ### Example hook | ||
|
|
||
| A minimal hook that appends a constraint: | ||
|
|
||
| ```python | ||
| def get_build_system_dependencies( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Today's global hooks (post_build, post_bootstrap, prebuilt_wheel) are fire-and-forget — they return nothing, they don't chain, and execution order doesn't matter. The proposed get_build_system_dependencies hook fundamentally changes this: hooks receive the previous hook's output and return a modified list. |
||
| *, | ||
| ctx: context.WorkContext, | ||
| req: Requirement, | ||
| sdist_root_dir: pathlib.Path, | ||
| build_dir: pathlib.Path, | ||
| requirements: list[str], | ||
| ) -> list[str]: | ||
| # Inspect sdist content and conditionally add constraints | ||
| if needs_constraint(build_dir): | ||
| return requirements + ["setuptools<82"] | ||
| return requirements | ||
| ``` | ||
|
|
||
| ## Interaction with existing mechanisms | ||
|
|
||
| | Mechanism | Scope | Relationship | | ||
| | -- | -- | -- | | ||
| | `update_build_requires` (YAML) | Per-package, static | Runs during `prepare_source`, before this hook. | | ||
| | `remove_build_requires` (YAML) | Per-package, static | Same as above. | | ||
| | Per-package plugin | Per-package, dynamic | Runs first. Global hooks receive its output. | | ||
| | Cached `build-system-requirements.txt` | Per-package | If cache exists, function returns early. Hooks do not run. | | ||
| | **Global hooks (Option B)** | All packages, dynamic | Runs after per-package plugin, before marker filtering. | | ||
There was a problem hiding this comment.
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
🧩 Analysis chain
🏁 Script executed:
Repository: python-wheel-build/fromager
Length of output: 2918
Clarify what Setuptools 81 removed.
Setuptools 81 removed support for the
setup.py --dry-runoption and changed some related class/function signatures. It does not document removal ofdistutils.spawn(dry_run=...)orremove_tree(dry_run=...).Suggested wording
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Path instructions