Skip to content

[proposal] New patcher config#940

Open
tiran wants to merge 1 commit intopython-wheel-build:mainfrom
tiran:proposal-new-patcher-config
Open

[proposal] New patcher config#940
tiran wants to merge 1 commit intopython-wheel-build:mainfrom
tiran:proposal-new-patcher-config

Conversation

@tiran
Copy link
Copy Markdown
Collaborator

@tiran tiran commented Feb 27, 2026

Add design proposal for new patcher configuration.

See: #939

Summary by CodeRabbit

  • Documentation
    • Added a new "Fromager Enhancement Proposals" section to the documentation structure
    • Included an initial proposal for a declarative patching configuration system

@mergify mergify bot added the ci label Feb 27, 2026
@tiran tiran force-pushed the proposal-new-patcher-config branch 3 times, most recently from ef44739 to 143622a Compare March 3, 2026 14:12
@tiran tiran force-pushed the proposal-new-patcher-config branch from 143622a to feeb9ab Compare March 17, 2026 13:59
@tiran tiran marked this pull request as ready for review March 18, 2026 12:47
@tiran tiran requested a review from a team as a code owner March 18, 2026 12:47
@tiran tiran changed the title WIP: [proposal] New patcher config [proposal] New patcher config Mar 18, 2026
@tiran
Copy link
Copy Markdown
Collaborator Author

tiran commented Mar 27, 2026

@Mergifyio rebase

Add design proposal for new patcher configuration.

See: python-wheel-build#939
Signed-off-by: Christian Heimes <cheimes@redhat.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 27, 2026

rebase

✅ Branch has been successfully rebased

@tiran tiran force-pushed the proposal-new-patcher-config branch from feeb9ab to 119bc67 Compare March 27, 2026 09:35
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

📝 Walkthrough

Walkthrough

Added documentation for a new declarative patching configuration proposal to the Sphinx docs. The addition includes a proposals index page and detailed proposal specification for a patch configuration system supporting multiple action types and version-scoped edits.

Changes

Cohort / File(s) Summary
Proposal Documentation Structure
docs/index.rst, docs/proposals/index.rst
Added new proposals section to documentation navigation with index page containing toctree reference to new patcher configuration proposal.
Patcher Configuration Proposal
docs/proposals/new-patcher-config.md
Introduced detailed proposal for declarative patching system featuring patch array with title, version constraints, action types (replace-line, delete-line, fix-pkg-info, etc.), execution semantics across build phases, error handling via ignore_missing, and deprecation of legacy project_override fields.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A patcher proposal hops into view,
With config that's tidy and ever so new,
From replace-line to pins, what a show!
Documentation grows where good ideas flow. 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[proposal] New patcher config' directly describes the main change—adding a new proposal document for a declarative patcher configuration system.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/proposals/new-patcher-config.md`:
- Around line 79-83: The document uses inconsistent action identifiers;
standardize on a single canonical name (use pin-install-requires-to-build)
everywhere: update the narrative occurrences and the YAML example action
identifier so both reference pin-install-requires-to-build (also ensure any
references in the example blocks or headings that currently use the alternate
identifier are replaced with pin-install-requires-to-build).
- Around line 9-10: Fix grammar and typos in the proposal text by editing
awkward phrases for clarity—e.g., change the sentence "This enhancement document
proposal a new approach to patch sources and wheel metadata through declarative
configuration." to "This enhancement document proposes a new approach to patch
sources and wheel metadata through declarative configuration." and apply similar
edits to the other flagged sentences (the phrases around lines referenced in the
review: the sentence starting "proposal a...", and the other problematic phrases
near the provided snippets for lines 32, 39, 76, 81, and 111) to correct verb
agreement, article usage, and awkward wording so the document reads clearly and
consistently.
- Around line 27-28: The document uses an impossible version range example
`>=1.0,<1.0` in the proposal (lines referencing the when_version example), which
matches no versions; update those occurrences to a satisfiable range such as
`>=1.0,<2.0` (or a tighter range like `>=1.0,<1.1`) so the when_version
semantics are clear and demonstrable—replace both instances at lines ~28 and ~55
(the `when_version` example) accordingly.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 00bd8bcc-c492-4a82-9e80-c6f63edd24a5

📥 Commits

Reviewing files that changed from the base of the PR and between 9b399c8 and 119bc67.

📒 Files selected for processing (3)
  • docs/index.rst
  • docs/proposals/index.rst
  • docs/proposals/new-patcher-config.md

Comment on lines +9 to +10
This enhancement document proposal a new approach to patch sources and wheel
metadata through declarative configuration. The new feature supplements
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clean up grammar/typos in proposal text.

Several phrases read incorrectly and should be corrected for clarity (Line 9, Line 32, Line 39, Line 76, Line 81, Line 111).

Proposed copy edits
-This enhancement document proposal a new approach to patch sources and wheel
+This enhancement document proposes a new approach to patch sources and wheel
 metadata through declarative configuration. The new feature supplements
 current features like patch files and plugin hooks.
...
-  design to pin Torch version to ensure `libtorch` ABI compatibility.
+  designed to pin Torch version to ensure `libtorch` ABI compatibility.
...
-- CPU architecture-specific and variant-specific patches won't be supported,
-  They are considered a misfeature. Patches should be architecture-
+- CPU architecture-specific and variant-specific patches won't be supported.
+  They are considered a misfeature. Patches should be architecture-
   independent, so they can be pushed to upstream eventually.
...
-- The `fix-pkg-info` addresses issues with sdist's `PKG-INFO` files, e.g.
+- The `fix-pkg-info` action addresses issues with sdist's `PKG-INFO` files, e.g.
   wrong metadata version.
...
-  primary use case is Torch. If a package is build with Torch 2.9.1, then
+  primary use case is Torch. If a package is built with Torch 2.9.1, then
   we want the wheel to depend on exactly `torch==2.9.1`.
...
-  - title: Pin Torch to global cosntraint
+  - title: Pin Torch to global constraint

Also applies to: 32-32, 39-40, 76-76, 81-82, 111-111

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/proposals/new-patcher-config.md` around lines 9 - 10, Fix grammar and
typos in the proposal text by editing awkward phrases for clarity—e.g., change
the sentence "This enhancement document proposal a new approach to patch sources
and wheel metadata through declarative configuration." to "This enhancement
document proposes a new approach to patch sources and wheel metadata through
declarative configuration." and apply similar edits to the other flagged
sentences (the phrases around lines referenced in the review: the sentence
starting "proposal a...", and the other problematic phrases near the provided
snippets for lines 32, 39, 76, 81, and 111) to correct verb agreement, article
usage, and awkward wording so the document reads clearly and consistently.

Comment on lines +27 to +28
- support version-specific patching (e.g. patch build system requirements for
`>=1.0,<1.0`).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix impossible version-range example.

Line 28 and Line 55 use >=1.0,<1.0, which matches no versions. Please replace with a satisfiable example so when_version semantics are unambiguous.

Proposed doc fix
-- support version-specific patching (e.g. patch build system requirements for
-  `>=1.0,<1.0`).
+- support version-specific patching (e.g. patch build system requirements for
+  `>=1.0,<2.0`).
...
-- The optional field `when_version` can be used to limit the action,
-   e.g. `>=1.0,<1.0`.
-   > **NOTE**: `when_version` is not compatible with nightly builds.
+- The optional field `when_version` can be used to limit the action,
+  e.g. `>=1.0,<2.0`.
+  > **NOTE**: `when_version` is incompatible with nightly builds.

Also applies to: 54-56

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/proposals/new-patcher-config.md` around lines 27 - 28, The document uses
an impossible version range example `>=1.0,<1.0` in the proposal (lines
referencing the when_version example), which matches no versions; update those
occurrences to a satisfiable range such as `>=1.0,<2.0` (or a tighter range like
`>=1.0,<1.1`) so the when_version semantics are clear and demonstrable—replace
both instances at lines ~28 and ~55 (the `when_version` example) accordingly.

Comment on lines +79 to +83
- The `pin-install-requires-to-build` pins `requires-dist` in sdist
and wheel metadata to the exact version in the build environment. The
primary use case is Torch. If a package is build with Torch 2.9.1, then
we want the wheel to depend on exactly `torch==2.9.1`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Align action name across narrative and YAML example.

Line 79 and Line 112 describe different action identifiers. Please keep one canonical name throughout the document to avoid spec ambiguity.

Also applies to: 111-113

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/proposals/new-patcher-config.md` around lines 79 - 83, The document uses
inconsistent action identifiers; standardize on a single canonical name (use
pin-install-requires-to-build) everywhere: update the narrative occurrences and
the YAML example action identifier so both reference
pin-install-requires-to-build (also ensure any references in the example blocks
or headings that currently use the alternate identifier are replaced with
pin-install-requires-to-build).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants