mirror of
https://github.com/stablyai/orca.git
synced 2026-10-01 00:02:10 +00:00
perf(ci): diff against the merge commit's first parent so PR checkouts can be shallow (#23562)
Every changed-path gate asked git for `--merge-base "$BASE_SHA" "$HEAD_SHA"`, which needs the event payload's base SHA to be in the local graph. That is the only reason two jobs cloned all 8127 refs' history. On a pull_request checkout HEAD is already the merge commit, so its first parent is the base side and no merge base has to be computed. config/scripts/git-pull-request-diff-base.mjs resolved that for the two Node gates; the workflow's inline gates now use the same helper through a small CLI rather than open-coding it. code_paths gates all 22 jobs, so its checkout is charged to the start of every one of them: measured 20.7s to 1.6s, keeping blob:none because its sparse tree is ~7 files and leaves no blobs to refetch. Static analysis drops the filter instead, since populating all 30,226 files makes blob:none force a second promisor fetch: 23s to ~11s. Verified on a real merge ref. At depth 50 the old and new forms produce identical changed-file sets. At depth 2 the new form still works and the old one fails with `fatal: bad object`, which is the failure a stale base would have caused once the checkout stopped being complete. Also drops the dead resolveBase + merge-base prelude in the changed-code gate, whose result resolvePullRequestDiffBase already discarded on every PR.
This commit is contained in:
+30
-22
@@ -59,12 +59,15 @@ jobs:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
# Why blob:none: full history is needed for the merge-base diff, but historical
|
||||
# file contents are not. Blobs are ~89% of this repo's pack, and Git fetches the
|
||||
# few this job actually reads on demand.
|
||||
fetch-depth: 0
|
||||
# Why depth 50 and not 0: every diff below resolves to HEAD^1, so only the merge commit
|
||||
# and a little slack are needed. Fetching all 8127 refs' commit graph cost ~20s here and
|
||||
# is charged to the start of all 22 jobs, since they all need this one.
|
||||
# Why blob:none stays: the sparse tree below is ~7 files, so there are no blobs to
|
||||
# materialize and no promisor refetch. Measured 9.5s -> 1.6s against 20.7s today.
|
||||
fetch-depth: 50
|
||||
filter: blob:none
|
||||
sparse-checkout: |
|
||||
/config/scripts/git-pull-request-diff-base.mjs
|
||||
/package.json
|
||||
/README.md
|
||||
/docs/readme/
|
||||
@@ -79,8 +82,9 @@ jobs:
|
||||
- name: Reject new root-level files and folders
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: node .github/scripts/check-root-directory-entries.mjs "$BASE_SHA" "$HEAD_SHA"
|
||||
run: |
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA")"
|
||||
node .github/scripts/check-root-directory-entries.mjs "$DIFF_BASE" HEAD
|
||||
|
||||
# The full Git index retains link targets outside the sparse working tree.
|
||||
- name: Check README local links
|
||||
@@ -99,12 +103,15 @@ jobs:
|
||||
id: filter
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
# Why HEAD^1 and not --merge-base: HEAD is the pull request merge commit, so its first
|
||||
# parent is the base side already. Computing a merge base instead would require the
|
||||
# payload base SHA to be in the graph, which is what forced a full-history checkout.
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA")"
|
||||
# Why --no-renames: name-only rename detection can report only the destination.
|
||||
# A code file moved under docs/ must still expose its code-side deletion.
|
||||
CHANGED="$(git diff --name-only --no-renames --diff-filter=ACDMR --merge-base "$BASE_SHA" "$HEAD_SHA")"
|
||||
CHANGED="$(git diff --name-only --no-renames --diff-filter=ACDMR "$DIFF_BASE" HEAD)"
|
||||
echo "Changed paths:"
|
||||
printf '%s\n' "$CHANGED"
|
||||
printf '%s\n' "$CHANGED" | node config/scripts/pr-code-change-scope.mjs | tee -a "$GITHUB_OUTPUT"
|
||||
@@ -116,8 +123,8 @@ jobs:
|
||||
run: |
|
||||
set -euo pipefail
|
||||
BASE="${{ github.event.pull_request.base.sha }}"
|
||||
HEAD="${{ github.event.pull_request.head.sha }}"
|
||||
CHANGED="$(git diff --name-only --diff-filter=AMCR --merge-base "$BASE" "$HEAD")"
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE")"
|
||||
CHANGED="$(git diff --name-only --diff-filter=AMCR "$DIFF_BASE" HEAD)"
|
||||
# Source routes are executable contracts so a test can prove exact
|
||||
# authorities, exclusions, and sentinels without evaluating workflow shell.
|
||||
TEST_FILES_JSON="$(printf '%s\n' "$CHANGED" | node config/scripts/pr-e2e-source-routing.mjs)"
|
||||
@@ -131,7 +138,7 @@ jobs:
|
||||
# trigger on IME source rather than on a spec name in some route's list.
|
||||
NATIVE_IME_SOURCE_CHANGED="$(printf '%s\n' "$CHANGED" | node config/scripts/pr-e2e-source-routing.mjs --native-ime-source)"
|
||||
echo "native_ime_source_changed=$NATIVE_IME_SOURCE_CHANGED" >> "$GITHUB_OUTPUT"
|
||||
WSL_CHANGED="$(git diff --name-only --no-renames --diff-filter=ACDMR --merge-base "$BASE" "$HEAD")"
|
||||
WSL_CHANGED="$(git diff --name-only --no-renames --diff-filter=ACDMR "$DIFF_BASE" HEAD)"
|
||||
WSL_SOURCE_CHANGED="$(printf '%s\n' "$WSL_CHANGED" | node config/scripts/pr-e2e-source-routing.mjs --wsl-source)"
|
||||
echo "wsl_source_changed=$WSL_SOURCE_CHANGED" >> "$GITHUB_OUTPUT"
|
||||
echo "Native IME source changed: $NATIVE_IME_SOURCE_CHANGED"
|
||||
@@ -154,11 +161,12 @@ jobs:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v6
|
||||
with:
|
||||
# Why blob:none: full history is needed for the merge-base diff, but historical
|
||||
# file contents are not. Blobs are ~89% of this repo's pack, and Git fetches the
|
||||
# few this job actually reads on demand.
|
||||
fetch-depth: 0
|
||||
filter: blob:none
|
||||
# Why depth 50: the gates below diff against HEAD^1, so no merge base is computed and
|
||||
# the payload base SHA need not be in the graph.
|
||||
# Why no blob:none here, unlike code_paths: this job checks out all 30,226 files, and
|
||||
# the filter then forces a second promisor fetch of nearly every blob. Measured 23s
|
||||
# blobless against ~11s without it.
|
||||
fetch-depth: 50
|
||||
persist-credentials: false
|
||||
|
||||
# Why two guarded installs: the mixed root+mobile store entry is 537 MB against
|
||||
@@ -228,9 +236,9 @@ jobs:
|
||||
- name: Check VM runtime rollback compatibility
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
if git diff --quiet --merge-base "$BASE_SHA" "$HEAD_SHA" -- \
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA")"
|
||||
if git diff --quiet "$DIFF_BASE" HEAD -- \
|
||||
src/shared/ephemeral-vm-runtime-store.ts \
|
||||
src/shared/ephemeral-vm-runtime-feature-store.ts \
|
||||
src/shared/ephemeral-vm-runtime-rollback-projection.ts \
|
||||
@@ -262,10 +270,10 @@ jobs:
|
||||
background: true
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
run: |
|
||||
# Detection failures run the full check; renames retain the removed input path.
|
||||
if git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA" > "$RUNNER_TEMP/localization-changes" &&
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA")"
|
||||
if git diff --name-only --no-renames -z "$DIFF_BASE" HEAD > "$RUNNER_TEMP/localization-changes" &&
|
||||
scope="$(node config/scripts/localization-extraction-change-scope.mjs "$RUNNER_TEMP/localization-changes")" && [ "$scope" = false ]; then
|
||||
echo "Localization extraction inputs are unchanged."
|
||||
else
|
||||
@@ -278,11 +286,11 @@ jobs:
|
||||
- name: Boot orcad and round-trip a terminal
|
||||
env:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
ORCA_BACKGROUND_LAUNCH: '1'
|
||||
run: |
|
||||
# Detection failures run the smoke; renames retain the removed input path.
|
||||
if git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA" > "$RUNNER_TEMP/orcad-smoke-changes" &&
|
||||
DIFF_BASE="$(node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA")"
|
||||
if git diff --name-only --no-renames -z "$DIFF_BASE" HEAD > "$RUNNER_TEMP/orcad-smoke-changes" &&
|
||||
scope="$(node config/scripts/orcad-terminal-smoke-change-scope.mjs "$RUNNER_TEMP/orcad-smoke-changes")" && [ "$scope" = false ]; then
|
||||
echo "Orcad terminal smoke inputs are unchanged."
|
||||
else
|
||||
|
||||
@@ -136,9 +136,14 @@ function resolveBase(root, requestedBase) {
|
||||
}
|
||||
|
||||
export function collectAddedLineRanges(root, requestedBase) {
|
||||
const base = resolveBase(root, requestedBase)
|
||||
const mergeBase = runGit(root, ['merge-base', base, 'HEAD']).trim()
|
||||
const comparisonBase = resolvePullRequestDiffBase(root, mergeBase)
|
||||
// On a pull_request checkout HEAD is the merge commit, so its first parent is the base side and
|
||||
// no merge base has to be computed. Resolving it first is what lets CI checkout shallowly: the
|
||||
// payload base SHA can lag HEAD^1 by any number of commits and need not be in the graph at all.
|
||||
// Off that ref (local runs) the requested base is an arbitrary branch tip, so the merge base is
|
||||
// still what isolates this branch's own lines.
|
||||
const comparisonBase =
|
||||
resolvePullRequestDiffBase(root, null) ??
|
||||
runGit(root, ['merge-base', resolveBase(root, requestedBase), 'HEAD']).trim()
|
||||
const changedFiles = splitNullDelimited(
|
||||
runGit(root, ['diff', '--name-only', '-z', '--diff-filter=ACMRTUB', comparisonBase, '--'])
|
||||
)
|
||||
@@ -174,7 +179,7 @@ export function collectAddedLineRanges(root, requestedBase) {
|
||||
const lineCount = readFileSync(absolutePath, 'utf8').split(/\r?\n/).length
|
||||
rangesByFile.set(file, [{ start: 1, end: lineCount }])
|
||||
}
|
||||
return { base, comparisonBase, rangesByFile }
|
||||
return { comparisonBase, rangesByFile }
|
||||
}
|
||||
|
||||
function parseOxlintOutput(stdout, label) {
|
||||
@@ -414,10 +419,12 @@ export function main(
|
||||
root = process.cwd(),
|
||||
requestedBase = process.argv.slice(2).find((argument) => argument !== '--')
|
||||
) {
|
||||
const { base, comparisonBase, rangesByFile } = collectAddedLineRanges(root, requestedBase)
|
||||
const { comparisonBase, rangesByFile } = collectAddedLineRanges(root, requestedBase)
|
||||
const files = [...rangesByFile.keys()]
|
||||
if (files.length === 0) {
|
||||
console.log(`Changed-code quality gate: no changed JavaScript or TypeScript since ${base}.`)
|
||||
console.log(
|
||||
`Changed-code quality gate: no changed JavaScript or TypeScript since ${comparisonBase.slice(0, 12)}.`
|
||||
)
|
||||
return 0
|
||||
}
|
||||
|
||||
|
||||
@@ -228,8 +228,14 @@ describe('root directory guard', () => {
|
||||
|
||||
expect(guardJob.if).toBeUndefined()
|
||||
expect(guardStep.if).toBeUndefined()
|
||||
expect(guardJob.steps[0].with['fetch-depth']).toBe(0)
|
||||
// Why >= 2 rather than 0: the guard compares the merge commit's first parent against the
|
||||
// merged tree, so it needs both parents present but no history beyond them. Depth 1 would
|
||||
// leave HEAD^1 unreachable and the guard would fail closed on every run.
|
||||
expect(guardJob.steps[0].with['fetch-depth']).toBeGreaterThanOrEqual(2)
|
||||
expect(guardStep.run).toContain('node .github/scripts/check-root-directory-entries.mjs')
|
||||
// The base side must come from the merge ref, not the event payload, or a shallow checkout
|
||||
// cannot resolve it.
|
||||
expect(guardStep.run).toContain('git-pull-request-diff-base.mjs')
|
||||
expect(workflow.jobs.verify.needs).toContain('code_paths')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import process from 'node:process'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
export function selectPullRequestDiffBase(requestedBase, headParents, eventName) {
|
||||
if (eventName === 'pull_request' && headParents.length >= 2) {
|
||||
@@ -21,3 +22,15 @@ export function resolvePullRequestDiffBase(
|
||||
.split(/\s+/)
|
||||
return selectPullRequestDiffBase(requestedBase, headParents, eventName)
|
||||
}
|
||||
|
||||
// Why a CLI: the workflow's inline gates used `git diff --merge-base "$BASE_SHA" "$HEAD_SHA"`,
|
||||
// which needs the payload base to be reachable and so forced a full-history checkout. Printing
|
||||
// the resolved base lets those steps diff against `HEAD^1` with the same logic the Node gates
|
||||
// already use, instead of each one open-coding it.
|
||||
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
|
||||
const resolved = resolvePullRequestDiffBase(process.cwd(), process.argv[2])
|
||||
if (!resolved) {
|
||||
throw new Error('No diff base: pass the pull request base SHA, or run on a merge ref.')
|
||||
}
|
||||
process.stdout.write(`${resolved}\n`)
|
||||
}
|
||||
|
||||
@@ -43,12 +43,13 @@ it('preserves deleted and renamed inputs and falls back to extraction on detecti
|
||||
(candidate) => candidate.name === 'Verify localization extraction'
|
||||
)
|
||||
expect(step.env).toEqual({
|
||||
BASE_SHA: '${{ github.event.pull_request.base.sha }}',
|
||||
HEAD_SHA: '${{ github.event.pull_request.head.sha }}'
|
||||
BASE_SHA: '${{ github.event.pull_request.base.sha }}'
|
||||
})
|
||||
expect(step.run).toContain(
|
||||
'git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA"'
|
||||
)
|
||||
// The base side comes from the merge ref's first parent, so the gate needs no merge base and
|
||||
// works on a shallow checkout. The payload head SHA is no longer read: HEAD is the merged tree
|
||||
// the gate is actually deciding about.
|
||||
expect(step.run).toContain('node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA"')
|
||||
expect(step.run).toContain('git diff --name-only --no-renames -z "$DIFF_BASE" HEAD')
|
||||
expect(step.run).not.toContain('--diff-filter')
|
||||
expect(step.run).toContain('&& [ "$scope" = false ]; then')
|
||||
expect(step.run).toMatch(/else\s+pnpm run verify:localization-extraction\s+fi/)
|
||||
|
||||
@@ -97,12 +97,12 @@ it('only skips the unchanged smoke after a successful diff and dependency analys
|
||||
)
|
||||
expect(step.env).toEqual({
|
||||
BASE_SHA: '${{ github.event.pull_request.base.sha }}',
|
||||
HEAD_SHA: '${{ github.event.pull_request.head.sha }}',
|
||||
ORCA_BACKGROUND_LAUNCH: '1'
|
||||
})
|
||||
expect(step.run).toContain(
|
||||
'git diff --name-only --no-renames -z --merge-base "$BASE_SHA" "$HEAD_SHA"'
|
||||
)
|
||||
// See the localization gate: HEAD^1 removes the merge-base computation, so a shallow checkout
|
||||
// is enough and the payload head SHA is unused.
|
||||
expect(step.run).toContain('node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA"')
|
||||
expect(step.run).toContain('git diff --name-only --no-renames -z "$DIFF_BASE" HEAD')
|
||||
expect(step.run).not.toContain('--diff-filter')
|
||||
expect(step.run).toContain('&& [ "$scope" = false ]; then')
|
||||
expect(step.run).toMatch(/else\s+pnpm run smoke:orcad-terminal\s+fi/)
|
||||
|
||||
@@ -530,7 +530,11 @@ describe('PR Checks skip wiring', () => {
|
||||
)
|
||||
expect(classify.run).toContain('--diff-filter=ACDMR')
|
||||
expect(classify.run).toContain('--no-renames')
|
||||
expect(classify.run).toContain('--merge-base "$BASE_SHA" "$HEAD_SHA"')
|
||||
// HEAD is the merge commit, so HEAD^1 is the base side and no merge base is computed.
|
||||
// That is what lets this job check out shallowly, which every other job waits on.
|
||||
expect(classify.run).toContain('node config/scripts/git-pull-request-diff-base.mjs "$BASE_SHA"')
|
||||
expect(classify.run).toContain('"$DIFF_BASE" HEAD')
|
||||
expect(classify.run).not.toContain('--merge-base "$')
|
||||
expect(classify.run).toContain('node config/scripts/pr-code-change-scope.mjs')
|
||||
expect(classify.run).toContain('tee -a "$GITHUB_OUTPUT"')
|
||||
expect(prWorkflow.jobs.code_paths.outputs.should_run).toBe(
|
||||
|
||||
@@ -238,7 +238,7 @@ describe('PR E2E gate contract', () => {
|
||||
})
|
||||
|
||||
it('scopes detection to the PR range so base drift cannot false-trigger', () => {
|
||||
expect(filterStep.run).toContain('--merge-base "$BASE" "$HEAD"')
|
||||
expect(filterStep.run).toMatch(/diff-base\.mjs "\$BASE"[\s\S]*"\$DIFF_BASE" HEAD/)
|
||||
expect(filterStep.run).toContain('set -euo pipefail')
|
||||
})
|
||||
|
||||
@@ -444,7 +444,7 @@ describe('PR E2E gate contract', () => {
|
||||
})
|
||||
|
||||
it('scopes the VM rollback oracle to the PR range and recipe schema authorities', () => {
|
||||
expect(rollbackStep.run).toContain('--merge-base "$BASE_SHA" "$HEAD_SHA"')
|
||||
expect(rollbackStep.run).toMatch(/diff-base\.mjs "\$BASE_SHA"[\s\S]*"\$DIFF_BASE" HEAD --/)
|
||||
expect(rollbackStep.run).toContain('src/shared/ephemeral-vm-recipes.ts')
|
||||
expect(rollbackStep.run).toContain('src/shared/orca-yaml-hook-types.ts')
|
||||
expect(selectPrE2eSpecs(['src/shared/ephemeral-vm-recipes.ts'])).toEqual([
|
||||
|
||||
Reference in New Issue
Block a user