gh-107652: Fix CIFuzz build - #110576
Conversation
Sorry, something went wrong.
hugovk
left a comment
There was a problem hiding this comment.
There's an error with this, please could you check?
Sorry, something went wrong.
illia-v
left a comment
There was a problem hiding this comment.
I'm sorry for the error. Thank you for the fix!
Sorry, something went wrong.
| # 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 with main
-
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 |
Sorry, something went wrong.
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
Sorry, something went wrong.
hugovk
left a comment
There was a problem hiding this comment.
Thanks!
Sorry, something went wrong.
|
Btw, I use https://explainshell.com/explain?cmd=grep+-qE all the time, can recommend for bash reviews :) Thanks, everyone! |
Sorry, something went wrong.
|
I can verify that my C changes now trigger CIFuzz: #110573 |
Sorry, something went wrong.
|
#109854 was created before merging CIFuzz, it changed .pre-commit-config.yaml and Tools/patchcheck/patchcheck.py. I updated it from main, and it did the CIFuzz checks, somewhat surprisingly. Do you know why? |
Sorry, something went wrong.
|
@hugovk I guess it happened because the merge commit contained changes to the relevant files |
Sorry, something went wrong.
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_cifuzz was not set. Now we use the same logic as for other tools by setting it to false
Refs #107653
Refs #107652