Skip to content

list: fail clearly for uninstalled casks when piped - #24345

Merged
MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
aktanazat:list-uninstalled-cask-non-tty
Oct 24, 2026
Merged

MikeMcQuaid merged 1 commit into
Homebrew:mainfrom
aktanazat:list-uninstalled-cask-non-tty

Conversation

@aktanazat

Copy link
Copy Markdown
Contributor

  • 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 brew benchmark results.
  • 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?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Opus 5.5 found the cause, wrote the one-line fix and the spec, and ran the checks below. I reviewed the diff and the before/after output myself.


When stdout is not a TTY, or with --verbose, brew list --cask runs find on each named cask's Caskroom directory without checking that the cask is installed. For a cask that isn't installed you get a find failure instead of the normal error:

$ brew list --cask crossover | cat
find: /opt/homebrew/Caskroom/crossover: No such file or directory
Error: Failure while executing; `/usr/bin/env find /opt/homebrew/Caskroom/crossover -not -type d -not -name .DS_Store -print` exited with 1. Here's the output:
find: /opt/homebrew/Caskroom/crossover: No such file or directory

In a terminal the same command prints Error: Cask 'crossover' is not installed., and formulae already fail cleanly on both paths. This matters for scripts that pipe brew list output.

To reproduce, with a cask that isn't installed:

  1. brew list --cask crossover | cat
  2. brew list --cask --verbose crossover

This raises Cask::CaskNotInstalledError before running find, the same check Cask::List.list_casks does for the TTY path. Installed casks still list their files. After the change both commands print Error: Cask 'crossover' is not installed. and exit 1.

Locally brew lgtm passes. The new example in brew tests --only=cmd/list fails on main with the find error and passes with this change.

When stdout is not a TTY (or with `--verbose`), `brew list --cask NAME`
ran `find` on the Caskroom path even if the cask was not installed, so
it failed with a `find` execution error instead of the
"Cask 'NAME' is not installed." error the TTY path gives.

Raise `CaskNotInstalledError` before running `find`, as
`Cask::List.list_casks` does.
@coderabbitai

coderabbitai Bot commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fe01cb71-792c-489f-98a1-698c4e6285be
Commits

Reviewing files that changed from the base of the PR and between d4c23f2 and e753987.

Files selected for processing: 2
  • Library/Homebrew/cmd/list.rb
  • Library/Homebrew/test/cmd/list_spec.rb

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

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Listing a named cask that is not installed now reports a clear error instead of attempting to display its paths. This applies to verbose listings and listings where output is not directed to a terminal. Listings of installed casks and formula paths are unchanged.

In verbose or non-terminal named cask listings, list now checks whether each cask is installed and raises CaskNotInstalledError for the first uninstalled cask before listing paths. A test checks the error message and confirms that stdout is empty.

Changes: Named cask listing

Layer / File(s) Summary
Check cask installation before listing paths
Library/Homebrew/cmd/list.rb, Library/Homebrew/test/cmd/list_spec.rb
The command raises CaskNotInstalledError for the first uninstalled cask before listing cask paths. The test checks the error message and confirms that stdout is empty.

Priority: Low

Merge Risk: Minimal · up to e7539

No actionable merge-blocking risk is established.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly summarises the main change: brew list now reports uninstalled casks clearly when output is piped.
Description check Passed The description directly explains the bug, reproduction steps, expected behaviour, implementation, tests, and validation results.
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.
  • 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.

@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.

Good catch, thanks!

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Oct 24, 2026
Merged via the queue into Homebrew:main with commit 7c5b605 Oct 24, 2026
55 checks passed
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

⌂ 换网址