Skip to content

bun patch --help: add --commit to the custom patches directory example - #45417

Open
robobun wants to merge 4 commits into
mainfrom
robobun/e27b5148/patch-help-example-commit
Open

robobun wants to merge 4 commits into
mainfrom
robobun/e27b5148/patch-help-example-commit

Conversation

@robobun

@robobun robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The third example of bun patch --help is bun patch --patches-dir 'my-patches' 'node_modules/jquery'. Its description is "Generate a patch file in a custom directory for changes made to jquery". Run as printed on an edited folder, it exits 0, writes no patch file, and the edit is gone.
  • The example lacks --commit. Without --commit, bun patch puts a fresh copy of the package in the folder and never reads --patches-dir (src/install/PackageManager/CommandLineArguments.rs:1626).

Fix

  • The example now reads bun patch --commit --patches-dir 'my-patches' 'node_modules/jquery' (CommandLineArguments.rs:989). completions/bun-cli.json holds a copy and gets the same edit.
  • Correct because the flag's help line says "(only if --commit is used)". docs/pm/cli/patch.mdx:61 already shows the flag with --commit.
  • Verified: test/cli/install/bun-patch.test.ts runs both "Generate a patch file" examples as printed and compares the patch text. The custom directory example fails on the unfixed build. The file passes with the fix.

Background

  • bun patch <pkg> prepares a package for editing: it replaces node_modules/<pkg> with a fresh copy. bun patch --commit <folder> writes the patch file and records it in patchedDependencies.
  • misctools/generate-cli-completions.ts generates completions/bun-cli.json from the --help output.
  • Considered an error for --patches-dir without --commit, and a warning before bun patch replaces an edited folder. Both change the command and need a maintainer decision.

Downsides

  • A user who types --patches-dir without --commit still loses the edit, with exit 0 and no message.
  • The help string is 9 bytes longer. The diff changes no code.
Notes

Reproduction, on 1.4.3-canary.1 (367d939) and on debug builds of main (116dd64, d7562f2 and d0831bc):

  1. Install a project with the dependency jquery.
  2. Run bun patch jquery.
  3. Edit node_modules/jquery/index.js.
  4. Run bun patch --patches-dir 'my-patches' 'node_modules/jquery'.

Result: exit 0, no my-patches directory, no patchedDependencies in package.json, and index.js has its original content.

The test uses "jquery": "npm:no-deps@1.0.0" so that the examples run unchanged against the local test registry.

Test run with the fix, debug build with ASAN on Linux x64, main merged in at d0831bc: 220 pass, 5 skip (Windows only), 0 fail.

The test first checked only that the patch file contains the added line. After review it compares the whole patch text.

git grep my-patches finds one more example, in bun patch-commit --help. That example is correct: bun patch-commit always commits.

A full regeneration of completions/bun-cli.json with a debug build of 116dd64 changes 4,399 lines. This PR changes only the example line. The generator emits the same line for the new help text.

The example has had this form since #20117.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-patch.test.ts

The example under "Generate a patch file in a custom directory for
changes made to jquery" lacked --commit. Without --commit, bun patch
prepares the folder again and does not read --patches-dir. Run as
printed, the example wrote no patch file and replaced the edited folder
with a fresh copy of the package.

completions/bun-cli.json holds a copy of the example and gets the same
edit.
@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for a maintainer. The diff is green. CI is red only on macOS lanes, for reasons outside this diff.

Reproduction, on 1.4.3-canary.1 (367d939) and on debug builds of main (116dd64, d7562f2, d0831bc):

  1. Install a project with the dependency jquery.
  2. Run bun patch jquery.
  3. Edit node_modules/jquery/index.js.
  4. Run the example as bun patch --help prints it: bun patch --patches-dir 'my-patches' 'node_modules/jquery'.

The command exits 0, writes no patch file, and the edit is gone. With --commit in the command, it writes my-patches/<name>@<version>.patch and keeps the edit.

CI on 49b7dcb (build 128027):

  • test/cli/install/bun-patch.test.ts passed on every lane that ran tests: the seven Linux lanes, Windows x64, Windows aarch64, and macOS x64 (214 pass, 0 fail).
  • macOS aarch64 ran no tests. One shard timed out on the download of the build artifact. The other shard timed out in bun install for test/package.json on all four attempts. The same test file passed on macOS aarch64 in build 127062 (214 pass, 0 fail), on 598533c. The three files of this diff are identical in both commits.
  • macOS x64: test/js/bun/module-graph/module-graph-isolation.test.ts failed on all four attempts. This diff does not touch that code. On main at the merged commit the same test needed two retries to pass (build 127964).
  • The binary-size job did not start. EC2 returned CapacityLimitExceeded.

No further push is planned for these failures.

@robobun

robobun commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 3:00 AM PT - Oct 18th, 2026

❌ @robobun, your commit 49b7dcb has 4 failures in Build #128027 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 45417

That installs a local version of the PR into your bun-45417 executable, so you can run:

bun-45417 --bun

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4ad1b05c-b05f-42bd-9e10-64c0c92eb7ef
📥 Commits

Reviewing files that changed from the base of the PR and between 116dd64 and 793a27e.

📒 Files selected for processing: 3
  • completions/bun-cli.json
  • src/install/PackageManager/CommandLineArguments.rs
  • test/cli/install/bun-patch.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The bun patch help examples now include --commit. Tests execute the examples for default and custom patch directories and verify the generated patch files and package changes.

Changes

bun patch help examples

Layer / File(s) Summary
Help examples and execution tests
src/install/PackageManager/CommandLineArguments.rs, completions/bun-cli.json, test/cli/install/bun-patch.test.ts
Both help examples include --commit. Tests run the preparation and commit commands for the default patches directory and custom my-patches directory, then verify the patch path, manifest entry, patch content, and edited index.js.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 793a2

The help example for a custom patches directory now matches the command's behavior, so users who copy it will get a patch file. No merge risk remains.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the help example change and the addition of the --commit option.
Description check Passed The description explains what the PR changes and how the change was verified. It uses different headings from the template but provides the required information in the Problem, Fix, and Verified secti…
Full details: Description check

Explanation

The description explains what the PR changes and how the change was verified. It uses different headings from the template but provides the required information in the Problem, Fix, and Verified sections.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread test/cli/install/bun-patch.test.ts Outdated
Comment thread src/install/PackageManager/CommandLineArguments.rs
The block "the examples in 'bun patch --help'" checked only that the
patch file contains the added line. It now compares the whole file, for
the default directory and for the custom directory.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — a one-token help-text fix mirrored into the generated completions, and the follow-up push addresses the earlier note by asserting the exact patch contents.

What was reviewed:

  • The bun patch --help example and completions/bun-cli.json now carry the identical --commit --patches-dir 'my-patches' line; the bun patch-commit --patches-dir example is unchanged and correct since that subcommand always commits.
  • The new test drives both "Generate a patch file" examples straight from the printed help output, so the my-patches case fails on the old text; existsSync is not load-bearing because the patch file is also read and matched against an inline snapshot.
  • installedProject, bunOk, and patched are existing helpers in the same file, so the test inherits the local-registry setup and stdout/stderr/exit assertions.
Extended reasoning...

The diff changes one example line in the bun patch --help string in src/install/PackageManager/CommandLineArguments.rs, the same line in the generated completions/bun-cli.json, and adds a concurrent test.each block in test/cli/install/bun-patch.test.ts that runs the printed examples against the local test registry. No code paths change and no security-sensitive surface is touched. The bug hunt ran dry, no CODEOWNERS entry covers the changed files, and the only prior inline feedback (a toContain assertion on the patch file) was replaced by an exact inline snapshot in the latest push, so nothing remains outstanding.

robobun added a commit that referenced this pull request Oct 20, 2026
`bun patch --patches-dir <dir> <pkg>` took the flag, ignored it, and ran
the prepare step. The prepare step replaces the folder of the package,
so an edit that was not committed was gone, with exit code 0.

The flag is now an error when `--commit` is absent.
`CommandLineArguments::parse` prints `--patches-dir only applies to bun
patch --commit` and exits 1 before it reads the project. `bun patch
--commit --patches-dir` and `bun patch-commit --patches-dir` do not
change.

The custom directory example of `bun patch --help` gets `--commit`, and
`completions/bun-cli.json` gets the same edit. Both lines are the same
as in #45417.

This branch has not been deployed

No deployments
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