GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — gh-107652: Fix CIFuzz build by sobolevn · Pull Request #110576 · python/cpython Conversation
|
|
cc @illia-v |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| # 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.
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 PRSecond 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 an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem 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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Btw, I use https://explainshell.com/explain?cmd=grep+-qE all the time, can recommend for Thanks, everyone! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I can verify that my C changes now trigger CIFuzz: #110573 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
#109854 was created before merging CIFuzz, it changed I updated it from Do you know why? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@hugovk I guess it happened because the merge commit contained changes to the relevant files |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.