fix: stop and remove daemon on netbird CLI formula uninstall - #7
fix: stop and remove daemon on netbird CLI formula uninstall#7CoderSufiyan wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe Homebrew formula now cleans up the NetBird LaunchDaemon during uninstallation and displays instructions for stopping and uninstalling the service before upgrading or removing the formula. ChangesNetBird service cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@netbird.rb`:
- Around line 54-59: Replace the unsupported uninstall_post hook with a
supported Homebrew uninstall mechanism so the LaunchDaemon cleanup runs during
brew uninstall. Preserve the existing launchctl bootout and netbird.plist
removal behavior, including ignored errors, and use the formula’s supported
uninstall stanza or lifecycle hook.
- Around line 55-56: Update the uninstall_post cleanup in the netbird formula to
avoid unauthorised system daemon operations during a normal brew uninstall:
either remove the launchctl bootout and plist deletion and rely on the caveat’s
sudo netbird service uninstall, or replace them with an explicitly authorised
uninstall path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| def uninstall_post | ||
| system "launchctl", "bootout", "system/netbird" | ||
| rm_f "/Library/LaunchDaemons/netbird.plist" | ||
| rescue | ||
| # ignore errors if launchctl is unavailable or service is already stopped | ||
| end |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
brew_repo="$(brew --repository)"
grep -R -nE '\buninstall_post\b' "$brew_repo/Library/Homebrew" || true
grep -R -nE 'def uninstall\b|keg\.uninstall' "$brew_repo/Library/Homebrew" | head -50Repository: netbirdio/homebrew-tap
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the formula file and inspect the relevant section.
wc -l netbird.rb
cat -n netbird.rb | sed -n '1,140p'Repository: netbirdio/homebrew-tap
Length of output: 3207
🌐 Web query:
Homebrew Formula Cookbook uninstall_post formula hook supported lifecycle uninstall callback
💡 Result:
There is no "uninstall_post" formula hook in the Homebrew Formula lifecycle [1][2]. The term "uninstall_post" often appears in unrelated contexts, such as the CumulusCI tool, which uses it as a task name in its own configuration files [3][4][5]. In the context of Homebrew: 1. Formulae do not support custom Ruby hooks for uninstall actions (e.g., pre_uninstall or post_uninstall) [1][6]. 2. Homebrew's philosophy is that packages should be contained within their prefix (the Cellar) [6]. When a formula is uninstalled, Homebrew removes the files within that specific keg, and generally expects that no state or files are left behind outside the Cellar that would require a custom uninstall hook [6][7]. 3. While Homebrew Casks do support certain lifecycle hooks (such as uninstall_preflight or uninstall_postflight) to handle complex installations or removals, these are distinct from Formulae [8]. If you are encountering references to "uninstall_post" in a project, it is almost certainly a custom task or configuration specific to that application's internal automation, not a native feature of the Homebrew package manager [3][4][9].
Citations:
- 1: Formula
{pre,post}_uninstallhooks Homebrew/legacy-homebrew#33329 - 2: https://docs.brew.sh/rubydoc/Formula.html
- 3: https://github.com/SFDO-Tooling/CumulusCI/blob/671a0e88cef79e9aeefe1e2b835816cd8141bdbb/cumulusci/cumulusci.yml
- 4: https://pypi.org/project/cumulusci/3.29.0/
- 5: https://cumulusci.readthedocs.io/en/stable/flows.html
- 6: Homebrew should support uninstall blocks to be run to clean up files that aren't automatically removed by "brew uninstall" Homebrew/legacy-homebrew#1881
- 7: https://github.com/Homebrew/brew/blob/cbc2b248/Library/Homebrew/uninstall.rb
- 8: https://docs.brew.sh/rubydoc/file.json_api_postinstall_preflight_postflight_plan.html
- 9: https://cumulusci.readthedocs.io/en/stable/history.html
uninstall_post is not a supported Homebrew formula hook. brew uninstall won’t call this method, so the LaunchDaemon cleanup never runs. Move this to a supported uninstall path or document a separate cleanup step.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbird.rb` around lines 54 - 59, Replace the unsupported uninstall_post hook
with a supported Homebrew uninstall mechanism so the LaunchDaemon cleanup runs
during brew uninstall. Preserve the existing launchctl bootout and netbird.plist
removal behavior, including ignored errors, and use the formula’s supported
uninstall stanza or lifecycle hook.
| system "launchctl", "bootout", "system/netbird" | ||
| rm_f "/Library/LaunchDaemons/netbird.plist" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== netbird.rb ==\n'
git ls-files | rg '^netbird\.rb$|/netbird\.rb$|^Formula/.*netbird.*\.rb$' || true
for f in $(git ls-files | rg '^netbird\.rb$|/netbird\.rb$|^Formula/.*netbird.*\.rb$' || true); do
printf '\n--- %s (line-numbered excerpt) ---\n' "$f"
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
done
printf '\n== Search for launchctl/launchdaemon references ==\n'
rg -n --hidden --no-ignore-vcs 'launchctl|LaunchDaemons|netbird\.plist|bootout|sudo' .Repository: netbirdio/homebrew-tap
Length of output: 3869
Use a sudo-capable path for system daemon cleanup. uninstall_post runs as a normal brew uninstall, so launchctl bootout system/netbird and rm_f "/Library/LaunchDaemons/netbird.plist" will usually fail without privileges. Either drop this hook and rely on the caveat’s sudo netbird service uninstall, or run the uninstall under authorization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@netbird.rb` around lines 55 - 56, Update the uninstall_post cleanup in the
netbird formula to avoid unauthorised system daemon operations during a normal
brew uninstall: either remove the launchctl bootout and plist deletion and rely
on the caveat’s sudo netbird service uninstall, or replace them with an
explicitly authorised uninstall path.
1 participant
Problem
brew uninstall netbirdleaves behind the running daemon and/Library/LaunchDaemons/netbird.plistbecause the formula has no cleanup hook.Solution
Add
uninstall_postto bootout thenetbirdlaunchd service and remove the plist. Also addcaveatsreminding users to runnetbird service uninstallbefore upgrading.Fixes netbirdio/netbird#5852
Validation
ruby -c netbird.rbSummary by CodeRabbit
New Features
Documentation