Conversation
hugovk
left a comment
There was a problem hiding this comment.
There's an error with this, please could you check?
|
cc @illia-v |
illia-v
left a comment
There was a problem hiding this comment.
I'm sorry for the error. Thank you for the fix!
.github/workflows/build.yml
Outdated
| # be broken. | ||
| if [ "$GITHUB_BASE_REF" = "main" ]; then | ||
| FUZZ_RELEVANT_FILES='(\.c$|\.h$|\.cpp$|^configure$|^\.github/workflows/build\.yml$|^Modules/_xxtestfuzz)' | ||
| if [ "$GITHUB_BASE_REF" = "main" ] && [ "$(git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qvE $FUZZ_RELEVANT_FILES; echo $?)" -eq 1 ]; then |
There was a problem hiding this comment.
Do I get this logic right?
First part:
[ "$GITHUB_BASE_REF" = "main" ]- this is a PR
Second part:
-
git diff --name-only origin/$GITHUB_BASE_REF..- lists files changed compared withmain -
grep -qvE $FUZZ_RELEVANT_FILES- checks the changed files are NOT of the relevant type -
-eq 1- error code 1, so true only when the files ARE found
Instead of checking no match is false: (...; echo $?)" -eq 1
Can we check for a match, something along the lines of this?
| if [ "$GITHUB_BASE_REF" = "main" ] && [ "$(git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qvE $FUZZ_RELEVANT_FILES; echo $?)" -eq 1 ]; then | |
| if [ "$GITHUB_BASE_REF" = "main" ] && [ "$(git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qE $FUZZ_RELEVANT_FILES)" ]; then |
There was a problem hiding this comment.
When using grep -q in $(), we have to rely on exit code, because no output is produced.
So:
(.venv) ~/Desktop/cpython fix-ci-fuzz-build ✔
» echo 'abc' | grep -qE 'b'; echo $?
0
(.venv) ~/Desktop/cpython fix-ci-fuzz-build ✔
» echo 'abc' | grep -qE 'y'; echo $?
1|
Btw, I use https://explainshell.com/explain?cmd=grep+-qE all the time, can recommend for Thanks, everyone! |
|
I can verify that my C changes now trigger CIFuzz: #110573 |
|
#109854 was created before merging CIFuzz, it changed I updated it from Do you know why? |
|
@hugovk I guess it happened because the merge commit contained changes to the relevant files |
When CI job was skipped the job was failing: https://github.com/python/cpython/actions/runs/6459321247/job/17536559744?pr=110573
This happened because
run_cifuzzwas not set. Now we use the same logic as for other tools by setting it tofalseRefs #107653
Refs #107652