Skip to content

Show application of file and data patches#23110

Merged
Bo98 merged 1 commit into
mainfrom
embedded-patch-ohai
Jul 14, 2026
Merged

Show application of file and data patches#23110
Bo98 merged 1 commit into
mainfrom
embedded-patch-ohai

Conversation

@Bo98

@Bo98 Bo98 commented Jul 14, 2026

Copy link
Copy Markdown
Member

When applying a remote patch we print Applying something.patch. We didn't for file & DATA patches. When installing a formula like libssh2, this made you think it was applying less patches than it actually was.

This PR fixes the inconsistency.


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Copilot AI review requested due to automatic review settings July 14, 2026 17:57

@MikeMcQuaid MikeMcQuaid left a comment

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.

Thanks!

Copilot AI 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.

Pull request overview

This PR makes patch application output consistent by printing Applying ... for embedded patches (local file and DATA patches), matching the messaging already shown for remote/external patches.

Changes:

  • Add an EmbeddedPatch#filename API and print ohai "Applying #{filename}" when applying embedded patches.
  • Implement filename for LocalPatch (local patch files) and DATAPatch (__END__/DATA patches).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Library/Homebrew/embedded_patch.rb Adds a filename abstraction for embedded patches and prints Applying ... during apply.
Library/Homebrew/local_patch.rb Implements filename for local file patches to support the new Applying ... output.
Library/Homebrew/data_patch.rb Implements filename for DATA/__END__ patches to support the new Applying ... output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/embedded_patch.rb
Comment thread Library/Homebrew/local_patch.rb Outdated
@Bo98 Bo98 force-pushed the embedded-patch-ohai branch from fab4696 to 4de3af1 Compare July 14, 2026 18:01
@MikeMcQuaid

Copy link
Copy Markdown
Member

[x] Have you successfully run brew lgtm (style, typechecking and tests) locally?

@Bo98 is your brew typecheck broken?

@Bo98

Bo98 commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

No I just forgot to actually run git commit --amend when fixing it so was still in my working tree 😅

@Bo98 Bo98 force-pushed the embedded-patch-ohai branch from 4de3af1 to f5b06ea Compare July 14, 2026 18:08

@MikeMcQuaid MikeMcQuaid left a comment

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.

Optional and not crazy about naming but differentiation might be nice

Comment thread Library/Homebrew/string_patch.rb Outdated
Comment thread Library/Homebrew/data_patch.rb Outdated
@Bo98 Bo98 force-pushed the embedded-patch-ohai branch from f5b06ea to e500d91 Compare July 14, 2026 18:20
@Bo98 Bo98 added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit 38acf7e Jul 14, 2026
65 of 66 checks passed
@Bo98 Bo98 deleted the embedded-patch-ohai branch July 14, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants