Skip to content

perf: use batch brew install with native parallelism #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jerryjrxie wants to merge 11 commits into
base: main
Choose a base branch
from
Open

perf: use batch brew install with native parallelism #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ed44eee
fix: check all git config scopes, not just --global
Mar 29, 2026
7db09da
test: add test for git config scope fallback
Mar 29, 2026
7d6fec4
fix: don't check dotfiles repo state when URLs are empty
Mar 29, 2026
d9360ae
fix: include GUI apps (casks) when installing from remote config
Mar 29, 2026
0f5c077
test: add regression test for cask inclusion in SelectedPkgs
Mar 29, 2026
e5ae816
perf: pipe brew output directly instead of capturing and parsing
Mar 29, 2026
f5271b2
test: add tests for batch install and formula name resolution
Mar 29, 2026
b176f99
fix: verify actual package installation status after batch install
Mar 29, 2026
1370c97
fix: address PR review feedback for batch install
jerryjrxie Apr 1, 2026
5b13328
fix: use io.ReadAll instead of strings.Builder.ReadFrom in test
jerryjrxie Apr 1, 2026
193cc5d
test: cover batch install classification
jerryjrxie Apr 1, 2026
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
282 changes: 151 additions & 131 deletions internal/brew/brew.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
"github.com/openbootdotdev/openboot/internal/ui"
)

const maxWorkers = 1

type OutdatedPackage struct {
Name string
Current string
Expand Down Expand Up @@ -200,6 +198,31 @@ type installResult struct {
errMsg string
}

var (
getInstalledPackagesFn = GetInstalledPackages
preInstallChecksFn = PreInstallChecks

runBrewInstallBatchFn = func(args ...string) (string, error) {
cmd := brewInstallCmd(args...)
output, err := cmd.CombinedOutput()
return string(output), err
}

runBrewInstallBatchWithTTYFn = func(args ...string) (string, error) {
cmd := brewInstallCmd(args...)
tty, opened := system.OpenTTY()
if opened {
cmd.Stdin = tty
defer tty.Close()
}
output, err := cmd.CombinedOutput()
return string(output), err
}

installFormulaWithErrorFn = installFormulaWithError
installSmartCaskWithErrorFn = installSmartCaskWithError
)

func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedFormulae []string, installedCasks []string, err error) {
total := len(cliPkgs) + len(caskPkgs)
if total == 0 {
Expand All @@ -208,16 +231,16 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm

if dryRun {
ui.Info("Would install packages:")
for _, p := range cliPkgs {
fmt.Printf(" brew install %s\n", p)
if len(cliPkgs) > 0 {
fmt.Printf(" brew install %s\n", strings.Join(cliPkgs, " "))
}
for _, p := range caskPkgs {
fmt.Printf(" brew install --cask %s\n", p)
if len(caskPkgs) > 0 {
fmt.Printf(" brew install --cask %s\n", strings.Join(caskPkgs, " "))
}
return nil, nil, nil
}

alreadyFormulae, alreadyCasks, checkErr := GetInstalledPackages()
alreadyFormulae, alreadyCasks, checkErr := getInstalledPackagesFn()
if checkErr != nil {
return nil, nil, fmt.Errorf("list installed packages: %w", checkErr)
}
Expand Down Expand Up @@ -250,7 +273,7 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
return installedFormulae, installedCasks, nil
}

if preErr := PreInstallChecks(len(newCli) + len(newCask)); preErr != nil {
if preErr := preInstallChecksFn(len(newCli) + len(newCask)); preErr != nil {
return installedFormulae, installedCasks, preErr
}

Expand All @@ -261,37 +284,81 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
var allFailed []failedJob

if len(newCli) > 0 {
failed := runParallelInstallWithProgress(newCli, progress)
failedSet := make(map[string]bool, len(failed))
for _, f := range failed {
failedSet[f.name] = true
}
for _, p := range newCli {
if !failedSet[p] {
installedFormulae = append(installedFormulae, p)
progress.PrintLine(" Installing %d CLI packages via brew install...", len(newCli))
progress.PauseForInteractive()

args := append([]string{"install"}, newCli...)
cmdOutputStr, cmdErr := runBrewInstallBatchFn(args...)
progress.ResumeAfterInteractive()

// Re-check installed packages to determine actual success
postFormulae, _, postErr := getInstalledPackagesFn()
if postErr != nil {
// Fallback: use command error to determine status
for _, pkg := range newCli {
progress.IncrementWithStatus(cmdErr == nil)
if cmdErr == nil {
installedFormulae = append(installedFormulae, pkg)
} else {
allFailed = append(allFailed, failedJob{
installJob: installJob{name: pkg, isCask: false},
errMsg: parseBrewError(cmdOutputStr),
})
}
}
} else {
// Check each package individually
for _, pkg := range newCli {
isInstalled := postFormulae[pkg]
progress.IncrementWithStatus(isInstalled)
if isInstalled {
installedFormulae = append(installedFormulae, pkg)
} else {
allFailed = append(allFailed, failedJob{
installJob: installJob{name: pkg, isCask: false},
errMsg: extractPackageError(cmdOutputStr, pkg),
})
}
}
}
Comment on lines 286 to 323
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new non-dry-run behavior in InstallWithProgress (batch install + per-package classification) but current tests only cover the dry-run path. Adding unit-level coverage around the new classification path would help prevent regressions (e.g., by refactoring to inject the brew command runner and an "installed packages" provider so failures/partial successes can be simulated deterministically).

Copilot uses AI. Check for mistakes.
allFailed = append(allFailed, failed...)
}

if len(newCask) > 0 {
for _, pkg := range newCask {
progress.SetCurrent(pkg)
progress.PrintLine(" Installing %s...", pkg)
start := time.Now()
errMsg := installCaskWithProgress(pkg, progress)
elapsed := time.Since(start)
progress.IncrementWithStatus(errMsg == "")
duration := ui.FormatDuration(elapsed)
if errMsg == "" {
progress.PrintLine(" %s %s", ui.Green("✔ "+pkg), ui.Cyan("("+duration+")"))
installedCasks = append(installedCasks, pkg)
} else {
progress.PrintLine(" %s %s", ui.Red("✗ "+pkg+" ("+errMsg+")"), ui.Cyan("("+duration+")"))
allFailed = append(allFailed, failedJob{
installJob: installJob{name: pkg, isCask: true},
errMsg: errMsg,
})
progress.PrintLine(" Installing %d GUI apps via brew install --cask...", len(newCask))
progress.PauseForInteractive()

args := append([]string{"install", "--cask"}, newCask...)
cmdOutputStr, cmdErr := runBrewInstallBatchWithTTYFn(args...)
progress.ResumeAfterInteractive()

// Re-check installed casks to determine actual success
_, postCasks, postErr := getInstalledPackagesFn()
if postErr != nil {
// Fallback: use command error to determine status
for _, pkg := range newCask {
progress.IncrementWithStatus(cmdErr == nil)
if cmdErr == nil {
installedCasks = append(installedCasks, pkg)
} else {
allFailed = append(allFailed, failedJob{
installJob: installJob{name: pkg, isCask: true},
errMsg: parseBrewError(cmdOutputStr),
})
}
}
} else {
// Check each cask individually
for _, pkg := range newCask {
isInstalled := postCasks[pkg]
progress.IncrementWithStatus(isInstalled)
if isInstalled {
installedCasks = append(installedCasks, pkg)
} else {
allFailed = append(allFailed, failedJob{
installJob: installJob{name: pkg, isCask: true},
errMsg: extractPackageError(cmdOutputStr, pkg),
})
}
}
}
}
Expand All @@ -304,9 +371,9 @@ func InstallWithProgress(cliPkgs, caskPkgs []string, dryRun bool) (installedForm
for _, f := range allFailed {
var errMsg string
if f.isCask {
errMsg = installSmartCaskWithError(f.name)
errMsg = installSmartCaskWithErrorFn(f.name)
} else {
errMsg = installFormulaWithError(f.name)
errMsg = installFormulaWithErrorFn(f.name)
}
if errMsg == "" {
fmt.Printf(" ✔ %s (retry succeeded)\n", f.name)
Expand Down Expand Up @@ -366,101 +433,6 @@ type failedJob struct {
errMsg string
}

func runParallelInstallWithProgress(pkgs []string, progress *ui.StickyProgress) []failedJob {
if len(pkgs) == 0 {
return nil
}

jobs := make([]installJob, 0, len(pkgs))
for _, pkg := range pkgs {
jobs = append(jobs, installJob{name: pkg, isCask: false})
}

jobChan := make(chan installJob, len(jobs))
results := make(chan installResult, len(jobs))

var wg sync.WaitGroup
workers := maxWorkers
if len(jobs) < workers {
workers = len(jobs)
}

for i := 0; i < workers; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for job := range jobChan {
progress.SetCurrent(job.name)
start := time.Now()
errMsg := installFormulaWithError(job.name)
elapsed := time.Since(start)
progress.IncrementWithStatus(errMsg == "")
duration := ui.FormatDuration(elapsed)
if errMsg == "" {
progress.PrintLine(" %s %s", ui.Green("✔ "+job.name), ui.Cyan("("+duration+")"))
} else {
progress.PrintLine(" %s %s", ui.Red("✗ "+job.name+" ("+errMsg+")"), ui.Cyan("("+duration+")"))
}
results <- installResult{name: job.name, failed: errMsg != "", isCask: job.isCask, errMsg: errMsg}
}
}()
}

go func() {
for _, job := range jobs {
jobChan <- job
}
close(jobChan)
}()

go func() {
wg.Wait()
close(results)
}()

var failed []failedJob
for result := range results {
if result.failed {
failed = append(failed, failedJob{
installJob: installJob{name: result.name, isCask: result.isCask},
errMsg: result.errMsg,
})
}
}

return failed
}

func installCaskWithProgress(pkg string, progress *ui.StickyProgress) string {
progress.PauseForInteractive()

cmd := brewInstallCmd("install", "--cask", pkg)
tty, opened := system.OpenTTY()
if opened {
defer tty.Close()
}
cmd.Stdin = tty
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Run()

progress.ResumeAfterInteractive()

if err != nil {
return "install failed"
}
return ""
}

func printBrewOutput(output string, progress *ui.StickyProgress) {
for _, line := range strings.Split(strings.TrimSpace(output), "\n") {
line = strings.TrimSpace(line)
if line != "" {
progress.PrintLine(" %s", line)
}
}
}

func brewInstallCmd(args ...string) *exec.Cmd {
cmd := exec.Command("brew", args...)
cmd.Env = append(os.Environ(), "HOMEBREW_NO_AUTO_UPDATE=1")
Expand Down Expand Up @@ -598,6 +570,31 @@ func parseBrewError(output string) string {
}
}

// extractPackageError tries to find an error message specific to pkg in the
// combined brew output. Falls back to parseBrewError on the full output.
func extractPackageError(output, pkg string) string {
// Scan for lines mentioning the package name near an error indicator.
lowerPkg := strings.ToLower(pkg)
for _, line := range strings.Split(output, "\n") {
lower := strings.ToLower(line)
if strings.Contains(lower, lowerPkg) && strings.Contains(lower, "error") {
line = strings.TrimSpace(line)
if len(line) > 80 {
return line[:77] + "..."
}
return line
}
}

// No package-specific line found; fall back to the general parser but
// indicate the package was not installed after the batch attempt.
parsed := parseBrewError(output)
if parsed == "unknown error" {
return "not installed after batch attempt"
}
return parsed
}

func Uninstall(packages []string, dryRun bool) error {
if len(packages) == 0 {
return nil
Expand Down Expand Up @@ -857,3 +854,26 @@ func PreInstallChecks(packageCount int) error {

return nil
}

// ResolveFormulaName resolves a formula alias to its canonical name.
// This handles cases like "postgresql" → "postgresql@18" or "kubectl" → "kubernetes-cli".
// Returns the original name if resolution fails.
func ResolveFormulaName(name string) string {
cmd := exec.Command("brew", "info", "--json", name)
output, err := cmd.Output()
if err != nil {
return name
}

var result []struct {
Name string `json:"name"`
}
if err := json.Unmarshal(output, &result); err != nil {
return name
}

if len(result) > 0 && result[0].Name != "" {
return result[0].Name
}
return name
}
Loading
Loading