From 786a1bf22d0c2b4310018c7b5f7e7f1a4e59e798 Mon Sep 17 00:00:00 2001 From: Neil Conway Date: Thu, 22 Oct 2026 14:23:39 -0400 Subject: [PATCH] ci: compare the semver check against the commit the PR was merged into On pull_request events, the semver check job checks out GitHub's test merge commit, which merges the PR into the base branch as it was when the event fired. The job then fetched the base branch again when it started and used that as the baseline. Jobs currently wait hours for a runner, and in all 59 runs from October 21 and 22, main had moved 2 to 17 commits ahead of the commit the PR was merged into. If main gains a public item in a crate that a waiting PR also changes, the check reports that item as removed by the PR. None of the reports in those runs was wrong, but nothing prevented one. Use the merge commit's first parent as the baseline instead. That is the base branch commit the PR was merged into, so the comparison covers only the PR's changes. The first parent is one generation back, so the checkout needs fetch-depth: 2 rather than the full history of every branch and tag, and the separate fetch of the base branch goes away. In a single-threaded local test, fetching a PR's merge ref at depth 2 took 0.7s of CPU and 16 MB, against 9.2s and 267 MB for the full history. The full-history checkout takes a median of 14s in CI. Tested by fetching refs/pull/26565/merge at depth 2 and running changed_crates.sh against HEAD^1. It lists the same four crates as CI's run for that PR, and semver-check reports the same removed function, register_physical_expr. --- .../workflows/breaking_changes_detector.yml | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/breaking_changes_detector.yml b/.github/workflows/breaking_changes_detector.yml index 8b33ca352662f..187a0068a0f65 100644 --- a/.github/workflows/breaking_changes_detector.yml +++ b/.github/workflows/breaking_changes_detector.yml @@ -55,27 +55,30 @@ jobs: name: Check semver runs-on: ubuntu-latest steps: + # The checkout is GitHub's test merge commit of the PR into the base + # branch. Depth 2 fetches that commit and its two parents: the base + # branch commit the PR was merged into, and the PR head. - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - fetch-depth: 0 + fetch-depth: 2 - # `origin` may point at a fork (when a contributor runs this locally) or - # at a stale ref. Fetch the base branch from the PR's upstream repo into - # a dedicated `apache/` ref so the baseline is unambiguous and the - # same ref name works locally (`git remote add apache ...`) and in CI. - - name: Fetch base branch - env: - BASE_REF: ${{ github.base_ref }} - REPO: ${{ github.repository }} - run: git fetch "https://github.com/${REPO}.git" "${BASE_REF}:refs/remotes/apache/${BASE_REF}" + # Compare against the base branch commit the PR was merged into, not the + # base branch's current tip, which may include changes the PR is not + # built on. + - name: Determine base commit + id: base + run: | + BASE_SHA=$(git rev-parse HEAD^1) + echo "sha=$BASE_SHA" >> "$GITHUB_OUTPUT" + echo "Base commit: $BASE_SHA" - name: Determine changed crates id: changed_crates env: - BASE_REF: ${{ github.base_ref }} + BASE_SHA: ${{ steps.base.outputs.sha }} run: | - PACKAGES=$(ci/scripts/changed_crates.sh changed-crates "apache/${BASE_REF}") + PACKAGES=$(ci/scripts/changed_crates.sh changed-crates "$BASE_SHA") echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT" echo "Changed crates: $PACKAGES" @@ -97,14 +100,13 @@ jobs: id: check_semver if: steps.changed_crates.outputs.packages != '' env: - BASE_REF: ${{ github.base_ref }} + BASE_SHA: ${{ steps.base.outputs.sha }} PACKAGES: ${{ steps.changed_crates.outputs.packages }} run: | set +e # `tee` lets cargo's output stream live into the Actions log # while we also keep a copy for the PR comment. - # Using `apache` remote here to point to the repository the pull request is against - ci/scripts/changed_crates.sh semver-check "apache/${BASE_REF}" $PACKAGES \ + ci/scripts/changed_crates.sh semver-check "$BASE_SHA" $PACKAGES \ 2>&1 | tee /tmp/semver-output.txt EXIT_CODE=${PIPESTATUS[0]} # Pass the result through an output instead of failing the job: