Skip to content

Commit 4549bda

Browse files
l0lawrenceCopilot
andcommitted
Centralize git invocations behind git()/gitChecked() wrappers
Route every git call through thin git()/gitChecked() helpers in util.ts (the only place the 'git' literal now lives) and drop the setOriginRemote add-or-update dance in favor of configuring origin once at repo creation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent ae947dc commit 4549bda

5 files changed

Lines changed: 46 additions & 40 deletions

File tree

eng/emitter-diff/src/baseline-cache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { tmpdir } from "node:os";
44
import { join, resolve, sep } from "node:path";
55

66
import type { Logger } from "./types.js";
7-
import { ensureDir, run } from "./util.js";
7+
import { ensureDir, git } from "./util.js";
88

99
export interface BaselineCacheProfileInput {
1010
emitter?: string;
@@ -37,7 +37,7 @@ export function computeBaselineProfileKey(input: BaselineCacheProfileInput): str
3737
}
3838

3939
export async function detectBaselineIdentity(dir: string): Promise<string> {
40-
const gitHead = await run("git", ["rev-parse", "--verify", "HEAD"], { cwd: dir });
40+
const gitHead = await git(["rev-parse", "--verify", "HEAD"], { cwd: dir });
4141
const sha = gitHead.code === 0 ? gitHead.stdout.trim() : "";
4242
if (sha) return `git:${sha}`;
4343

eng/emitter-diff/src/cli.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
resolveGithubIdentity,
3434
} from "./resolver.js";
3535
import type { ClassifiedRef, EmitterConfig, Logger } from "./types.js";
36-
import { color, createLogger, ensureDir, run, runChecked } from "./util.js";
36+
import { color, createLogger, ensureDir, git, gitChecked, runChecked } from "./util.js";
3737

3838
function shouldUseBaselineCache(ciMode: boolean): { enabled: boolean; reason?: string } {
3939
if (ciMode) {
@@ -94,7 +94,7 @@ ${color.bold("Options:")}
9494
async function resolveDefaultBaselineRef(repoRoot: string): Promise<string> {
9595
// Pin the branch name from origin/HEAD (handles non-`main` defaults), falling
9696
// back to `main`.
97-
const originHead = await run("git", ["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], {
97+
const originHead = await git(["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], {
9898
cwd: repoRoot,
9999
});
100100
const branch =
@@ -255,7 +255,7 @@ async function main(): Promise<number> {
255255
if (!config) return 2;
256256

257257
// Repo root = current git working tree.
258-
const repoRoot = (await runChecked("git", ["rev-parse", "--show-toplevel"])).stdout.trim();
258+
const repoRoot = (await gitChecked(["rev-parse", "--show-toplevel"])).stdout.trim();
259259

260260
const workDir = ensureDir(values["work-dir"] ?? defaultWorkDir());
261261
log.info(`${color.dim("work dir:")} ${workDir}`);
@@ -318,11 +318,6 @@ async function main(): Promise<number> {
318318
const generatedDir = join(runDir, config.generatedCodePath);
319319
const inherit = logPrefix === undefined;
320320
if (runSetup && config.setup && config.setup.length > 0) {
321-
// Setup runs in a tree the tool materialized from GitHub. A cached worktree
322-
// is reused across runs, so a sentinel keyed on the setup commands lets us
323-
// prep it exactly once — re-running full setup (e.g. an isolated wheel
324-
// build) on an already-prepared tree is both wasteful and flaky. Changing
325-
// the setup commands changes the key and forces a re-prep.
326321
const setupKey = createHash("sha256").update(JSON.stringify(config.setup)).digest("hex");
327322
const sentinel = join(tree, ".emitter-diff-setup-done");
328323
const alreadyPrepared =

eng/emitter-diff/src/diff.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { basename, dirname, resolve } from "node:path";
99
import { pathToFileURL } from "node:url";
1010

1111
import type { Logger } from "./types.js";
12-
import { color, run } from "./util.js";
12+
import { color, git } from "./util.js";
1313

1414
export interface DiffResult {
1515
/** The unified patch text (empty when there are no differences). */
@@ -43,7 +43,7 @@ export async function diffDirs(
4343
: ["diff", "--no-index", "--no-color", "--", baselineDir, headDir];
4444

4545
// `git diff --no-index` exits 1 when there are differences — that is not an error.
46-
const result = await run("git", args, { cwd: sharesParent ? parent : undefined });
46+
const result = await git(args, { cwd: sharesParent ? parent : undefined });
4747
if (result.code > 1) {
4848
throw new Error(`git diff failed (${result.code}): ${result.stderr}`);
4949
}

eng/emitter-diff/src/resolver.ts

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { tmpdir } from "node:os";
1111
import { isAbsolute, join, resolve } from "node:path";
1212

1313
import type { ClassifiedRef, Logger } from "./types.js";
14-
import { ensureDir, run, runChecked } from "./util.js";
14+
import { ensureDir, git, gitChecked } from "./util.js";
1515

1616
/**
1717
* Classify a ref string. Explicit prefixes (`local:`, `github:`, `gh:`) are
@@ -95,7 +95,7 @@ const FALLBACK_REPO = "microsoft/typespec";
9595
async function detectOriginRepo(repoRoot: string | undefined, log: Logger): Promise<string> {
9696
if (!repoRoot) return FALLBACK_REPO;
9797
try {
98-
const res = await run("git", ["remote", "get-url", "origin"], { cwd: repoRoot });
98+
const res = await git(["remote", "get-url", "origin"], { cwd: repoRoot });
9999
if (res.code === 0) {
100100
const parsed = parseOwnerRepoFromUrl(res.stdout.trim());
101101
if (parsed) return parsed;
@@ -114,7 +114,7 @@ async function detectOriginRepo(repoRoot: string | undefined, log: Logger): Prom
114114
* origin.
115115
*/
116116
export async function getRemoteRepo(repoRoot: string, remote: string): Promise<string | undefined> {
117-
const res = await run("git", ["remote", "get-url", remote], { cwd: repoRoot });
117+
const res = await git(["remote", "get-url", remote], { cwd: repoRoot });
118118
if (res.code !== 0) return undefined;
119119
return parseOwnerRepoFromUrl(res.stdout.trim());
120120
}
@@ -199,21 +199,20 @@ async function cloneGithub(
199199
const cloneUrl = `https://github.com/${repo}.git`;
200200

201201
log.step(`Fetching ${repo}@${gitRef}`);
202-
// Shallow-init and fetch the requested ref.
203-
await runChecked("git", ["init", "-q"], { cwd: dest });
204-
// Ignore failure when origin already exists (dest dir reused from a prior fetch).
205-
await runChecked("git", ["remote", "add", "origin", cloneUrl], { cwd: dest }).catch(() => {});
206-
const fetched = await runChecked("git", ["fetch", "--depth", "1", "origin", gitRef], {
207-
cwd: dest,
208-
}).catch(() => undefined);
209-
if (fetched) {
202+
// Shallow-init and configure origin once, on first creation of this dir.
203+
if (!existsSync(join(dest, ".git"))) {
204+
await gitChecked(["init", "-q"], { cwd: dest });
205+
await gitChecked(["remote", "add", "origin", cloneUrl], { cwd: dest });
206+
}
207+
const fetched = await git(["fetch", "--depth", "1", "origin", gitRef], { cwd: dest });
208+
if (fetched.code === 0) {
210209
// Targeted fetch resolves to FETCH_HEAD.
211-
await runChecked("git", ["checkout", "-q", "FETCH_HEAD"], { cwd: dest });
210+
await gitChecked(["checkout", "-q", "FETCH_HEAD"], { cwd: dest });
212211
} else {
213212
// Some servers reject bare SHA fetches; full-fetch and checkout by ref name.
214213
// `--end-of-options` prevents treating the ref as a flag.
215-
await runChecked("git", ["fetch", "origin"], { cwd: dest });
216-
await runChecked("git", ["checkout", "-q", "--end-of-options", gitRef], { cwd: dest });
214+
await gitChecked(["fetch", "origin"], { cwd: dest });
215+
await gitChecked(["checkout", "-q", "--end-of-options", gitRef], { cwd: dest });
217216
}
218217
return dest;
219218
}
@@ -233,7 +232,7 @@ async function checkoutCachedWorktree(repo: string, gitRef: string, log: Logger)
233232

234233
ensureDir(cacheRoot);
235234
log.step(`Creating cached worktree ${repo}@${sha}`);
236-
await runChecked("git", ["worktree", "add", "--detach", dest, sha], { cwd: cacheRepo });
235+
await gitChecked(["worktree", "add", "--detach", dest, sha], { cwd: cacheRepo });
237236
return dest;
238237
}
239238

@@ -242,21 +241,19 @@ async function ensureCacheRepo(repo: string, _log: Logger): Promise<string> {
242241
const gitDir = join(repoRoot, ".git");
243242
const cloneUrl = `https://github.com/${repo}.git`;
244243

244+
// Init and configure origin once, on first creation of the cache repo.
245245
if (!existsSync(gitDir)) {
246-
await runChecked("git", ["init", "-q"], { cwd: repoRoot });
246+
await gitChecked(["init", "-q"], { cwd: repoRoot });
247+
await gitChecked(["remote", "add", "origin", cloneUrl], { cwd: repoRoot });
247248
}
248-
249-
// Ensure remote points at the expected repo, without failing if it already exists.
250-
await runChecked("git", ["remote", "add", "origin", cloneUrl], { cwd: repoRoot }).catch(() => {});
251-
await runChecked("git", ["remote", "set-url", "origin", cloneUrl], { cwd: repoRoot });
252249
return repoRoot;
253250
}
254251

255252
async function fetchAndResolveCommit(cacheRepo: string, gitRef: string): Promise<string> {
256253
// Resolve an immutable commit SHA from FETCH_HEAD.
257-
await runChecked("git", ["fetch", "--depth", "1", "origin", gitRef], { cwd: cacheRepo });
254+
await gitChecked(["fetch", "--depth", "1", "origin", gitRef], { cwd: cacheRepo });
258255
const sha = (
259-
await runChecked("git", ["rev-parse", "--verify", "FETCH_HEAD"], { cwd: cacheRepo })
256+
await gitChecked(["rev-parse", "--verify", "FETCH_HEAD"], { cwd: cacheRepo })
260257
).stdout.trim();
261258
if (!sha) {
262259
throw new Error(`Could not resolve git ref '${gitRef}' from FETCH_HEAD.`);

eng/emitter-diff/src/util.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ export interface RunResult {
4747
stderr: string;
4848
}
4949

50+
/** Options shared by {@link run}, {@link runChecked}, and the git wrappers. */
51+
export interface RunOptions {
52+
cwd?: string;
53+
env?: NodeJS.ProcessEnv;
54+
inherit?: boolean;
55+
prefix?: string;
56+
}
57+
5058
/**
5159
* Spawn a command and resolve with its captured output. Never rejects on a
5260
* non-zero exit; inspect {@link RunResult.code}. Set `inherit` to stream
@@ -55,11 +63,7 @@ export interface RunResult {
5563
* when several children run concurrently and their logs would otherwise
5664
* interleave unintelligibly. `prefix` is ignored when `inherit` is set.
5765
*/
58-
export function run(
59-
cmd: string,
60-
args: string[],
61-
opts: { cwd?: string; env?: NodeJS.ProcessEnv; inherit?: boolean; prefix?: string } = {},
62-
): Promise<RunResult> {
66+
export function run(cmd: string, args: string[], opts: RunOptions = {}): Promise<RunResult> {
6367
return new Promise((resolve, reject) => {
6468
// Only route through a shell for Windows .cmd shims (npm/pnpm/npx/code/yarn).
6569
// Native binaries like git/node are spawned directly to avoid the shell
@@ -156,7 +160,7 @@ function quoteForShell(s: string): string {
156160
export async function runChecked(
157161
cmd: string,
158162
args: string[],
159-
opts: { cwd?: string; env?: NodeJS.ProcessEnv; inherit?: boolean; prefix?: string } = {},
163+
opts: RunOptions = {},
160164
): Promise<RunResult> {
161165
const result = await run(cmd, args, opts);
162166
if (result.code !== 0) {
@@ -165,3 +169,13 @@ export async function runChecked(
165169
}
166170
return result;
167171
}
172+
173+
/** Run `git` with the given args, capturing output (never throws on non-zero). */
174+
export function git(args: string[], opts: RunOptions = {}): Promise<RunResult> {
175+
return run("git", args, opts);
176+
}
177+
178+
/** Run `git` and throw if it exits non-zero. */
179+
export function gitChecked(args: string[], opts: RunOptions = {}): Promise<RunResult> {
180+
return runChecked("git", args, opts);
181+
}

0 commit comments

Comments
 (0)