diff --git a/internal/brew/brew.go b/internal/brew/brew.go index b89c55a..28e897f 100644 --- a/internal/brew/brew.go +++ b/internal/brew/brew.go @@ -18,6 +18,12 @@ import ( const maxWorkers = 1 +// Test seams for install behavior. Production uses the real implementations. +var ( + checkNetworkFunc = CheckNetwork + sleepFunc = time.Sleep +) + type OutdatedPackage struct { Name string Current string @@ -222,12 +228,17 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm return nil, nil, fmt.Errorf("list installed packages: %w", checkErr) } + // Batch-resolve formula aliases in a single brew info call. + // Casks don't have an alias system, so we skip resolution for them. + aliasMap := ResolveFormulaNames(cliPkgs) + var newCli []string for _, p := range cliPkgs { - if !alreadyFormulae[p] { + resolvedName := aliasMap[p] + if !alreadyFormulae[resolvedName] { newCli = append(newCli, p) } else { - installedFormulae = append(installedFormulae, p) + installedFormulae = append(installedFormulae, resolvedName) } } var newCask []string @@ -268,7 +279,7 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm } for _, p := range newCli { if !failedSet[p] { - installedFormulae = append(installedFormulae, p) + installedFormulae = append(installedFormulae, aliasMap[p]) } } allFailed = append(allFailed, failed...) @@ -313,28 +324,31 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm if f.isCask { installedCasks = append(installedCasks, f.name) } else { - installedFormulae = append(installedFormulae, f.name) + installedFormulae = append(installedFormulae, aliasMap[f.name]) } } else { fmt.Printf(" ✗ %s (still failed)\n", f.name) } } - type pkgKey struct { - name string - isCask bool - } - succeeded := make(map[pkgKey]bool) + succeededFormulae := make(map[string]bool, len(installedFormulae)) for _, p := range installedFormulae { - succeeded[pkgKey{p, false}] = true + succeededFormulae[p] = true } + succeededCasks := make(map[string]bool, len(installedCasks)) for _, p := range installedCasks { - succeeded[pkgKey{p, true}] = true + succeededCasks[p] = true } var stillFailed []failedJob for _, f := range allFailed { - if !succeeded[pkgKey{f.name, f.isCask}] { - stillFailed = append(stillFailed, f) + if f.isCask { + if !succeededCasks[f.name] { + stillFailed = append(stillFailed, f) + } + } else { + if !succeededFormulae[aliasMap[f.name]] { + stillFailed = append(stillFailed, f) + } } } allFailed = stillFailed @@ -503,7 +517,7 @@ func installFormulaWithError(pkg string) string { errMsg := parseBrewError(outputStr) if attempt < maxAttempts && isRetryableError(errMsg) { delay := time.Duration(attempt) * 2 * time.Second - time.Sleep(delay) + sleepFunc(delay) continue } @@ -553,7 +567,7 @@ func installSmartCaskWithError(pkg string) string { if attempt < maxAttempts && isRetryableError(errMsg) { delay := time.Duration(attempt) * 2 * time.Second - time.Sleep(delay) + sleepFunc(delay) continue } @@ -598,6 +612,53 @@ func parseBrewError(output string) string { } } +// ResolveFormulaNames resolves formula aliases to their canonical names in a +// single batched `brew info --json` call. It returns a map from each input name +// to its canonical name. On any error it falls back to an identity mapping. +func ResolveFormulaNames(names []string) map[string]string { + if len(names) == 0 { + return make(map[string]string) + } + + args := append([]string{"info", "--json"}, names...) + cmd := exec.Command("brew", args...) + output, err := cmd.Output() + if err != nil { + return identityMap(names) + } + + return parseFormulaAliases(names, output) +} + +// parseFormulaAliases builds an alias map from the JSON response of +// `brew info --json`. The JSON array is positionally aligned with names. +func parseFormulaAliases(names []string, jsonData []byte) map[string]string { + var entries []struct { + Name string `json:"name"` + } + if err := json.Unmarshal(jsonData, &entries); err != nil { + return identityMap(names) + } + + resolved := make(map[string]string, len(names)) + for i, n := range names { + if i < len(entries) && entries[i].Name != "" { + resolved[n] = entries[i].Name + } else { + resolved[n] = n + } + } + return resolved +} + +func identityMap(names []string) map[string]string { + m := make(map[string]string, len(names)) + for _, n := range names { + m[n] = n + } + return m +} + func Uninstall(packages []string, dryRun bool) error { if len(packages) == 0 { return nil @@ -828,7 +889,7 @@ func DoctorDiagnose() ([]string, error) { func PreInstallChecks(packageCount int) error { ui.Info("Checking network connectivity...") - if err := CheckNetwork(); err != nil { + if err := checkNetworkFunc(); err != nil { return fmt.Errorf("network check failed: %v\nPlease check your internet connection and try again", err) } diff --git a/internal/brew/install_progress_behavior_test.go b/internal/brew/install_progress_behavior_test.go new file mode 100644 index 0000000..cee4f00 --- /dev/null +++ b/internal/brew/install_progress_behavior_test.go @@ -0,0 +1,182 @@ +package brew + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func captureOutput(t *testing.T, fn func()) string { + t.Helper() + + oldStdout := os.Stdout + oldStderr := os.Stderr + + r, w, err := os.Pipe() + require.NoError(t, err) + + os.Stdout = w + os.Stderr = w + defer func() { + os.Stdout = oldStdout + os.Stderr = oldStderr + }() + + done := make(chan string, 1) + go func() { + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + done <- buf.String() + }() + + fn() + + require.NoError(t, w.Close()) + + return <-done +} + +func TestInstallWithProgress_BatchesAliasResolutionAndSkipsCaskAliases(t *testing.T) { + tmpDir := t.TempDir() + logPath := filepath.Join(tmpDir, "brew-calls.log") + + setupFakeBrew(t, fmt.Sprintf(`#!/bin/sh +log_file=%q +printf '%%s\n' "$*" >> "$log_file" + +if [ "$1" = "list" ] && [ "$2" = "--formula" ]; then + exit 0 +fi +if [ "$1" = "list" ] && [ "$2" = "--cask" ]; then + exit 0 +fi +if [ "$1" = "info" ]; then + shift 2 + for arg in "$@"; do + if [ "$arg" = "firefox" ]; then + echo "unexpected cask alias resolution" >&2 + exit 1 + fi + done + cat <<'EOF' +[{"name":"postgresql@16"},{"name":"kubernetes-cli"}] +EOF + exit 0 +fi +if [ "$1" = "update" ]; then + exit 0 +fi +if [ "$1" = "install" ] && [ "$2" = "--cask" ]; then + exit 0 +fi +if [ "$1" = "install" ]; then + exit 0 +fi +exit 0 +`, logPath)) + + originalCheckNetwork := checkNetworkFunc + checkNetworkFunc = func() error { return nil } + t.Cleanup(func() { checkNetworkFunc = originalCheckNetwork }) + + formulae, casks, err := InstallWithProgress([]string{"postgresql", "kubectl"}, []string{"firefox"}, false) + require.NoError(t, err) + assert.Equal(t, []string{"postgresql@16", "kubernetes-cli"}, formulae) + assert.Equal(t, []string{"firefox"}, casks) + + logContent, err := os.ReadFile(logPath) + require.NoError(t, err) + + var infoLines []string + for _, line := range strings.Split(strings.TrimSpace(string(logContent)), "\n") { + if strings.HasPrefix(line, "info --json") { + infoLines = append(infoLines, line) + } + } + + require.Len(t, infoLines, 1) + assert.Equal(t, "info --json postgresql kubectl", infoLines[0]) + assert.NotContains(t, infoLines[0], "firefox") +} + +func TestInstallWithProgress_RetrySuccessTracksCanonicalNames(t *testing.T) { + tmpDir := t.TempDir() + logPath := filepath.Join(tmpDir, "brew-calls.log") + statePath := filepath.Join(tmpDir, "foo-attempts") + + setupFakeBrew(t, fmt.Sprintf(`#!/bin/sh +log_file=%q +state_file=%q +printf '%%s\n' "$*" >> "$log_file" + +if [ "$1" = "list" ] && [ "$2" = "--formula" ]; then + exit 0 +fi +if [ "$1" = "list" ] && [ "$2" = "--cask" ]; then + exit 0 +fi +if [ "$1" = "info" ]; then + cat <<'EOF' +[{"name":"foo-canonical"}] +EOF + exit 0 +fi +if [ "$1" = "update" ]; then + exit 0 +fi +if [ "$1" = "install" ] && [ "$2" = "foo" ]; then + attempt=0 + if [ -f "$state_file" ]; then + attempt=$(cat "$state_file") + fi + attempt=$((attempt + 1)) + echo "$attempt" > "$state_file" + if [ "$attempt" -eq 1 ]; then + echo "Error: Connection timed out while downloading" + exit 1 + fi + exit 0 +fi +if [ "$1" = "install" ] && [ "$2" = "--cask" ]; then + exit 0 +fi +if [ "$1" = "install" ]; then + exit 0 +fi +exit 0 +`, logPath, statePath)) + + originalCheckNetwork := checkNetworkFunc + originalSleep := sleepFunc + checkNetworkFunc = func() error { return nil } + sleepFunc = func(time.Duration) {} + t.Cleanup(func() { + checkNetworkFunc = originalCheckNetwork + sleepFunc = originalSleep + }) + + var formulae, casks []string + var err error + output := captureOutput(t, func() { + formulae, casks, err = InstallWithProgress([]string{"foo"}, nil, false) + }) + + require.NoError(t, err) + assert.Equal(t, []string{"foo-canonical"}, formulae) + assert.Empty(t, casks) + assert.NotContains(t, output, "packages failed to install") + assert.Contains(t, output, "✔ foo") + + logContent, err := os.ReadFile(logPath) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(logContent), "info --json foo")) + assert.Equal(t, 2, strings.Count(string(logContent), "install foo")) +} diff --git a/internal/brew/resolve_test.go b/internal/brew/resolve_test.go new file mode 100644 index 0000000..2e7ac18 --- /dev/null +++ b/internal/brew/resolve_test.go @@ -0,0 +1,82 @@ +package brew + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveFormulaNames_EmptyInput(t *testing.T) { + assert.Empty(t, ResolveFormulaNames(nil)) + assert.Empty(t, ResolveFormulaNames([]string{})) +} + +func TestParseFormulaAliases(t *testing.T) { + tests := []struct { + name string + input []string + json interface{} + expected map[string]string + }{ + { + name: "resolves aliases to canonical names", + input: []string{"postgresql", "kubectl", "vim"}, + json: []struct { + Name string `json:"name"` + }{ + {Name: "postgresql@16"}, + {Name: "kubernetes-cli"}, + {Name: "vim"}, + }, + expected: map[string]string{ + "postgresql": "postgresql@16", + "kubectl": "kubernetes-cli", + "vim": "vim", + }, + }, + { + name: "falls back to identity when response is shorter than input", + input: []string{"foo", "bar", "baz"}, + json: []struct { + Name string `json:"name"` + }{ + {Name: "foo-canonical"}, + }, + expected: map[string]string{ + "foo": "foo-canonical", + "bar": "bar", + "baz": "baz", + }, + }, + { + name: "falls back to identity for empty name fields", + input: []string{"foo", "bar"}, + json: []struct { + Name string `json:"name"` + }{ + {Name: "foo-resolved"}, + {Name: ""}, + }, + expected: map[string]string{ + "foo": "foo-resolved", + "bar": "bar", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + data, err := json.Marshal(tc.json) + require.NoError(t, err) + assert.Equal(t, tc.expected, parseFormulaAliases(tc.input, data)) + }) + } +} + +func TestParseFormulaAliases_InvalidJSON(t *testing.T) { + names := []string{"pkg1", "pkg2"} + result := parseFormulaAliases(names, []byte("not json")) + assert.Equal(t, map[string]string{"pkg1": "pkg1", "pkg2": "pkg2"}, result) +} diff --git a/internal/diff/compare.go b/internal/diff/compare.go index 57c292f..c7f414c 100644 --- a/internal/diff/compare.go +++ b/internal/diff/compare.go @@ -171,6 +171,12 @@ func diffDotfiles(systemURL, referenceURL string) *DotfilesDiff { dd.RepoChanged = &ValueChange{System: systemURL, Reference: referenceURL} } + // Only check local dotfiles repo state if dotfiles are actually configured + // If both URLs are empty, there's no dotfiles setup to check + if sysNorm == "" && refNorm == "" { + return dd + } + // Check local dotfiles repo for dirty state home, err := os.UserHomeDir() if err != nil {