fix(brew): update Homebrew before non-interactive installs#43
Conversation
Zsh startup benchmarkUbuntu 24.04 with Linuxbrew, atuin, fnm, fzf, zoxide, zinit, and the shipped plugins. Lower is better. Warm startup (daily use)
Cache-miss startupCompletion and tool-init caches are removed before every run; installed tools and plugins remain.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to ensure brew update is run before installing Homebrew packages, while tracking the update state to avoid redundant executions. This logic is implemented across install.sh, src/steps/bootstrap.js, and src/utils/shell.js, with corresponding test updates. The review feedback highlights a potential issue with test isolation and redundant updates due to the module-level global state. It suggests exporting a helper function setHomebrewUpdated to allow resetting the state in tests and updating it in bootstrap.js after the initial update.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
| } | ||
|
|
||
| let homebrewUpdated = false; |
There was a problem hiding this comment.
为了支持在其他模块和测试中更新或重置 homebrewUpdated 的状态,建议导出一个 setHomebrewUpdated 辅助函数。
这样做有两个主要好处:
- 避免重复更新:在
src/steps/bootstrap.js中执行brew update后,可以将其设置为true,从而避免后续调用brewInstall时再次触发缓慢的brew update。 - 测试隔离:在单元测试中,可以在
beforeEach中将其重置为false,防止测试用例之间共享全局状态导致测试不稳定。
let homebrewUpdated = false;
export function setHomebrewUpdated(value) {
homebrewUpdated = value;
}| if (manager === "brew") { | ||
| await runStream("brew install --no-ask zsh"); | ||
| await runStream("env -u HOMEBREW_ASK brew update && brew install --no-ask zsh"); | ||
| } else if (manager === "apt-get") { |
There was a problem hiding this comment.
在成功执行 brew update 并安装 Zsh 后,建议调用 setHomebrewUpdated(true)。
这样可以确保后续通过 brewInstall 安装其他软件时,不会再次触发冗余且缓慢的 brew update,从而显著提升整体安装效率。
注意:
- 需要在文件顶部导入
setHomebrewUpdated:import { commandExists, run, runStream, setHomebrewUpdated } from "../utils/shell.js";- 需要在
tests/bootstrap.test.js的shell.js模块 mock 中添加setHomebrewUpdated: vi.fn(),以避免测试报错。
| if (manager === "brew") { | |
| await runStream("brew install --no-ask zsh"); | |
| await runStream("env -u HOMEBREW_ASK brew update && brew install --no-ask zsh"); | |
| } else if (manager === "apt-get") { | |
| if (manager === "brew") { | |
| await runStream("env -u HOMEBREW_ASK brew update && brew install --no-ask zsh"); | |
| setHomebrewUpdated(true); | |
| } else if (manager === "apt-get") { |
|
|
||
| expect(brewInstall("ghostty", { cask: true })).toBe(true); | ||
| expect(execSync).toHaveBeenCalledWith("brew install --no-ask --cask ghostty", { stdio: "inherit" }); | ||
| expect(execSync.mock.calls.filter(([command]) => command.includes("brew update"))).toHaveLength(1); |
There was a problem hiding this comment.
由于 homebrewUpdated 是 src/utils/shell.js 中的模块级全局变量,它会在不同的测试用例之间共享。
在当前测试中,第一个 brewInstall("jq") 会将 homebrewUpdated 设为 true。这会导致后续的测试(例如 "propagates Ctrl-C from Homebrew installs")在调用 brewInstall 时直接跳过 brewUpdate()。
为了保证测试的隔离性,建议在 beforeEach 中重置该状态:
- 在文件顶部导入
setHomebrewUpdated:import { brewInstall, runStream, setHomebrewUpdated } from "../src/utils/shell.js";
- 在
beforeEach中重置状态:beforeEach(() => { vi.clearAllMocks(); setHomebrewUpdated(false); });
问题
旧版 Homebrew 不支持
brew install --no-ask,导致选中的软件全部安装失败。修复
brew updatebrew install --no-ask验证
npm test(231 tests passed)git diff --check