mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: use HEAD instead of fetching base branch for emdash linter (#26733)
## Problem The `lint/emdash` check fails on Graphite-stacked PRs. See [this failed run](https://github.com/coder/coder/actions/runs/28225390084/job/83616080375?pr=26650): ``` Base ref origin/graphite-base/26650 not found locally, fetching graphite-base/26650... ERROR: could not fetch base ref origin/graphite-base/26650. ERROR: could not determine base ref. make: *** [Makefile:768: lint/emdash] Error 1 ``` `scripts/check_emdash.sh` resolved its diff base by fetching `origin/$GITHUB_BASE_REF` and computing a merge-base. Graphite sets `GITHUB_BASE_REF` to a `graphite-base/<n>` ref that is ephemeral (it is not reliably present on origin), so the fetch fails and the check errors out instead of running. ## Fix `actions/checkout` checks out the PR **merge commit** (`refs/pull/<n>/merge`), whose **first parent (`HEAD^1`) is the exact base commit GitHub merged against**. Diffing `HEAD^1` against the checkout yields every change the PR makes against its base branch, for normal and Graphite-stacked PRs alike. No base-branch fetch, no merge-base computation, no `gh`-based deepen dance. - `scripts/check_emdash.sh`: use `HEAD^1` (the PR base commit) as the diff base in CI. Drops `resolve_merge_base` and `fetch_base_ref`. Emits a clear error if `HEAD^1` is missing (checkout too shallow). - `.github/workflows/ci.yaml`: bump the `lint` job checkout to `fetch-depth: 2` so `HEAD^1` is present with no runtime fetch. Local dev behavior (merge-base against `origin/main`) is unchanged. ## Verification - `make lint/emdash`, `make lint/shellcheck`, `make lint/actions/actionlint` pass. - Simulated the CI path with `GITHUB_BASE_REF` set: the check resolves to `HEAD^1` without fetching and still flags an added line containing an emdash. <details> <summary>Why the merge commit's first parent</summary> For a `pull_request` checkout of `refs/pull/<n>/merge`: - `HEAD` = GitHub's synthetic PR merge commit - `HEAD^1` = the exact base commit used for the merge - `HEAD^2` = the PR head commit `git diff HEAD^1 HEAD` is the full-tree diff from the base snapshot to the merged result, i.e. all of the PR's changes against its base. This is immutable and always local (given depth >= 2), unlike base branch refs which are mutable and, for Graphite stacks, ephemeral. </details> --- This PR was generated by Coder Agents on behalf of @dannykopping.
This commit is contained in:
+15
-78
@@ -39,87 +39,24 @@ scan_all_files() {
|
||||
fi
|
||||
}
|
||||
|
||||
# resolve_merge_base finds the merge-base between HEAD and the given ref.
|
||||
# In shallow CI clones the merge-base is not directly reachable, so we
|
||||
# query the PR commit count via `gh`, deepen HEAD by count+1, and
|
||||
# resolve HEAD~N which is the parent of the first PR commit.
|
||||
resolve_merge_base() {
|
||||
local base_ref="$1"
|
||||
|
||||
# Fast path: merge-base already reachable (full clone or sufficient depth).
|
||||
local mb
|
||||
mb=$(git merge-base HEAD "$base_ref" 2>/dev/null || true)
|
||||
if [[ -n "$mb" ]]; then
|
||||
echo "$mb"
|
||||
return
|
||||
fi
|
||||
|
||||
if ! command -v gh >/dev/null 2>&1; then
|
||||
echo "gh CLI not found, cannot determine PR commit count." >&2
|
||||
return
|
||||
fi
|
||||
|
||||
# Use the PR commit count to deepen HEAD past the PR commits.
|
||||
# HEAD~N is the parent of the oldest PR commit, i.e. the merge-base.
|
||||
local count
|
||||
count=$(gh pr view --json commits --jq '.commits | length' 2>/dev/null || true)
|
||||
if [[ -z "$count" || "$count" -le 0 ]]; then
|
||||
echo "Could not determine PR commit count from gh." >&2
|
||||
return
|
||||
fi
|
||||
|
||||
echo "Deepening HEAD by $((count + 1)) to reach PR base..." >&2
|
||||
git fetch --deepen="$((count + 1))" 2>/dev/null || true
|
||||
|
||||
# Retry merge-base now that we have more history.
|
||||
mb=$(git merge-base HEAD "$base_ref" 2>/dev/null || true)
|
||||
if [[ -n "$mb" ]]; then
|
||||
echo "$mb"
|
||||
return
|
||||
fi
|
||||
|
||||
# Last resort: walk first-parent history. This is correct for
|
||||
# linear PRs but may traverse the wrong branch for merge-commit
|
||||
# checkouts.
|
||||
git rev-parse --verify "HEAD~${count}" 2>/dev/null || true
|
||||
}
|
||||
|
||||
# fetch_base_ref ensures origin/$GITHUB_BASE_REF is available locally.
|
||||
# CI shallow clones (fetch-depth: 1) typically omit the base branch.
|
||||
fetch_base_ref() {
|
||||
local base_ref="$1"
|
||||
|
||||
if git rev-parse --verify "$base_ref" >/dev/null 2>&1; then
|
||||
return 0
|
||||
fi
|
||||
|
||||
local ref="${base_ref#origin/}"
|
||||
echo "Base ref $base_ref not found locally, fetching $ref..." >&2
|
||||
git fetch origin "$ref" --depth=1 2>/dev/null || true
|
||||
|
||||
if ! git rev-parse --verify "$base_ref" >/dev/null 2>&1; then
|
||||
echo "ERROR: could not fetch base ref $base_ref." >&2
|
||||
return 1
|
||||
fi
|
||||
}
|
||||
|
||||
# resolve_diff_base determines the base ref to diff against.
|
||||
# resolve_diff_base determines the base commit to diff against.
|
||||
resolve_diff_base() {
|
||||
# CI pull requests: use merge-base against the target branch.
|
||||
# CI pull requests: actions/checkout checks out the PR merge commit
|
||||
# (refs/pull/<n>/merge). Its first parent (HEAD^1) is the exact base
|
||||
# commit GitHub merged against, so diffing HEAD^1 against the checkout
|
||||
# yields every change the PR makes against its base branch. We rely on
|
||||
# this commit rather than fetching the base branch by name: branch
|
||||
# names are mutable and Graphite stacks target an ephemeral
|
||||
# graphite-base/<n> ref that may not exist on origin. Requires the
|
||||
# checkout to use fetch-depth >= 2 so HEAD^1 is present.
|
||||
if [[ -n "${GITHUB_BASE_REF:-}" ]]; then
|
||||
local base_ref="origin/${GITHUB_BASE_REF}"
|
||||
fetch_base_ref "$base_ref" || return 1
|
||||
|
||||
local base
|
||||
base=$(resolve_merge_base "$base_ref")
|
||||
if [[ -n "$base" ]]; then
|
||||
echo "$base"
|
||||
return
|
||||
if ! git rev-parse --verify --quiet "HEAD^1" >/dev/null; then
|
||||
echo "ERROR: the PR base commit (HEAD^1) is missing. Check out" >&2
|
||||
echo " the PR with fetch-depth >= 2 so the merge commit's" >&2
|
||||
echo " base parent is available." >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
# Could not determine merge-base; fall back to branch tip.
|
||||
echo "WARNING: could not find merge-base with $base_ref, using branch tip (diff may include non-PR changes)." >&2
|
||||
echo "$base_ref"
|
||||
git rev-parse "HEAD^1"
|
||||
return
|
||||
fi
|
||||
|
||||
|
||||
Reference in New Issue
Block a user