Skip to content

Commit 18eccb4

Browse files
jerryjrxieJerry Xieclaude
authored
fix: resolve formula aliases before tracking installed packages (#20)
* fix: resolve formula aliases before tracking installed packages Homebrew has formula aliases where the requested name differs from the actual installed name. Examples: - postgresql → installs as postgresql@18 - kubectl → installs as kubernetes-cli This caused packages to be reinstalled on every run because the state file tracked the alias name (e.g., postgresql) but brew list returns the actual name (e.g., postgresql@18), so they never matched during reconciliation. Fix: - Added ResolveFormulaName() function to resolve aliases using brew info --json - Updated all package tracking to use resolved names - Handles both CLI packages and casks Closes #17 (formula alias tracking issue) * fix: isolate diff tests from local dotfiles repo state * fix: address review feedback on formula alias resolution - Batch brew info --json calls into single invocation with cached map - Fix retry path to use resolved alias names - Skip alias resolution for casks (no alias system) - Add unit tests for parseFormulaAliases with table-driven cases - Extract identityMap helper to deduplicate fallback logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: cover alias install progress behavior * test: fix retry success expectation --------- Co-authored-by: Jerry Xie <jerryxie@Jerrys-MacBook-Air.local> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent da8b1b5 commit 18eccb4

3 files changed

Lines changed: 341 additions & 16 deletions

File tree

internal/brew/brew.go

Lines changed: 77 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ import (
1818

1919
const maxWorkers = 1
2020

21+
// Test seams for install behavior. Production uses the real implementations.
22+
var (
23+
checkNetworkFunc = CheckNetwork
24+
sleepFunc = time.Sleep
25+
)
26+
2127
type OutdatedPackage struct {
2228
Name string
2329
Current string
@@ -222,12 +228,17 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
222228
return nil, nil, fmt.Errorf("list installed packages: %w", checkErr)
223229
}
224230

231+
// Batch-resolve formula aliases in a single brew info call.
232+
// Casks don't have an alias system, so we skip resolution for them.
233+
aliasMap := ResolveFormulaNames(cliPkgs)
234+
225235
var newCli []string
226236
for _, p := range cliPkgs {
227-
if !alreadyFormulae[p] {
237+
resolvedName := aliasMap[p]
238+
if !alreadyFormulae[resolvedName] {
228239
newCli = append(newCli, p)
229240
} else {
230-
installedFormulae = append(installedFormulae, p)
241+
installedFormulae = append(installedFormulae, resolvedName)
231242
}
232243
}
233244
var newCask []string
@@ -268,7 +279,7 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
268279
}
269280
for _, p := range newCli {
270281
if !failedSet[p] {
271-
installedFormulae = append(installedFormulae, p)
282+
installedFormulae = append(installedFormulae, aliasMap[p])
272283
}
273284
}
274285
allFailed = append(allFailed, failed...)
@@ -313,28 +324,31 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
313324
if f.isCask {
314325
installedCasks = append(installedCasks, f.name)
315326
} else {
316-
installedFormulae = append(installedFormulae, f.name)
327+
installedFormulae = append(installedFormulae, aliasMap[f.name])
317328
}
318329
} else {
319330
fmt.Printf(" ✗ %s (still failed)\n", f.name)
320331
}
321332
}
322333

323-
type pkgKey struct {
324-
name string
325-
isCask bool
326-
}
327-
succeeded := make(map[pkgKey]bool)
334+
succeededFormulae := make(map[string]bool, len(installedFormulae))
328335
for _, p := range installedFormulae {
329-
succeeded[pkgKey{p, false}] = true
336+
succeededFormulae[p] = true
330337
}
338+
succeededCasks := make(map[string]bool, len(installedCasks))
331339
for _, p := range installedCasks {
332-
succeeded[pkgKey{p, true}] = true
340+
succeededCasks[p] = true
333341
}
334342
var stillFailed []failedJob
335343
for _, f := range allFailed {
336-
if !succeeded[pkgKey{f.name, f.isCask}] {
337-
stillFailed = append(stillFailed, f)
344+
if f.isCask {
345+
if !succeededCasks[f.name] {
346+
stillFailed = append(stillFailed, f)
347+
}
348+
} else {
349+
if !succeededFormulae[aliasMap[f.name]] {
350+
stillFailed = append(stillFailed, f)
351+
}
338352
}
339353
}
340354
allFailed = stillFailed
@@ -503,7 +517,7 @@ func installFormulaWithError(pkg string) string {
503517
errMsg := parseBrewError(outputStr)
504518
if attempt < maxAttempts && isRetryableError(errMsg) {
505519
delay := time.Duration(attempt) * 2 * time.Second
506-
time.Sleep(delay)
520+
sleepFunc(delay)
507521
continue
508522
}
509523

@@ -553,7 +567,7 @@ func installSmartCaskWithError(pkg string) string {
553567

554568
if attempt < maxAttempts && isRetryableError(errMsg) {
555569
delay := time.Duration(attempt) * 2 * time.Second
556-
time.Sleep(delay)
570+
sleepFunc(delay)
557571
continue
558572
}
559573

@@ -598,6 +612,53 @@ func parseBrewError(output string) string {
598612
}
599613
}
600614

615+
// ResolveFormulaNames resolves formula aliases to their canonical names in a
616+
// single batched `brew info --json` call. It returns a map from each input name
617+
// to its canonical name. On any error it falls back to an identity mapping.
618+
func ResolveFormulaNames(names []string) map[string]string {
619+
if len(names) == 0 {
620+
return make(map[string]string)
621+
}
622+
623+
args := append([]string{"info", "--json"}, names...)
624+
cmd := exec.Command("brew", args...)
625+
output, err := cmd.Output()
626+
if err != nil {
627+
return identityMap(names)
628+
}
629+
630+
return parseFormulaAliases(names, output)
631+
}
632+
633+
// parseFormulaAliases builds an alias map from the JSON response of
634+
// `brew info --json`. The JSON array is positionally aligned with names.
635+
func parseFormulaAliases(names []string, jsonData []byte) map[string]string {
636+
var entries []struct {
637+
Name string `json:"name"`
638+
}
639+
if err := json.Unmarshal(jsonData, &entries); err != nil {
640+
return identityMap(names)
641+
}
642+
643+
resolved := make(map[string]string, len(names))
644+
for i, n := range names {
645+
if i < len(entries) && entries[i].Name != "" {
646+
resolved[n] = entries[i].Name
647+
} else {
648+
resolved[n] = n
649+
}
650+
}
651+
return resolved
652+
}
653+
654+
func identityMap(names []string) map[string]string {
655+
m := make(map[string]string, len(names))
656+
for _, n := range names {
657+
m[n] = n
658+
}
659+
return m
660+
}
661+
601662
func Uninstall(packages []string, dryRun bool) error {
602663
if len(packages) == 0 {
603664
return nil
@@ -828,7 +889,7 @@ func DoctorDiagnose() ([]string, error) {
828889

829890
func PreInstallChecks(packageCount int) error {
830891
ui.Info("Checking network connectivity...")
831-
if err := CheckNetwork(); err != nil {
892+
if err := checkNetworkFunc(); err != nil {
832893
return fmt.Errorf("network check failed: %v\nPlease check your internet connection and try again", err)
833894
}
834895

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
package brew
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"io"
7+
"os"
8+
"path/filepath"
9+
"strings"
10+
"testing"
11+
"time"
12+
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func captureOutput(t *testing.T, fn func()) string {
18+
t.Helper()
19+
20+
oldStdout := os.Stdout
21+
oldStderr := os.Stderr
22+
23+
r, w, err := os.Pipe()
24+
require.NoError(t, err)
25+
26+
os.Stdout = w
27+
os.Stderr = w
28+
defer func() {
29+
os.Stdout = oldStdout
30+
os.Stderr = oldStderr
31+
}()
32+
33+
done := make(chan string, 1)
34+
go func() {
35+
var buf bytes.Buffer
36+
_, _ = io.Copy(&buf, r)
37+
done <- buf.String()
38+
}()
39+
40+
fn()
41+
42+
require.NoError(t, w.Close())
43+
44+
return <-done
45+
}
46+
47+
func TestInstallWithProgress_BatchesAliasResolutionAndSkipsCaskAliases(t *testing.T) {
48+
tmpDir := t.TempDir()
49+
logPath := filepath.Join(tmpDir, "brew-calls.log")
50+
51+
setupFakeBrew(t, fmt.Sprintf(`#!/bin/sh
52+
log_file=%q
53+
printf '%%s\n' "$*" >> "$log_file"
54+
55+
if [ "$1" = "list" ] && [ "$2" = "--formula" ]; then
56+
exit 0
57+
fi
58+
if [ "$1" = "list" ] && [ "$2" = "--cask" ]; then
59+
exit 0
60+
fi
61+
if [ "$1" = "info" ]; then
62+
shift 2
63+
for arg in "$@"; do
64+
if [ "$arg" = "firefox" ]; then
65+
echo "unexpected cask alias resolution" >&2
66+
exit 1
67+
fi
68+
done
69+
cat <<'EOF'
70+
[{"name":"postgresql@16"},{"name":"kubernetes-cli"}]
71+
EOF
72+
exit 0
73+
fi
74+
if [ "$1" = "update" ]; then
75+
exit 0
76+
fi
77+
if [ "$1" = "install" ] && [ "$2" = "--cask" ]; then
78+
exit 0
79+
fi
80+
if [ "$1" = "install" ]; then
81+
exit 0
82+
fi
83+
exit 0
84+
`, logPath))
85+
86+
originalCheckNetwork := checkNetworkFunc
87+
checkNetworkFunc = func() error { return nil }
88+
t.Cleanup(func() { checkNetworkFunc = originalCheckNetwork })
89+
90+
formulae, casks, err := InstallWithProgress([]string{"postgresql", "kubectl"}, []string{"firefox"}, false)
91+
require.NoError(t, err)
92+
assert.Equal(t, []string{"postgresql@16", "kubernetes-cli"}, formulae)
93+
assert.Equal(t, []string{"firefox"}, casks)
94+
95+
logContent, err := os.ReadFile(logPath)
96+
require.NoError(t, err)
97+
98+
var infoLines []string
99+
for _, line := range strings.Split(strings.TrimSpace(string(logContent)), "\n") {
100+
if strings.HasPrefix(line, "info --json") {
101+
infoLines = append(infoLines, line)
102+
}
103+
}
104+
105+
require.Len(t, infoLines, 1)
106+
assert.Equal(t, "info --json postgresql kubectl", infoLines[0])
107+
assert.NotContains(t, infoLines[0], "firefox")
108+
}
109+
110+
func TestInstallWithProgress_RetrySuccessTracksCanonicalNames(t *testing.T) {
111+
tmpDir := t.TempDir()
112+
logPath := filepath.Join(tmpDir, "brew-calls.log")
113+
statePath := filepath.Join(tmpDir, "foo-attempts")
114+
115+
setupFakeBrew(t, fmt.Sprintf(`#!/bin/sh
116+
log_file=%q
117+
state_file=%q
118+
printf '%%s\n' "$*" >> "$log_file"
119+
120+
if [ "$1" = "list" ] && [ "$2" = "--formula" ]; then
121+
exit 0
122+
fi
123+
if [ "$1" = "list" ] && [ "$2" = "--cask" ]; then
124+
exit 0
125+
fi
126+
if [ "$1" = "info" ]; then
127+
cat <<'EOF'
128+
[{"name":"foo-canonical"}]
129+
EOF
130+
exit 0
131+
fi
132+
if [ "$1" = "update" ]; then
133+
exit 0
134+
fi
135+
if [ "$1" = "install" ] && [ "$2" = "foo" ]; then
136+
attempt=0
137+
if [ -f "$state_file" ]; then
138+
attempt=$(cat "$state_file")
139+
fi
140+
attempt=$((attempt + 1))
141+
echo "$attempt" > "$state_file"
142+
if [ "$attempt" -eq 1 ]; then
143+
echo "Error: Connection timed out while downloading"
144+
exit 1
145+
fi
146+
exit 0
147+
fi
148+
if [ "$1" = "install" ] && [ "$2" = "--cask" ]; then
149+
exit 0
150+
fi
151+
if [ "$1" = "install" ]; then
152+
exit 0
153+
fi
154+
exit 0
155+
`, logPath, statePath))
156+
157+
originalCheckNetwork := checkNetworkFunc
158+
originalSleep := sleepFunc
159+
checkNetworkFunc = func() error { return nil }
160+
sleepFunc = func(time.Duration) {}
161+
t.Cleanup(func() {
162+
checkNetworkFunc = originalCheckNetwork
163+
sleepFunc = originalSleep
164+
})
165+
166+
var formulae, casks []string
167+
var err error
168+
output := captureOutput(t, func() {
169+
formulae, casks, err = InstallWithProgress([]string{"foo"}, nil, false)
170+
})
171+
172+
require.NoError(t, err)
173+
assert.Equal(t, []string{"foo-canonical"}, formulae)
174+
assert.Empty(t, casks)
175+
assert.NotContains(t, output, "packages failed to install")
176+
assert.Contains(t, output, "✔ foo")
177+
178+
logContent, err := os.ReadFile(logPath)
179+
require.NoError(t, err)
180+
assert.Equal(t, 1, strings.Count(string(logContent), "info --json foo"))
181+
assert.Equal(t, 2, strings.Count(string(logContent), "install foo"))
182+
}

0 commit comments

Comments
 (0)