Skip to content

bundle: write to tab through the wrapper#21410

Merged
p-linnane merged 1 commit intomainfrom
bundle-tab
Jan 16, 2026
Merged

bundle: write to tab through the wrapper#21410
p-linnane merged 1 commit intomainfrom
bundle-tab

Conversation

@Bo98
Copy link
Copy Markdown
Member

@Bo98 Bo98 commented Jan 16, 2026

Some external brew wrappers treat brew bundle with lower permissions due to its execution of user code. In such scenarios, writing to the Homebrew prefix might fail. Running through brew tab will avoid these issues as it will go through the brew wrapper again, and more closely mimic how the rest of brew bundle works (e.g. issuing brew install commands).

This will be a little slower but will generally only be a one-time operation anyway.

Copilot AI review requested due to automatic review settings January 16, 2026 03:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request changes the mark_as_installed_on_request! method to use brew tab --installed-on-request command instead of directly writing to tab files. This approach ensures proper privilege escalation when using external brew wrappers that restrict brew bundle permissions due to user code execution.

Changes:

  • Refactored from individual tab file writes to a batched brew tab command invocation
  • Changed from .each iteration with immediate writes to .filter_map collecting formula names for batch processing
  • Added explanatory comment about wrapper privilege handling

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

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Could this instead go through the wrapper only if the write fails given it's a slowdown?

@Bo98
Copy link
Copy Markdown
Member Author

Bo98 commented Jan 16, 2026

Sure, I think Errno::EACCES is enough. Or at least it is for my case.

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Jan 16, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2026
@p-linnane p-linnane added this pull request to the merge queue Jan 16, 2026
Merged via the queue into main with commit cbdb39a Jan 16, 2026
53 of 61 checks passed
@p-linnane p-linnane deleted the bundle-tab branch January 16, 2026 16:42
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.

4 participants