Skip to content

Commit 8e74c18

Browse files
sawenzelclaude
andcommitted
Fix gate step to include deletions, improve merge-base error handling, and clarify quoting comment
This fixes three issues in the simulation tests workflow: the gate step's git diff now includes deletions so PRs that only delete files are not incorrectly skipped, the merge-base computation is properly wrapped to catch and annotate failures, and the alienv quoting comment now clearly explains the hazard. - Gate step now uses git diff without --diff-filter=AMR to include deletions - merge-base failure is assigned to a variable first, checked, then used - Comment updated to clearly state that a space or "$" in the workspace path would break or double-evaluate Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent 9e12401 commit 8e74c18

1 file changed

Lines changed: 13 additions & 6 deletions

File tree

.github/workflows/sim-tests.yml

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@ jobs:
4646
echo "::error title=Cannot resolve diff head::pull request head commit $HEAD_SHA does not resolve in this checkout"
4747
exit 1
4848
}
49-
echo "sha=$(git merge-base "$BASE_SHA" "$HEAD_SHA")" >> "$GITHUB_OUTPUT"
49+
merge_base=$(git merge-base "$BASE_SHA" "$HEAD_SHA") || {
50+
echo "::error title=Cannot compute diff base::git merge-base failed"
51+
exit 1
52+
}
53+
[ -n "$merge_base" ] || {
54+
echo "::error title=Cannot compute diff base::merge base is empty"
55+
exit 1
56+
}
57+
echo "sha=$merge_base" >> "$GITHUB_OUTPUT"
5058
5159
- name: Skip when not relevant or opted into the source build
5260
id: gate
@@ -56,7 +64,7 @@ jobs:
5664
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
5765
run: |
5866
set -eu
59-
changed=$(git diff --diff-filter=AMR --name-only "$BASE_SHA" "$HEAD_SHA")
67+
changed=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA")
6068
if grep -qx 'test/needs-o2-dev' <<< "$changed" ; then
6169
echo "::notice title=Skipped::this pull request touches test/needs-o2-dev, so it is tested by build/O2DPG/sim/o2dev against O2 dev instead"
6270
echo "skip=true" >> "$GITHUB_OUTPUT"
@@ -115,10 +123,9 @@ jobs:
115123
run: |
116124
set -eu
117125
# Everything after "-c" is joined into one string and re-evaluated
118-
# by the CVMFS alienv via "bash -c \"$*\"", so this quoting is only
119-
# applied once here and then discarded. Safe for the runner's
120-
# actual workspace path, but a space or "$" in that path away from
121-
# breaking.
126+
# by the CVMFS alienv via "bash -c \"$*\"". Quoting here is applied
127+
# once then discarded, so it is safe for the runner's workspace path,
128+
# but a path with a space or "$" would break or double-evaluate.
122129
/cvmfs/alice.cern.ch/bin/alienv setenv "O2PDPSuite/$O2PDPSUITE_TAG" -c \
123130
env O2DPG_ROOT="$PWD" O2DPG_MC_CONFIG_ROOT="$PWD" \
124131
O2DPG_TEST_REPO_DIR="$PWD" \

0 commit comments

Comments
 (0)