Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 77 additions & 16 deletions internal/brew/brew.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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...)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
182 changes: 182 additions & 0 deletions internal/brew/install_progress_behavior_test.go
Original file line number Diff line number Diff line change
@@ -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"))
}
Loading
Loading