From 8f7e592c72453dd02d1f14e3d87d57b27cd6dc1e Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 27 May 2026 17:43:00 -0500 Subject: [PATCH] fixed MacOS parallelism --- batch_test.go | 74 ++++++++++++++++++++++++++++++++++++++++++++++----- system.go | 61 +++++++++++++++++++++++++++++++----------- 2 files changed, 113 insertions(+), 22 deletions(-) diff --git a/batch_test.go b/batch_test.go index cd57c6f..50b6ef3 100644 --- a/batch_test.go +++ b/batch_test.go @@ -128,19 +128,78 @@ func TestPkgInstallManyEmpty(t *testing.T) { } } -// TestPkgInstallManyBrew uses parallel per-package install (no batching at -// the CLI level because brew formula installs may have their own preferences). +// TestPkgInstallManyBrew verifies that brew is batched into a single +// `brew install f1 f2 …` call (formulas only — no casks in this test). +// Parallel brew calls would deadlock on shared transitive-dep locks +// (cmake, ninja, libsodium, …), so we deliberately batch and serialize. func TestPkgInstallManyBrew(t *testing.T) { defer resetMocks() pkgMgr = "brew" var mu sync.Mutex - var seen []string + var calls [][]string runCmd = func(argv []string, _ CmdOpts) CmdResult { mu.Lock() - // last arg is the package name for `brew install ` - seen = append(seen, argv[len(argv)-1]) + calls = append(calls, append([]string(nil), argv...)) mu.Unlock() + return CmdResult{ExitCode: 0} + } + + if failed := pkgInstallMany([]string{"git", "vim", "curl"}); len(failed) != 0 { + t.Errorf("expected no failures, got %v", failed) + } + if len(calls) != 1 { + t.Fatalf("expected exactly 1 batched brew call, got %d: %v", len(calls), calls) + } + got := strings.Join(calls[0], " ") + if got != "brew install git vim curl" { + t.Errorf("expected 'brew install git vim curl', got %q", got) + } +} + +// TestPkgInstallManyBrewSplitCasks verifies that casks and formulas are +// emitted in separate calls (because --cask is mutually exclusive with +// formula installs in one invocation). +func TestPkgInstallManyBrewSplitCasks(t *testing.T) { + defer resetMocks() + pkgMgr = "brew" + + var calls [][]string + runCmd = func(argv []string, _ CmdOpts) CmdResult { + calls = append(calls, append([]string(nil), argv...)) + return CmdResult{ExitCode: 0} + } + + // "docker" is in brewCasks; the rest are formulas. + pkgInstallMany([]string{"git", "docker", "vim"}) + + if len(calls) != 2 { + t.Fatalf("expected 2 calls (1 formula batch + 1 cask batch), got %d: %v", len(calls), calls) + } + formula := strings.Join(calls[0], " ") + cask := strings.Join(calls[1], " ") + if formula != "brew install git vim" { + t.Errorf("expected 'brew install git vim', got %q", formula) + } + if cask != "brew install --cask docker" { + t.Errorf("expected 'brew install --cask docker', got %q", cask) + } +} + +// TestPkgInstallManyBrewFallback: batched formula install fails; we retry +// per-package and identify the broken one. +func TestPkgInstallManyBrewFallback(t *testing.T) { + defer resetMocks() + pkgMgr = "brew" + + calls := 0 + runCmd = func(argv []string, _ CmdOpts) CmdResult { + calls++ + // First call is the batch — fail it. + if calls == 1 { + return CmdResult{ExitCode: 1} + } + // Per-package retries: only "broken" fails. if argv[len(argv)-1] == "broken" { return CmdResult{ExitCode: 1} } @@ -151,8 +210,9 @@ func TestPkgInstallManyBrew(t *testing.T) { if len(failed) != 1 || failed[0] != "broken" { t.Errorf("expected only 'broken' to fail, got %v", failed) } - if len(seen) != 3 { - t.Errorf("expected 3 brew calls (one per pkg), got %d: %v", len(seen), seen) + // 1 batch + 3 per-package retries = 4 calls. + if calls != 4 { + t.Errorf("expected 4 total calls, got %d", calls) } } diff --git a/system.go b/system.go index 5fe690b..16e950c 100644 --- a/system.go +++ b/system.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "strings" - "sync" ) // Special packages: installed outside the regular package manager because @@ -396,28 +395,22 @@ func pkgInstall(pkg string) CmdResult { // pkgInstallMany installs all named packages in a single invocation of the // host package manager. This is dramatically faster than per-package install -// loops because apt/dnf/pacman amortize metadata refresh, dependency +// loops because apt/dnf/pacman/brew amortize metadata refresh, dependency // resolution, and (most importantly) only acquire the install lock once. // // On batch failure we fall back to per-package installs so callers can -// continue to report which specific packages failed via errLog. brew gets -// each formula in parallel goroutines (it tolerates concurrent invocations -// when packages don't share build dependencies; cask installs go through -// the same path). +// continue to report which specific packages failed via errLog. brew is +// split into formula vs cask batches because `--cask` is mutually exclusive +// with formula installs in one invocation. We deliberately do NOT run brew +// invocations in parallel — brew acquires per-Cellar locks on transitive +// dependencies (cmake, ninja, libsodium, etc.), and concurrent invocations +// that both pull in the same dep abort with "process has already locked". func pkgInstallMany(pkgs []string) (failed []string) { if len(pkgs) == 0 { return nil } if pkgMgr == "brew" { - var mu sync.Mutex - parallelDo(pkgs, cpuWorkers(), func(_ int, p string) { - if !pkgInstall(p).OK() { - mu.Lock() - failed = append(failed, p) - mu.Unlock() - } - }) - return failed + return brewInstallMany(pkgs) } var argv []string switch pkgMgr { @@ -440,6 +433,44 @@ func pkgInstallMany(pkgs []string) (failed []string) { return failed } +// brewInstallMany installs pkgs via brew, batching formulas and casks into +// two single invocations (`brew install f1 f2 …` and `brew install --cask +// c1 c2 …`). Brew resolves and parallelizes the internal dep graph itself, +// so a single batched call is both faster and lock-safe — multiple +// concurrent `brew install` processes deadlock on shared deps. On batch +// failure we retry per-package serially to identify which specific package +// broke. +func brewInstallMany(pkgs []string) (failed []string) { + var formulas, casks []string + for _, p := range pkgs { + if brewCasks[p] { + casks = append(casks, p) + } else { + formulas = append(formulas, p) + } + } + tryBatch := func(label string, names []string, extra ...string) (batchFailed []string) { + if len(names) == 0 { + return nil + } + argv := append([]string{"brew", "install"}, extra...) + argv = append(argv, names...) + if runCmd(argv, CmdOpts{}).OK() { + return nil + } + warn(fmt.Sprintf("Batched brew %s install failed; retrying %d packages individually ...", label, len(names))) + for _, p := range names { + if !pkgInstall(p).OK() { + batchFailed = append(batchFailed, p) + } + } + return batchFailed + } + failed = append(failed, tryBatch("formula", formulas)...) + failed = append(failed, tryBatch("cask", casks, "--cask")...) + return failed +} + // installSystemPackages installs the regular + special package lists. func installSystemPackages(regular, special []string) { fmt.Println("\n=== System Packages ===")