Skip to content

Fix detect-changed-benchmarks: handle shallow base fetch (no merge base)#1595

Merged
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/detect-changed-benchmarks-shallow-merge-base
May 30, 2026
Merged

Fix detect-changed-benchmarks: handle shallow base fetch (no merge base)#1595
ChrisRackauckas merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/detect-changed-benchmarks-shallow-merge-base

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Please ignore until reviewed by @ChrisRackauckas. Opened as a draft.

Problem

.github/scripts/detect-changed-benchmarks.sh fails the Detect Changed Benchmarks job on PR re-runs with:

fatal: origin/master...HEAD: no merge base
##[error]Process completed with exit code 128.

The PR path does:

git fetch origin "${BASE_SHA}" --depth=1 2>/dev/null || true
CHANGED_FILES=$(git diff --name-only "origin/${BASE_SHA}...HEAD" -- 'benchmarks/')

The three-dot range origin/master...HEAD diffs from the merge base of the two commits, which requires a common ancestor to exist locally. But the base is re-fetched at --depth=1 (an orphan graft with no ancestors). Once master advances past the PR's branch point, the depth=1 base tip shares no ancestor with HEAD, so git aborts with "no merge base" (exit 128). That line has no || true, so under set -eo pipefail the whole step fails.

This is why it's intermittent: a freshly-dispatched PR (base == current master tip) finds a merge base, but a re-run after master moves on does not.

Observed

Both fail at this exact line; it is latent for the whole v7-refresh batch on any re-run.

Fix

Suppress the three-dot error and fall back to diffing the PR merge commit against its first parent (HEAD~1 = base tip), which yields exactly the PR's changes without needing a merge base. This mirrors the robust 2>/dev/null || true + HEAD~1 pattern already used on the push path in the same script.

CHANGED_FILES=$(git diff --name-only "origin/${BASE_SHA}...HEAD" -- 'benchmarks/' 2>/dev/null || true)
if [[ -z "${CHANGED_FILES}" ]]; then
    CHANGED_FILES=$(git diff --name-only HEAD~1 HEAD -- 'benchmarks/' 2>/dev/null || true)
fi

Verification

Reproduced the failure locally in a scratch repo recreating the CI condition (master advanced past the merge ref's base; base re-fetched at --depth=1):

  • Old code: fatal: origin/master...HEAD: no merge base (exit 128) — matches CI exactly.
  • New code: three-dot returns empty (suppressed), fallback returns exactly the PR's changed benchmark file.

bash -n passes on the edited script.

🤖 Generated with Claude Code

detect-changed-benchmarks.sh fetches the PR base at --depth=1 and then runs
a three-dot diff (origin/master...HEAD), which requires a merge base. Once
master advances past the PR's branch point, the depth=1 base graft shares no
ancestor with HEAD, so git aborts with "fatal: origin/master...HEAD: no merge
base" (exit 128) and the Detect Changed Benchmarks job fails. This broke
re-runs of e.g. SciML#1581 and SciML#1570.

Suppress the three-dot error and fall back to diffing the PR merge commit
against its first parent (HEAD~1 = base tip), which yields exactly the PR's
changes without needing a merge base. This mirrors the existing robust pattern
already used on the push path in the same script.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@ChrisRackauckas ChrisRackauckas marked this pull request as ready for review May 30, 2026 15:06
@ChrisRackauckas ChrisRackauckas merged commit 1a5eceb into SciML:master May 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants