Skip to content

Commit 230c29b

Browse files
koddssonclaude
andcommitted
Consistent cask installed checks across all code paths
Several code paths that determine whether a cask is installed were using different heuristics (raw `ls`, directory existence, `Caskroom.casks`), which could disagree with `Cask#installed?`. This consolidates them all to use `.metadata` directory presence as the source of truth. - Add `Cask::Caskroom.cask_installed?` utility method - Update `Caskroom.any_casks_installed?` to check `.metadata` - Update `list.sh` bash fast path to check `.metadata` - Update `cmd/list.rb` to use `Caskroom.casks` in all code paths - Update `cmd/update-report.rb` to delegate to `Caskroom.cask_installed?` - Add unit tests for consistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7ad35f8 commit 230c29b

File tree

7 files changed

+145
-22
lines changed

7 files changed

+145
-22
lines changed

Library/Homebrew/cask/caskroom.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,16 @@ def self.tokens
3131
paths.map { |path| path.basename.to_s }
3232
end
3333

34+
# Check if a cask with the given token is installed, consistent with {Cask#installed?}.
35+
# Checks for an installed caskfile inside `.metadata/<version>/<timestamp>/Casks/`.
36+
sig { params(token: String).returns(T::Boolean) }
37+
def self.cask_installed?(token)
38+
Pathname.glob(path/token/".metadata"/"*"/"*"/"Casks"/"*.{rb,json}").any?
39+
end
40+
3441
sig { returns(T::Boolean) }
3542
def self.any_casks_installed?
36-
paths.any?
43+
paths.any? { |p| Pathname.glob(p/".metadata"/"*"/"*"/"Casks"/"*.{rb,json}").any? }
3744
end
3845

3946
sig { void }

Library/Homebrew/cmd/list.rb

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,23 @@ def run
162162
system_command! "ls", args: [*ls_args, HOMEBREW_CELLAR], print_stdout: true
163163
puts if $stdout.tty? && !args.formula?
164164
end
165-
if !args.formula? && Cask::Caskroom.any_casks_installed?
166-
ohai "Casks" if $stdout.tty? && !args.cask?
167-
system_command! "ls", args: [*ls_args, Cask::Caskroom.path], print_stdout: true
165+
unless args.formula?
166+
casks = Cask::Caskroom.casks
167+
if args.t?
168+
casks.sort_by! { |c| c.install_time || Time.at(0) }.reverse!
169+
else
170+
casks.sort_by!(&:token)
171+
end
172+
casks.reverse! if args.r?
173+
cask_names = casks.map(&:token)
174+
if cask_names.present?
175+
ohai "Casks" if $stdout.tty? && !args.cask?
176+
if args.public_send(:"1?") || !$stdout.tty?
177+
puts cask_names
178+
else
179+
puts Formatter.columns(cask_names)
180+
end
181+
end
168182
end
169183
else
170184
kegs, casks = args.named.to_kegs_to_casks
@@ -215,15 +229,7 @@ def filtered_list
215229
sig { void }
216230
def list_casks
217231
casks = if args.no_named?
218-
cask_paths = Cask::Caskroom.path.children.reject(&:file?).map do |path|
219-
if path.symlink?
220-
real_path = path.realpath
221-
real_path.basename.to_s
222-
else
223-
path.basename.to_s
224-
end
225-
end.uniq.sort
226-
cask_paths.map { |name| Cask::CaskLoader.load(name) }
232+
Cask::Caskroom.casks
227233
else
228234
filtered_args = args.named.dup.delete_if do |n|
229235
Homebrew.failed = true unless Cask::Caskroom.path.join(n).exist?

Library/Homebrew/cmd/update-report.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ def installed?(formula)
938938

939939
sig { params(cask: String).returns(T::Boolean) }
940940
def cask_installed?(cask)
941-
(Cask::Caskroom.path/cask).directory?
941+
Cask::Caskroom.cask_installed?(cask)
942942
end
943943

944944
sig { returns(T::Array[T.untyped]) }

Library/Homebrew/list.sh

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,28 @@ homebrew-list() {
7171

7272
if [[ -z "${formula}" && -d "${HOMEBREW_CASKROOM}" ]]
7373
then
74-
if [[ -n "${tty}" && -z "${cask}" ]]
75-
then
76-
ohai "Casks"
77-
fi
74+
# Filter to only casks with installed metadata, consistent with
75+
# Cask::Caskroom.cask_installed? and Cask#installed?.
76+
local installed_cask_names=()
77+
for dir in "${HOMEBREW_CASKROOM}"/*/
78+
do
79+
[[ -d "${dir}.metadata" ]] && installed_cask_names+=("$(basename "${dir}")")
80+
done
7881

79-
local cask_output
80-
cask_output="$(/usr/bin/env "${ls_env[@]}" ls "${ls_args[@]}" "${HOMEBREW_CASKROOM}")" || exit 1
81-
if [[ -n "${cask_output}" ]]
82+
if ((${#installed_cask_names[@]} > 0))
8283
then
83-
echo "${cask_output}"
84+
if [[ -n "${tty}" && -z "${cask}" ]]
85+
then
86+
ohai "Casks"
87+
fi
88+
89+
local cask_output
90+
# Use ls -d on filtered names so formatting flags (-1/-l/-r/-t) are honoured
91+
cask_output="$(cd "${HOMEBREW_CASKROOM}" && /usr/bin/env "${ls_env[@]}" ls "${ls_args[@]}" -d -- "${installed_cask_names[@]}")" || exit 1
92+
if [[ -n "${cask_output}" ]]
93+
then
94+
echo "${cask_output}"
95+
fi
8496
fi
8597

8698
return 0
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# typed: false
2+
# frozen_string_literal: true
3+
4+
require "cask/caskroom"
5+
6+
RSpec.describe Cask::Caskroom do
7+
describe ".cask_installed?" do
8+
it "returns true when a cask has an installed caskfile" do
9+
Dir.mktmpdir do |dir|
10+
allow(described_class).to receive(:path).and_return(Pathname(dir))
11+
casks_dir = (Pathname(dir)/"test-cask"/".metadata"/"1.0"/"0"/"Casks")
12+
casks_dir.mkpath
13+
FileUtils.touch casks_dir/"test-cask.rb"
14+
15+
expect(described_class.cask_installed?("test-cask")).to be true
16+
end
17+
end
18+
19+
it "returns false when .metadata exists but has no caskfile" do
20+
Dir.mktmpdir do |dir|
21+
allow(described_class).to receive(:path).and_return(Pathname(dir))
22+
(Pathname(dir)/"test-cask"/".metadata"/"1.0"/"0"/"Casks").mkpath
23+
24+
expect(described_class.cask_installed?("test-cask")).to be false
25+
end
26+
end
27+
28+
it "returns false when a cask directory has no .metadata" do
29+
Dir.mktmpdir do |dir|
30+
allow(described_class).to receive(:path).and_return(Pathname(dir))
31+
(Pathname(dir)/"test-cask"/"1.0").mkpath
32+
33+
expect(described_class.cask_installed?("test-cask")).to be false
34+
end
35+
end
36+
37+
it "returns false when a cask directory does not exist" do
38+
Dir.mktmpdir do |dir|
39+
allow(described_class).to receive(:path).and_return(Pathname(dir))
40+
41+
expect(described_class.cask_installed?("nonexistent-cask")).to be false
42+
end
43+
end
44+
end
45+
46+
describe ".any_casks_installed?" do
47+
it "returns false when caskroom is empty" do
48+
Dir.mktmpdir do |dir|
49+
allow(described_class).to receive(:path).and_return(Pathname(dir))
50+
51+
expect(described_class.any_casks_installed?).to be false
52+
end
53+
end
54+
55+
it "returns false when only bare directories exist" do
56+
Dir.mktmpdir do |dir|
57+
allow(described_class).to receive(:path).and_return(Pathname(dir))
58+
(Pathname(dir)/"bare-cask"/"1.0").mkpath
59+
60+
expect(described_class.any_casks_installed?).to be false
61+
end
62+
end
63+
64+
it "returns true when at least one cask has an installed caskfile" do
65+
Dir.mktmpdir do |dir|
66+
allow(described_class).to receive(:path).and_return(Pathname(dir))
67+
(Pathname(dir)/"bare-cask"/"1.0").mkpath
68+
casks_dir = (Pathname(dir)/"installed-cask"/".metadata"/"1.0"/"0"/"Casks")
69+
casks_dir.mkpath
70+
FileUtils.touch casks_dir/"installed-cask.rb"
71+
72+
expect(described_class.any_casks_installed?).to be true
73+
end
74+
end
75+
end
76+
end

Library/Homebrew/test/cmd/list_spec.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,19 @@
2525
.to be_a_success
2626
.and not_to_output.to_stderr
2727
end
28+
29+
describe "cask listing" do
30+
it "lists only casks returned by Caskroom.casks" do
31+
installed_cask = instance_double(Cask::Cask, token: "installed-cask", install_time: Time.now)
32+
expect(Cask::Caskroom).to receive(:casks).and_return([installed_cask])
33+
expect(Cask::List).to receive(:list_casks).with(
34+
installed_cask,
35+
one: false,
36+
full_name: false,
37+
versions: true,
38+
)
39+
40+
described_class.new(["--cask", "--versions"]).run
41+
end
42+
end
2843
end

Library/Homebrew/test/cmd/update-report_spec.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,5 +280,12 @@ def perform_update(fixture_name = "")
280280
allow(Cask::Caskroom).to receive(:casks).and_return([])
281281
expect { hub.dump }.not_to output.to_stdout
282282
end
283+
284+
describe "#cask_installed?" do
285+
it "delegates to Cask::Caskroom.cask_installed?" do
286+
expect(Cask::Caskroom).to receive(:cask_installed?).with("test-cask").and_return(true)
287+
expect(hub.send(:cask_installed?, "test-cask")).to be true
288+
end
289+
end
283290
end
284291
end

0 commit comments

Comments
 (0)