diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index e3af0855ebfac..3850c76696f6a 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -4,6 +4,8 @@ import { it as effectIt } from "@effect/vitest"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { DesktopHostTelemetryMessage } from "@t3tools/contracts"; import * as Schema from "effect/Schema"; +import * as Scheduler from "effect/Scheduler"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; @@ -23,6 +25,7 @@ import { type CommandAvailabilityChecker, isCommandAvailable, listLoginShellCandidates, + locateXcodeGitWithXcrun, mergePathEntries, mergePathValues, npmShimExecutable, @@ -44,14 +47,26 @@ const decodeDesktopHostMessage = Schema.decodeUnknownSync(DesktopHostTelemetryMe const nativeProbe = vi.hoisted(() => ({ execFile: vi.fn(), original: undefined as unknown as typeof import("node:child_process").execFile, + spawn: vi.fn(), })); vi.mock("node:child_process", async (importOriginal) => { const original = await importOriginal(); nativeProbe.original = original.execFile; - return { ...original, execFile: nativeProbe.execFile.mockImplementation(original.execFile) }; + return { + ...original, + execFile: nativeProbe.execFile.mockImplementation(original.execFile), + spawn: nativeProbe.spawn.mockImplementation(original.spawn), + }; }); +const exitOf = (child: NodeChildProcess.ChildProcess) => + child.exitCode !== null || child.signalCode !== null + ? Promise.resolve({ code: child.exitCode, signal: child.signalCode }) + : new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => + child.once("exit", (code, signal) => resolve({ code, signal })), + ); + const withWindowsEnvironmentMocks = ( effect: Effect.Effect, readEnvironment: ( @@ -860,10 +875,12 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { Effect.provideService(SpawnExecutableResolution, (command) => command === "git" ? options.gitOnPath : files.has(command) ? command : undefined, ), - Effect.provideService(XcodeGitLocation, () => { - host.xcrunCalls += 1; - return host.xcrunGit; - }), + Effect.provideService(XcodeGitLocation, () => + Effect.sync(() => { + host.xcrunCalls += 1; + return host.xcrunGit; + }), + ), ); return { files, host, resolve }; }; @@ -879,6 +896,118 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); + // A small yield budget lets the scheduler switch callers between any two operations. + it.effect.each([undefined, 3, 4, 5, 6] as const)( + "waits for xcrun without blocking, sharing one lookup between callers (yield budget %s)", + (budget) => + Effect.gen(function* () { + let lookup = yield* Deferred.make(); + let xcrunCalls = 0; + const resolve = resolveSpawnCommand("git", ["status"], { + env: { PATH: "/fake/bin" }, + }).pipe( + Effect.map((resolved) => resolved.command), + Effect.provideService(HostProcess.Platform, "darwin"), + Effect.provideService(SpawnExecutableResolution, (command) => + command === "git" ? "/usr/bin/git" : command === XCODE_GIT ? command : undefined, + ), + Effect.provideService(XcodeGitLocation, () => { + xcrunCalls += 1; + return Deferred.await(lookup); + }), + budget === undefined + ? (effect) => effect + : Effect.provideService(Scheduler.MaxOpsBeforeYield, budget), + ); + const callers = Effect.forEach([1, 2, 3], () => + Effect.forkChild(resolve, { startImmediately: true }), + ); + const first = yield* callers; + yield* Deferred.succeed(lookup, XCODE_GIT); + expect(yield* Fiber.joinAll(first)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]); + expect(xcrunCalls).toBe(1); + + // Once the answer expires, the next callers share one new lookup. + lookup = yield* Deferred.make(); + yield* TestClock.adjust("31 seconds"); + const second = yield* callers; + yield* Deferred.succeed(lookup, XCODE_GIT); + expect(yield* Fiber.joinAll(second)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]); + expect(xcrunCalls).toBe(2); + }).pipe(Effect.provideService(CommandResolutionCache, new Map())), + ); + + it.effect("the xcrun lookup resolves nothing when xcrun is missing or fails", () => + Effect.gen(function* () { + const env = { PATH: "/usr/bin:/bin", DEVELOPER_DIR: "/nonexistent/t3-developer-dir" }; + const locate = yield* XcodeGitLocation; + expect(yield* locate(env)).toBeUndefined(); + expect( + yield* locateXcodeGitWithXcrun(env, { xcrun: "/nonexistent/t3-xcrun" }), + ).toBeUndefined(); + }), + ); + + it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "the xcrun lookup settles at its deadline when a descendant keeps stdout open", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" }); + const descendantPid = path.join(directory, "descendant.pid"); + const xcrun = path.join(directory, "xcrun"); + yield* fs.writeFileString( + xcrun, + `#!/bin/sh\nsleep 600 &\necho $! > '${descendantPid}'\necho /fake/git\n`, + ); + yield* fs.chmod(xcrun, 0o755); + const stopDescendant = fs.readFileString(descendantPid).pipe( + Effect.flatMap((pid) => Effect.sync(() => process.kill(Number(pid.trim())))), + Effect.ignore, + ); + yield* Effect.addFinalizer(() => stopDescendant); + const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe( + Effect.forkChild({ startImmediately: true }), + ); + const child = nativeProbe.spawn.mock.results.at(-1) + ?.value as NodeChildProcess.ChildProcess; + yield* Effect.promise(() => exitOf(child)); + + // xcrun has exited and printed a path, but its descendant still holds stdout open. + expect(lookup.pollUnsafe()).toBeUndefined(); + yield* TestClock.adjust("5 seconds"); + expect(yield* Fiber.join(lookup)).toBeUndefined(); + }).pipe(Effect.scoped), + ); + + it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "the xcrun lookup kills an xcrun that ignores SIGTERM at its deadline", + () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" }); + const xcrun = path.join(directory, "xcrun"); + yield* fs.writeFileString(xcrun, "#!/bin/sh\ntrap '' TERM\nexec sleep 600\n"); + yield* fs.chmod(xcrun, 0o755); + const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe( + Effect.forkChild({ startImmediately: true }), + ); + const child = nativeProbe.spawn.mock.results.at(-1) + ?.value as NodeChildProcess.ChildProcess; + const kill = vi.spyOn(child, "kill"); + const exited = exitOf(child); + yield* Effect.addFinalizer(() => Effect.sync(() => child.kill("SIGKILL"))); + + expect(lookup.pollUnsafe()).toBeUndefined(); + yield* TestClock.adjust("5 seconds"); + expect(yield* Fiber.join(lookup)).toBeUndefined(); + expect(kill).toHaveBeenCalledWith("SIGKILL"); + expect(yield* Effect.promise(() => exited)).toEqual({ code: null, signal: "SIGKILL" }); + }).pipe(Effect.scoped), + ); + it.effect("keeps git when PATH has another git or the host is not macOS", () => Effect.gen(function* () { const homebrew = setup({ gitOnPath: "/opt/homebrew/bin/git", xcrunGit: XCODE_GIT }); @@ -901,18 +1030,27 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); - it.effect("looks again when the resolved git disappears", () => - Effect.gen(function* () { - const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git"; - const { files, host, resolve } = setup({ gitOnPath: "/usr/bin/git", xcrunGit: XCODE_GIT }); - expect(yield* resolve).toBe(XCODE_GIT); - - files.delete(XCODE_GIT); - files.add(betaGit); - host.xcrunGit = betaGit; - expect(yield* resolve).toBe(betaGit); - expect(host.xcrunCalls).toBe(2); - }).pipe(Effect.provideService(CommandResolutionCache, new Map())), + it.effect.each([undefined, 3] as const)( + "looks again when the resolved git disappears (yield budget %s)", + (budget) => + Effect.gen(function* () { + const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git"; + const { files, host, resolve } = setup({ + gitOnPath: "/usr/bin/git", + xcrunGit: XCODE_GIT, + }); + const budgeted = + budget === undefined + ? resolve + : resolve.pipe(Effect.provideService(Scheduler.MaxOpsBeforeYield, budget)); + expect(yield* budgeted).toBe(XCODE_GIT); + + files.delete(XCODE_GIT); + files.add(betaGit); + host.xcrunGit = betaGit; + expect(yield* budgeted).toBe(betaGit); + expect(host.xcrunCalls).toBe(2); + }).pipe(Effect.provideService(CommandResolutionCache, new Map())), ); }); diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 28b666ce05531..aaf30870c6240 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -6,7 +6,9 @@ import * as NodeFS from "node:fs"; import * as Cache from "effect/Cache"; import * as Clock from "effect/Clock"; import * as Duration from "effect/Duration"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; @@ -760,22 +762,63 @@ export function npmShimExecutable( return isFile(executable) ? executable : undefined; } -export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => string | undefined; +export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => Effect.Effect; + +const XCRUN_MAX_OUTPUT_BYTES = 1024 * 1024; +const XCRUN_DEADLINE = Duration.seconds(5); + +// Stopping it kills xcrun with SIGKILL, which it cannot ignore, and stops reading a stdout +// that a descendant may still hold open after xcrun exits. +const runXcrunFindGit = (xcrun: string, env: NodeJS.ProcessEnv) => + Effect.callback>((resume) => { + let child: NodeChildProcess.ChildProcess; + try { + child = NodeChildProcess.spawn(xcrun, ["--find", "git"], { + env, + stdio: ["ignore", "pipe", "ignore"], + }); + } catch { + resume(Effect.succeedNone); + return; + } + const stop = () => { + child.kill("SIGKILL"); + child.unref(); + child.stdout?.destroy(); + }; + const chunks: Array = []; + let bytes = 0; + child.stdout?.on("data", (chunk: Buffer) => { + bytes += chunk.byteLength; + if (bytes > XCRUN_MAX_OUTPUT_BYTES) { + stop(); + resume(Effect.succeedNone); + return; + } + chunks.push(chunk); + }); + child.once("error", () => resume(Effect.succeedNone)); + child.once("close", (code) => { + const located = Buffer.concat(chunks).toString("utf8").trim(); + resume( + code === 0 && NodePath.isAbsolute(located) + ? Effect.succeedSome(located) + : Effect.succeedNone, + ); + }); + return Effect.sync(stop); + }); // xcrun honors DEVELOPER_DIR and the `xcode-select` choice, as the shim does. -function locateXcodeGitWithXcrun(env: NodeJS.ProcessEnv): string | undefined { - try { - const located = NodeChildProcess.execFileSync("/usr/bin/xcrun", ["--find", "git"], { - encoding: "utf8", - env, - timeout: 5_000, - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - return NodePath.isAbsolute(located) ? located : undefined; - } catch { - return undefined; - } -} +// It takes ~15 ms and reruns when the cached answer expires, so it must not block the event loop. +export const locateXcodeGitWithXcrun = ( + env: NodeJS.ProcessEnv, + options: { readonly xcrun?: string } = {}, +): Effect.Effect => + runXcrunFindGit(options.xcrun ?? "/usr/bin/xcrun", env).pipe( + Effect.timeoutOption(XCRUN_DEADLINE), + Effect.map((found) => Option.getOrUndefined(Option.flatten(found))), + ); export const XcodeGitLocation = Context.Reference( "@t3tools/shared/shell/XcodeGitLocation", @@ -784,6 +827,28 @@ export const XcodeGitLocation = Context.Reference( const MACOS_GIT_SHIM = "/usr/bin/git"; +// Callers that miss the cache together wait for one xcrun run instead of starting their own. +const xcodeGitLookups = new WeakMap< + Map, + Map> +>(); + +const lookupXcodeGit = ( + env: NodeJS.ProcessEnv, + resolveExecutable: SpawnExecutableResolver, + locateXcodeGit: XcodeGitLocator, +): Effect.Effect => + resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM + ? locateXcodeGit(env).pipe( + Effect.map((located) => + located !== undefined && resolveExecutable(located, "darwin", env) !== undefined + ? located + : null, + ), + Effect.catchCause(() => Effect.succeed(null)), + ) + : Effect.succeed(null); + /** * Skip the xcrun shim's per-spawn lookup only when git resolves to /usr/bin/git. * User-selected git on PATH still wins, children keep PATH, and lookup failures use `git`. @@ -812,13 +877,33 @@ const resolveMacosGit = Effect.fnUntraced(function* ( ) { return cached.resolvedPath ?? "git"; } - const located = - resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM ? locateXcodeGit(env) : undefined; - const realGit = - located !== undefined && resolveExecutable(located, "darwin", env) !== undefined - ? located - : null; - cacheCommandResolution(cache, cacheKey, realGit, nowNanos); + const pending = xcodeGitLookups.get(cache) ?? new Map>(); + xcodeGitLookups.set(cache, pending); + // Registering and starting the lookup cannot be split by a yield or an interrupt, so callers + // find it as soon as it exists. Detached, a caller that stops waiting does not stop it. + const lookup = yield* Effect.uninterruptible( + Effect.suspend(() => { + const existing = pending.get(cacheKey); + if (existing !== undefined) return Effect.succeed(existing); + const shared = Deferred.makeUnsafe(); + pending.set(cacheKey, shared); + return lookupXcodeGit(env, resolveExecutable, locateXcodeGit).pipe( + Effect.tap((realGit) => + Effect.sync(() => cacheCommandResolution(cache, cacheKey, realGit, nowNanos)), + ), + Effect.onExit((exit) => + Effect.sync(() => { + if (pending.get(cacheKey) === shared) pending.delete(cacheKey); + }).pipe( + Effect.andThen(Deferred.done(shared, Exit.isSuccess(exit) ? exit : Exit.succeed(null))), + ), + ), + Effect.forkDetach, + Effect.as(shared), + ); + }), + ); + const realGit = yield* Deferred.await(lookup); return realGit ?? "git"; });