Skip to content

bundle: add cleanup support to npm, cargo, go, and uv extensions#21883

Open
mvanhorn wants to merge 2 commits intoHomebrew:mainfrom
mvanhorn:osc/bundle-extension-cleanup
Open

bundle: add cleanup support to npm, cargo, go, and uv extensions#21883
mvanhorn wants to merge 2 commits intoHomebrew:mainfrom
mvanhorn:osc/bundle-extension-cleanup

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

@mvanhorn mvanhorn commented Apr 1, 2026

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Claude (Anthropic) was used to help write the cleanup methods and tests. All code was reviewed for correctness, tested locally with brew lgtm --online, and verified by running brew bundle cleanup against a real Brewfile on my machine.

brew bundle cleanup handles formulae, casks, taps, VSCode extensions, and Flatpak packages not listed in the Brewfile. But it silently ignores npm, cargo, go, and uv packages.

The Extension base class already defines cleanup_heading, cleanup_items, and cleanup! hooks. Flatpak and VSCode Extension override them. The other 4 extensions don't, so their packages are invisible to cleanup.

This PR adds the same 3 overrides to npm, cargo, go, and uv, following the flatpak pattern:

Extension cleanup! command
npm npm uninstall -g NAME
cargo cargo uninstall NAME (with CARGO_HOME/RUSTUP_HOME env)
go Removes binary from GOBIN/GOPATH after matching via go version -m
uv uv tool uninstall NAME

Tested locally. With 6 go packages installed and a Brewfile listing only one, cleanup now reports the other 5:

cleanup output

`brew bundle cleanup` already handles formulae, casks, taps, VSCode
extensions, and Flatpak packages. But npm, cargo, go, and uv
packages not listed in the Brewfile are silently ignored.

Add `cleanup_heading`, `cleanup_items`, and `cleanup!` overrides
to all four extensions, following the existing flatpak/vscode_extension
pattern.
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.

Hey @mvanhorn! Thanks, looking good so far.

Main concern here is that folks are may be surprised when they brew bundle cleanup --force and it nukes all their global cargo/go/npm/uv files.

I'm not sure how best to handle this. Any thoughts?

@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented Apr 1, 2026

Good question. A few thoughts:

  1. The existing cleanup dry-run already shows what would be removed before --force is used, so there's a built-in safety net. This matches how formulae/casks work today.

  2. Flatpak and VSCode extensions already follow this same pattern -- cleanup lists them, --force removes them. npm/cargo/go/uv are consistent with that.

  3. If you'd prefer an extra guardrail, one option would be a --skip-type flag (or env var like HOMEBREW_BUNDLE_CLEANUP_SKIP_TYPES) so users can opt specific extension types out of cleanup. Happy to add that if you think it's worth it.

Open to other ideas too.

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 @mvanhorn! You've convinced me given --force that we should just leave this as-is and consider additional flags/env if people complain.

Rereviewed and needs DRYd up a bit then good to go!

Comment on lines +71 to +74
sig { override.returns(String) }
def cleanup_heading
"Cargo packages"
end
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.

Ideally we'd not need this at all given BANNER_NAME, maybe needs logic changes in Extension

Comment on lines +76 to +88
sig { params(entries: T::Array[Object]).returns(T::Array[String]) }
def cleanup_items(entries)
return [].freeze unless package_manager_installed?

kept_packages = entries.filter_map do |entry|
entry = T.cast(entry, Dsl::Entry)
entry.name if entry.type == type
end

return [].freeze if kept_packages.empty?

packages - kept_packages
end
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.

This is copy/pasted below so should be moved to the Extension class

Bundle.system(cargo.to_s, "uninstall", name, verbose: false)
end
end
puts "Uninstalled #{items.size} Cargo package#{"s" if items.size != 1}"
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.

Suggested change
puts "Uninstalled #{items.size} Cargo package#{"s" if items.size != 1}"
puts "Uninstalled #{items.size} #{BANNER_NAME}#{"s" if items.size != 1}"


sig { override.returns(String) }
def cleanup_heading
"Go packages"
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.

same here

end

sig { params(entries: T::Array[Object]).returns(T::Array[String]) }
def cleanup_items(entries)
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.

same here

items.each do |name|
Bundle.system(npm.to_s, "uninstall", "-g", name, verbose: false)
end
puts "Uninstalled #{items.size} npm package#{"s" if items.size != 1}"
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.

same here


sig { override.returns(String) }
def cleanup_heading
"uv tools"
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.

same here

end

sig { params(entries: T::Array[Object]).returns(T::Array[String]) }
def cleanup_items(entries)
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.

same here

items.each do |name|
Bundle.system(uv.to_s, "tool", "uninstall", name, verbose: false)
end
puts "Uninstalled #{items.size} uv tool#{"s" if items.size != 1}"
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.

same here

Comment on lines +237 to +243
def cleanup!(items)
uv = package_manager_executable
return if uv.nil?

items.each do |name|
Bundle.system(uv.to_s, "tool", "uninstall", name, verbose: false)
end
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.

This is identical for all but go so should be moved into Extension.

Move cleanup_heading, cleanup_items, and the common cleanup! pattern
into Extension. Each extension now only defines uninstall_package!
with its specific uninstall args. Go keeps its custom cleanup! since
it does binary-level deletion rather than a package manager call.
@mvanhorn
Copy link
Copy Markdown
Contributor Author

mvanhorn commented Apr 2, 2026

DRYed up in 4aa2387:

  • cleanup_heading now returns banner_name from the base class (removed 4 overrides)
  • cleanup_items moved to Extension base class (removed 4 identical implementations)
  • Added uninstall_package! hook -- each extension only defines its specific uninstall args
  • Go keeps its custom cleanup! since it does binary-level deletion
  • All puts lines use banner_name instead of hardcoded strings

Net: -68 lines across the 4 extensions.

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.

2 participants