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

@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

@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 Sphinx documentation pages: registered docs/proposals/index.rst in docs/index.rst's Development toctree, created docs/proposals/index.rst, and added docs/proposals/new-patcher-config.md describing a declarative patching configuration format, execution semantics, actions, and deprecations.

Changes

Cohort / File(s) Summary
Docs TOC update
docs/index.rst
Added proposals/index.rst to the Development toctree so proposals are included in site navigation.
Proposals index
docs/proposals/index.rst
New Sphinx index page "Fromager Enhancement Proposals" with a toctree linking proposal children (new-patcher-config).
Patcher proposal
docs/proposals/new-patcher-config.md
New proposal describing a top-level patch array of operations (title, optional when_version, action discriminator like replace-line, delete-line, fix-pkg-info, pyproject-build-system, pin-requires-dist-to-constraint), file path resolution, phase-specific execution semantics, ignore_missing behavior, deprecations of certain project_override fields, and examples.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • EmilienM
  • LalatenduMohanty

Poem

🐰 I hopped to the docs with a bright little plan,

A patch-list in YAML, neat as I can,
Titles and actions, versions that bind,
Proposals now live for the curious mind,
Hooray for clear paths — a rabbit-approved find! 🥕

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title '[proposal] New patcher config' directly and clearly identifies the main change: adding a design proposal for new patcher configuration, which matches the changeset.
Description check ✅ Passed The description 'Add design proposal for new patcher configuration. See: #939' is directly related to the changeset, which adds proposal documentation.

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


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 +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).

- The `fix-pkg-info` addresses issues with sdist's `PKG-INFO` files, e.g.
wrong metadata version.

- The `pin-install-requires-to-build` pins `requires-dist` in sdist
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The proposal also talks about action: pin-requires-dist-to-constraint . The proposal mixes them together as if they're the same thing. Can you clarify?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! My first idea was pin-install-requires-to-build, but it's harder to implement and the additional complexity is not necessary. pin-requires-dist-to-constraint is easier to implement and solves the problem.

@tiran tiran force-pushed the proposal-new-patcher-config branch from 119bc67 to 652927e Compare April 7, 2026 09:14
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.

♻️ Duplicate comments (2)
docs/proposals/new-patcher-config.md (2)

38-40: ⚠️ Potential issue | 🟡 Minor

Fix sentence punctuation in non-goals bullet.

Line 39 uses a comma before a new sentence (..., They are ...). Split into two sentences for correctness.

Proposed doc fix
 - CPU architecture-specific and variant-specific patches won't be supported,
-  They are considered a misfeature. Patches should be architecture-
+  they are considered a misfeature. Patches should be architecture-
   independent, so they can be pushed to upstream eventually.
🤖 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 38 - 40, The non-goals
bullet contains a comma splice ("..., They are ..."); edit the sentence in the
non-goals bullet so it becomes two sentences by replacing the comma after
"supported" with a period and capitalizing "They" (i.e., "CPU
architecture-specific and variant-specific patches won't be supported. They are
considered a misfeature."). Ensure the rest of the bullet remains unchanged.

27-28: ⚠️ Potential issue | 🟠 Major

Replace unsatisfiable version range example.

Line 28 still uses >=1.0,<1.0, which matches no versions and conflicts with the valid example later in Line 55. Please make both examples satisfiable and consistent (e.g., >=1.0,<2.0).

Proposed doc fix
 - support version-specific patching (e.g. patch build system requirements for
-  `>=1.0,<1.0`).
+  `>=1.0,<2.0`).
🤖 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, Replace the
unsatisfiable version range string `>=1.0,<1.0` with a satisfiable, consistent
range such as `>=1.0,<2.0` in the examples; update the occurrence of
`>=1.0,<1.0` to match the later valid example (`>=1.0,<2.0`) so both examples
are consistent and satisfiable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/proposals/new-patcher-config.md`:
- Around line 38-40: The non-goals bullet contains a comma splice ("..., They
are ..."); edit the sentence in the non-goals bullet so it becomes two sentences
by replacing the comma after "supported" with a period and capitalizing "They"
(i.e., "CPU architecture-specific and variant-specific patches won't be
supported. They are considered a misfeature."). Ensure the rest of the bullet
remains unchanged.
- Around line 27-28: Replace the unsatisfiable version range string `>=1.0,<1.0`
with a satisfiable, consistent range such as `>=1.0,<2.0` in the examples;
update the occurrence of `>=1.0,<1.0` to match the later valid example
(`>=1.0,<2.0`) so both examples are consistent and satisfiable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8abe6fc3-cc57-4556-8440-3f5c61010242

📥 Commits

Reviewing files that changed from the base of the PR and between 119bc67 and 652927e.

📒 Files selected for processing (3)
  • docs/index.rst
  • docs/proposals/index.rst
  • docs/proposals/new-patcher-config.md
✅ Files skipped from review due to trivial changes (2)
  • docs/proposals/index.rst
  • docs/index.rst

Add design proposal for new patcher configuration.

See: python-wheel-build#939
Signed-off-by: Christian Heimes <cheimes@redhat.com>
@tiran tiran force-pushed the proposal-new-patcher-config branch from 652927e to 4c3c7ca Compare April 7, 2026 13:14
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.

♻️ Duplicate comments (1)
docs/proposals/new-patcher-config.md (1)

27-28: ⚠️ Potential issue | 🟠 Major

Fix unsatisfiable version range example.

>=1.0,<1.0 matches no versions, so this example is factually incorrect for when_version behavior.

Proposed 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`).

As per coding guidelines, "**/*.md: Only comment on factual errors or broken links. Skip all formatting and style suggestions."

🤖 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 example
`>=1.0,<1.0` is unsatisfiable; update the `when_version` example to a valid
range that demonstrates version-specific patching (e.g., replace the literal
`">=1.0,<1.0"` with a satisfiable range such as `">=1.0,<2.0"` or a more
specific range like `">=1.0,<1.1"`), and ensure any explanatory text mentions
`when_version` so readers know this demonstrates patching for that version
interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/proposals/new-patcher-config.md`:
- Around line 27-28: The example `>=1.0,<1.0` is unsatisfiable; update the
`when_version` example to a valid range that demonstrates version-specific
patching (e.g., replace the literal `">=1.0,<1.0"` with a satisfiable range such
as `">=1.0,<2.0"` or a more specific range like `">=1.0,<1.1"`), and ensure any
explanatory text mentions `when_version` so readers know this demonstrates
patching for that version interval.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e46f3145-aa77-4bfe-b8e3-4fbe2528705c

📥 Commits

Reviewing files that changed from the base of the PR and between 652927e and 4c3c7ca.

📒 Files selected for processing (3)
  • docs/index.rst
  • docs/proposals/index.rst
  • docs/proposals/new-patcher-config.md
✅ Files skipped from review due to trivial changes (2)
  • docs/index.rst
  • docs/proposals/index.rst

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.

3 participants