gh-107652: Fix CIFuzz build by sobolevn · Pull Request #110576 · python/cpython
Conversation
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an error with this, please could you check?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry for the error. Thank you for the fix!
| # 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 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I can verify that my C changes now trigger CIFuzz: #110573
#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?
Fix CIFuzz build
gh-107652: Fix CIFuzz build
@hugovk I guess it happened because the merge commit contained changes to the relevant files
