Skip to content

Commit 9d72685

Browse files
zaniebAlexWaygood
andauthored
Use the correct base commit for change determination (#16857)
`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 <[email protected]>
1 parent 47c4ccf commit 9d72685

File tree

1 file changed

+26
-6
lines changed

1 file changed

+26
-6
lines changed

.github/workflows/ci.yaml

+26-6
Original file line numberDiff line numberDiff line change
@@ -45,55 +45,75 @@ jobs:
4545
fetch-depth: 0
4646
persist-credentials: false
4747

48+
- name: Determine merge base
49+
id: merge_base
50+
env:
51+
BASE_REF: ${{ github.event.pull_request.base.ref || 'main' }}
52+
run: |
53+
sha=$(git merge-base HEAD "origin/${BASE_REF}")
54+
echo "sha=${sha}" >> "$GITHUB_OUTPUT"
55+
4856
- name: Check if the parser code changed
4957
id: check_parser
58+
env:
59+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
5060
run: |
51-
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
61+
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
5262
echo "changed=false" >> "$GITHUB_OUTPUT"
5363
else
5464
echo "changed=true" >> "$GITHUB_OUTPUT"
5565
fi
5666
5767
- name: Check if the linter code changed
5868
id: check_linter
69+
env:
70+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
5971
run: |
60-
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
72+
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
6173
echo "changed=false" >> "$GITHUB_OUTPUT"
6274
else
6375
echo "changed=true" >> "$GITHUB_OUTPUT"
6476
fi
6577
6678
- name: Check if the formatter code changed
6779
id: check_formatter
80+
env:
81+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
6882
run: |
69-
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
83+
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
7084
echo "changed=false" >> "$GITHUB_OUTPUT"
7185
else
7286
echo "changed=true" >> "$GITHUB_OUTPUT"
7387
fi
7488
7589
- name: Check if the fuzzer code changed
7690
id: check_fuzzer
91+
env:
92+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
7793
run: |
78-
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
94+
if git diff --quiet "${MERGE_BASE}...HEAD" -- ':Cargo.toml' ':Cargo.lock' ':fuzz/fuzz_targets/**' ':.github/workflows/ci.yaml'; then
7995
echo "changed=false" >> "$GITHUB_OUTPUT"
8096
else
8197
echo "changed=true" >> "$GITHUB_OUTPUT"
8298
fi
8399
84100
- name: Check if there was any code related change
85101
id: check_code
102+
env:
103+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
86104
run: |
87-
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
105+
if git diff --quiet "${MERGE_BASE}...HEAD" -- ':**/*' ':!**/*.md' ':crates/red_knot_python_semantic/resources/mdtest/**/*.md' ':!docs/**' ':!assets/**' ':.github/workflows/ci.yaml'; then
88106
echo "changed=false" >> "$GITHUB_OUTPUT"
89107
else
90108
echo "changed=true" >> "$GITHUB_OUTPUT"
91109
fi
92110
93111
- name: Check if there was any playground related change
94112
id: check_playground
113+
env:
114+
MERGE_BASE: ${{ steps.merge_base.outputs.sha }}
95115
run: |
96-
if git diff --quiet ${{ github.event.pull_request.base.sha || 'origin/main' }}...HEAD -- ':playground/**'; then
116+
if git diff --quiet "${MERGE_BASE}...HEAD" -- ':playground/**'; then
97117
echo "changed=false" >> "$GITHUB_OUTPUT"
98118
else
99119
echo "changed=true" >> "$GITHUB_OUTPUT"

0 commit comments

Comments
 (0)