From 9d72685f8d5df21f4ae8b14bf5a90b2a5c17ffcd Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 20 Mar 2025 08:03:03 -0500 Subject: [PATCH] Use the correct base commit for change determination (#16857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `base.sha` appears to be the commit of the base branch when the pull request was opened, not the base commit that's used to construct the test merge commit — which can lead to incorrect "determine changes" results where commits made to the base ref since the pull request are opened are included in the results. We use `git merge-base` to find the correct sha, as I don't think that GitHub provides this. They provide `merge_commit_sha` but my understanding is that is equivalent to the actual merge commit we're testing in CI. I tested this locally on an example pull request. I don't think it's worth trying to reproduce a specific situation here. --------- Co-authored-by: Alex Waygood --- .github/workflows/ci.yaml | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 418d4ce579..2cc6e82af7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -45,10 +45,20 @@ jobs: fetch-depth: 0 persist-credentials: false + - name: Determine merge base + id: merge_base + env: + BASE_REF: ${{ github.event.pull_request.base.ref || 'main' }} + run: | + sha=$(git merge-base HEAD "origin/${BASE_REF}") + echo "sha=${sha}" >> "$GITHUB_OUTPUT" + - name: Check if the parser code changed id: check_parser + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':Cargo.toml' ':Cargo.lock' ':crates/ruff_python_trivia/**' ':crates/ruff_source_file/**' ':crates/ruff_text_size/**' ':crates/ruff_python_ast/**' ':crates/ruff_python_parser/**' ':python/py-fuzzer/**' ':.github/workflows/ci.yaml'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':Cargo.toml' ':Cargo.lock' ':crates/ruff_python_trivia/**' ':crates/ruff_source_file/**' ':crates/ruff_text_size/**' ':crates/ruff_python_ast/**' ':crates/ruff_python_parser/**' ':python/py-fuzzer/**' ':.github/workflows/ci.yaml'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT" @@ -56,8 +66,10 @@ jobs: - name: Check if the linter code changed id: check_linter + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':Cargo.toml' ':Cargo.lock' ':crates/**' ':!crates/red_knot*/**' ':!crates/ruff_python_formatter/**' ':!crates/ruff_formatter/**' ':!crates/ruff_dev/**' ':!crates/ruff_db/**' ':scripts/*' ':python/**' ':.github/workflows/ci.yaml'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':Cargo.toml' ':Cargo.lock' ':crates/**' ':!crates/red_knot*/**' ':!crates/ruff_python_formatter/**' ':!crates/ruff_formatter/**' ':!crates/ruff_dev/**' ':!crates/ruff_db/**' ':scripts/*' ':python/**' ':.github/workflows/ci.yaml'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT" @@ -65,8 +77,10 @@ jobs: - name: Check if the formatter code changed id: check_formatter + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':Cargo.toml' ':Cargo.lock' ':crates/ruff_python_formatter/**' ':crates/ruff_formatter/**' ':crates/ruff_python_trivia/**' ':crates/ruff_python_ast/**' ':crates/ruff_source_file/**' ':crates/ruff_python_index/**' ':crates/ruff_python_index/**' ':crates/ruff_text_size/**' ':crates/ruff_python_parser/**' ':scripts/*' ':python/**' ':.github/workflows/ci.yaml'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':Cargo.toml' ':Cargo.lock' ':crates/ruff_python_formatter/**' ':crates/ruff_formatter/**' ':crates/ruff_python_trivia/**' ':crates/ruff_python_ast/**' ':crates/ruff_source_file/**' ':crates/ruff_python_index/**' ':crates/ruff_python_index/**' ':crates/ruff_text_size/**' ':crates/ruff_python_parser/**' ':scripts/*' ':python/**' ':.github/workflows/ci.yaml'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT" @@ -74,8 +88,10 @@ jobs: - name: Check if the fuzzer code changed id: check_fuzzer + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':Cargo.toml' ':Cargo.lock' ':fuzz/fuzz_targets/**' ':.github/workflows/ci.yaml'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':Cargo.toml' ':Cargo.lock' ':fuzz/fuzz_targets/**' ':.github/workflows/ci.yaml'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT" @@ -83,8 +99,10 @@ jobs: - name: Check if there was any code related change id: check_code + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':**/*' ':!**/*.md' ':crates/red_knot_python_semantic/resources/mdtest/**/*.md' ':!docs/**' ':!assets/**' ':.github/workflows/ci.yaml'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':**/*' ':!**/*.md' ':crates/red_knot_python_semantic/resources/mdtest/**/*.md' ':!docs/**' ':!assets/**' ':.github/workflows/ci.yaml'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT" @@ -92,8 +110,10 @@ jobs: - name: Check if there was any playground related change id: check_playground + env: + MERGE_BASE: ${{ steps.merge_base.outputs.sha }} run: | - if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':playground/**'; then + if git diff --quiet "${MERGE_BASE}...HEAD" -- ':playground/**'; then echo "changed=false" >> "$GITHUB_OUTPUT" else echo "changed=true" >> "$GITHUB_OUTPUT"