diff --git a/packages/host-watcher/src/workspace-status-watcher.ts b/packages/host-watcher/src/workspace-status-watcher.ts index df266c0db6..8dd7e459d3 100644 --- a/packages/host-watcher/src/workspace-status-watcher.ts +++ b/packages/host-watcher/src/workspace-status-watcher.ts @@ -30,7 +30,26 @@ const WORKSPACE_STATUS_WATCH_MAX_RETRY_DELAY_MS = 30_000; // Give up after a bounded number of attempts; the server recreates this watch // (resetting the count) when the watch set changes. const WORKSPACE_STATUS_WATCH_MAX_SETUP_RETRY_ATTEMPTS = 10; +// Plain entries are paths relative to the watch root: `.git` only excludes +// `/.git`, the workspace's own repository. const WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS = [".git"]; +// Glob entries are matched against the root-relative path. On Linux parcel +// tests every directory during its crawl and a match skips the whole subtree, +// so no inotify watch is created below it. On macOS and Windows parcel tests +// each event path instead, so the trailing `/**` is required: picomatch lets +// it match zero segments, so `**/node_modules/**` matches both the directory +// and everything inside it. The Git-derived ignore list below only covers the +// root's own top-level ignored directories; an "umbrella" root with untracked +// nested checkouts, or a root that is not a repository, otherwise gets one +// inotify watch per nested directory and can OOM the host (get-bb/bb#1779). +// `*/**/.git/**` skips nested repositories but keeps `/.git` watchable +// so a plain directory can still be promoted after `git init`. +const WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS = [ + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", +]; const WORKSPACE_ROOT_IGNORE_STATUS_TIMEOUT_MS = 5_000; const WORKSPACE_ROOT_IGNORE_STATUS_MAX_BUFFER_BYTES = 10 * 1024 * 1024; @@ -101,10 +120,21 @@ function collectIgnoredDirectoryPaths(statusOutput: string): string[] { return Array.from(ignoredDirectoryPaths).sort(); } +function createGitWorkspaceRootIgnores(): string[] { + return [ + ...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS, + ...WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS, + ]; +} + +function createPlainWorkspaceRootIgnores(): string[] { + return [...WORKSPACE_ROOT_ALWAYS_IGNORED_GLOBS]; +} + function mergeWorkspaceRootIgnores(gitIgnoredPaths: string[]): string[] { const ignoredPaths = new Set(); for (const ignoredPath of [ - ...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS, + ...createGitWorkspaceRootIgnores(), ...gitIgnoredPaths, ]) { ignoredPaths.add(ignoredPath); @@ -220,9 +250,11 @@ export class WorkspaceStatusWatcher { } if (!(await pathExists(path.join(this.args.cwd, ".git")))) { // A plain workspace can become a repository after `git init`. Watch the - // root without excluding `.git` until that marker appears. + // root without excluding `/.git` until that marker appears, but + // still skip nested repositories and heavy directories. this.startWatchSubscription({ kind: "workspace-root", + options: { ignore: createPlainWorkspaceRootIgnores() }, rootPath, }); return; @@ -267,11 +299,11 @@ export class WorkspaceStatusWatcher { } this.reportWorkspaceRootSetupError(rootPath, error); // Ignore discovery is an optimization, not a correctness gate. Keep the - // workspace live with the mandatory `.git` exclusion even when Git is - // too slow or its metadata is temporarily unavailable. + // workspace live with the mandatory `.git` and nested-tree exclusions + // even when Git is too slow or its metadata is temporarily unavailable. this.startWatchSubscription({ kind: "workspace-root", - options: { ignore: [...WORKSPACE_ROOT_ALWAYS_IGNORED_PATHS] }, + options: { ignore: createGitWorkspaceRootIgnores() }, rootPath, }); } diff --git a/packages/host-watcher/test/watch-status.test.ts b/packages/host-watcher/test/watch-status.test.ts index 3a4122e3a7..7cadd9c9d4 100644 --- a/packages/host-watcher/test/watch-status.test.ts +++ b/packages/host-watcher/test/watch-status.test.ts @@ -420,7 +420,8 @@ describe.sequential("watchWorkspaceStatus", () => { try { await ready.promise; - expect(workspaceRootOptions[0]?.ignore).toBeUndefined(); + expect(workspaceRootOptions[0]?.ignore).not.toContain(".git"); + expect(workspaceRootOptions[0]?.ignore).toContain("*/**/.git/**"); await runGit({ args: ["init", "-b", "main"], cwd: workspacePath }); const canonicalWorkspacePath = await fs.realpath(workspacePath); @@ -732,6 +733,10 @@ describe.sequential("watchWorkspaceStatus", () => { await ready; expect(getWorkspaceRootSubscribeOptions()?.ignore).toEqual([ ".git", + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", ".turbo", "coverage", ]); @@ -785,7 +790,13 @@ describe.sequential("watchWorkspaceStatus", () => { expect(ignoreDiscoveryErrors).toHaveLength(1); expect(ignoreDiscoveryErrors[0]).toContain(normalizeWatchPath(repoPath)); expect(subscribedRoots).toEqual([normalizeWatchPath(repoPath)]); - expect(subscribedOptions[0]?.ignore).toEqual([".git"]); + expect(subscribedOptions[0]?.ignore).toEqual([ + ".git", + "*/**/.git/**", + "**/node_modules/**", + "**/.cache/**", + "**/__pycache__/**", + ]); } finally { await stopWatching(); } diff --git a/packages/host-watcher/test/workspace-root-ignores.test.ts b/packages/host-watcher/test/workspace-root-ignores.test.ts new file mode 100644 index 0000000000..55e12344cf --- /dev/null +++ b/packages/host-watcher/test/workspace-root-ignores.test.ts @@ -0,0 +1,276 @@ +// Regression tests for get-bb/bb#1779: the workspace-root watch of an +// "umbrella" root (a directory with untracked nested checkouts that contain +// node_modules and .git) must not register an inotify watch on every nested +// directory, and must not report changes inside those trees. The Git-derived +// ignore list only covers the root's own top-level ignored directories, so the +// watcher has to add recursive glob ignores. +// +// The inotify tests are Linux only: they read the real watch count from +// /proc/self/fdinfo. The event test runs on every platform because parcel +// applies the globs during the crawl on Linux but per event on macOS/Windows. +import { execFile } from "node:child_process"; +import fsSync from "node:fs"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import parcelWatcher from "@parcel/watcher"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { watchWorkspaceStatus } from "../src/watch-status.js"; +import type { WorkspaceStatusChangeEvent } from "../src/watch-status-types.js"; + +const execFileAsync = promisify(execFile); +const tempDirs: string[] = []; +// Bind once at module load so a spy left behind by a failed test can never +// become the "real" implementation of the next spy. +const realParcelSubscribe = parcelWatcher.subscribe.bind(parcelWatcher); + +const NESTED_REPOS = 4; +const PACKAGES_PER_NESTED_REPO = 300; +const EVENT_TIMEOUT_MS = 5_000; +// The umbrella tree has thousands of directories; CI runners build it slowly. +const TEST_TIMEOUT_MS = 60_000; +// Root, apps/, apps/child-N and the root's own git-dir metadata watches. +const MAX_EXPECTED_WATCHES = 20; + +async function git(cwd: string, ...args: string[]): Promise { + await execFileAsync("git", args, { cwd, encoding: "utf8" }); +} + +async function initRepo(dir: string): Promise { + await fs.mkdir(dir, { recursive: true }); + await git(dir, "init", "-q", "-b", "main"); + await git(dir, "config", "user.name", "BB Tests"); + await git(dir, "config", "user.email", "bb@example.com"); + await fs.writeFile(path.join(dir, "README.md"), "hello\n"); + await git(dir, "add", "README.md"); + await git(dir, "commit", "-q", "-m", "init"); +} + +async function buildUmbrellaRoot(args: { + gitRoot: boolean; + nestedRepos?: number; + packagesPerNestedRepo?: number; +}): Promise<{ + root: string; + nestedDirCount: number; +}> { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "bb-1779-umbrella-")); + tempDirs.push(root); + if (args.gitRoot) { + await initRepo(root); + } + const nestedRepos = args.nestedRepos ?? NESTED_REPOS; + const packagesPerNestedRepo = + args.packagesPerNestedRepo ?? PACKAGES_PER_NESTED_REPO; + let nestedDirCount = 0; + for (let i = 0; i < nestedRepos; i += 1) { + const child = path.join(root, "apps", `child-${i}`); + await initRepo(child); + // The nested repo ignores its own node_modules, like every real project. + await fs.writeFile(path.join(child, ".gitignore"), "node_modules/\n"); + await git(child, "add", ".gitignore"); + await git(child, "commit", "-q", "-m", "ignore node_modules"); + await Promise.all( + Array.from({ length: packagesPerNestedRepo }, async (_unused, p) => { + const pkgLib = path.join(child, "node_modules", `pkg-${p}`, "lib"); + await fs.mkdir(pkgLib, { recursive: true }); + await fs.writeFile( + path.join(pkgLib, "index.js"), + "module.exports={}\n", + ); + }), + ); + nestedDirCount += packagesPerNestedRepo * 2; + } + return { root, nestedDirCount }; +} + +function countInotifyWatches(): number { + const fdinfoDir = "/proc/self/fdinfo"; + let count = 0; + for (const fd of fsSync.readdirSync(fdinfoDir)) { + try { + const info = fsSync.readFileSync(path.join(fdinfoDir, fd), "utf8"); + count += info + .split("\n") + .filter((line) => line.startsWith("inotify wd:")).length; + } catch { + // fd closed between readdir and read + } + } + return count; +} + +async function measureWorkspaceRootWatch(root: string): Promise<{ + ignore: string[] | undefined; + watches: number; +}> { + const seenOptions: Array<{ dir: string; ignore: string[] | undefined }> = []; + vi.spyOn(parcelWatcher, "subscribe").mockImplementation( + async (dir, cb, opts) => { + seenOptions.push({ dir, ignore: opts?.ignore }); + return realParcelSubscribe(dir, cb, opts); + }, + ); + const baselineWatches = countInotifyWatches(); + let ready!: () => void; + const readyPromise = new Promise((resolve) => { + ready = resolve; + }); + const stop = watchWorkspaceStatus(root, { + onChange: () => undefined, + onReady: () => ready(), + onWatchError: () => undefined, + }); + try { + await readyPromise; + // Settle the metadata (git-dir) subscriptions too. + await new Promise((resolve) => setTimeout(resolve, 300)); + const realRoot = fsSync.realpathSync(root); + const workspaceRootSubscribe = seenOptions.find( + (o) => o.dir === root || o.dir === realRoot, + ); + return { + ignore: workspaceRootSubscribe?.ignore, + watches: countInotifyWatches() - baselineWatches, + }; + } finally { + await stop(); + } +} + +async function waitFor( + predicate: () => boolean, + timeoutMs: number, +): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) { + throw new Error("Timed out waiting for workspace change events"); + } + await new Promise((resolve) => setTimeout(resolve, 20)); + } +} + +afterEach(async () => { + vi.restoreAllMocks(); + for (const dir of tempDirs.splice(0)) { + await fs.rm(dir, { force: true, recursive: true }); + } +}); + +describe.skipIf(process.platform !== "linux")( + "workspace root watch ignores nested heavy directories (#1779)", + () => { + it( + "does not watch nested node_modules or .git under a git umbrella root", + async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: true, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + expect(ignore).toContain(".git"); + expect(ignore).toContain("**/node_modules/**"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }, + TEST_TIMEOUT_MS, + ); + + it( + "does not watch nested node_modules or .git under a non-git root", + async () => { + const { root, nestedDirCount } = await buildUmbrellaRoot({ + gitRoot: false, + }); + const { ignore, watches } = await measureWorkspaceRootWatch(root); + expect(nestedDirCount).toBeGreaterThan(MAX_EXPECTED_WATCHES); + // `/.git` must stay watchable so `git init` promotion still fires. + expect(ignore).not.toContain(".git"); + expect(ignore).toContain("**/node_modules/**"); + expect(watches).toBeLessThan(MAX_EXPECTED_WATCHES); + }, + TEST_TIMEOUT_MS, + ); + }, +); + +describe("workspace root watch events inside nested heavy directories (#1779)", () => { + it( + "does not report changes inside nested node_modules or nested .git", + async () => { + const { root } = await buildUmbrellaRoot({ + gitRoot: true, + nestedRepos: 1, + packagesPerNestedRepo: 1, + }); + const realRoot = fsSync.realpathSync(root); + const nestedPackageFile = path.join( + realRoot, + "apps", + "child-0", + "node_modules", + "pkg-0", + "lib", + "index.js", + ); + const nestedGitFile = path.join( + realRoot, + "apps", + "child-0", + ".git", + "bb-marker", + ); + const visibleFile = path.join(realRoot, "apps", "child-0", "visible.txt"); + const events: WorkspaceStatusChangeEvent[] = []; + let ready!: () => void; + const readyPromise = new Promise((resolve) => { + ready = resolve; + }); + const stop = watchWorkspaceStatus(root, { + onChange: (event) => { + events.push(event); + }, + onReady: () => ready(), + onWatchError: () => undefined, + }); + try { + await readyPromise; + // Let the initial crawl and the FSEvents stream settle. + await new Promise((resolve) => setTimeout(resolve, 300)); + + await fs.writeFile( + nestedPackageFile, + "module.exports={changed:true}\n", + ); + await fs.writeFile(nestedGitFile, "marker\n"); + // The visible write is the control: it proves the watch is live and + // delivers events, so the absence of the nested paths is meaningful. + await fs.writeFile(visibleFile, "visible\n"); + await waitFor( + () => + events.some((event) => event.changedPaths.includes(visibleFile)), + EVENT_TIMEOUT_MS, + ); + // Give any straggling nested events a chance to arrive. + await new Promise((resolve) => setTimeout(resolve, 300)); + + const changedPaths = events.flatMap((event) => event.changedPaths); + expect(changedPaths).toContain(visibleFile); + expect(changedPaths).not.toContain(nestedPackageFile); + expect(changedPaths).not.toContain(nestedGitFile); + expect( + changedPaths.filter( + (changedPath) => + changedPath.includes(`${path.sep}node_modules${path.sep}`) || + changedPath.includes(`${path.sep}.git${path.sep}`), + ), + ).toEqual([]); + } finally { + await stop(); + } + }, + TEST_TIMEOUT_MS, + ); +});