Skip to content

gh-107652: Fix CIFuzz build #110576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 10, 2023
Merged

gh-107652: Fix CIFuzz build #110576

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b3a66d3
Fix CIFuzz build
sobolevn Oct 9, 2023
0348cbc
Fix CI
sobolevn Oct 9, 2023
3a5a449
Address review
sobolevn Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,15 @@ jobs:
# CPython, so CIFuzz should be run only for code that is likely to be
# merged into the main branch; compatibility with older branches may
# 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
Copy link
Member

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 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?

Suggested change
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

Copy link
Member Author

@sobolevn sobolevn Oct 9, 2023 •
edited
Loading

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

# The tests are pretty slow so they are executed only for PRs
# changing relevant files.
FUZZ_RELEVANT_FILES='(\.c$|\.h$|\.cpp$|^configure$|^\.github/workflows/build\.yml$|^Modules/_xxtestfuzz)'
git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qvE $FUZZ_RELEVANT_FILES && echo "run_cifuzz=true" >> $GITHUB_OUTPUT || true
echo "Run CIFuzz tests"
echo "run_cifuzz=true" >> $GITHUB_OUTPUT
else
echo "Branch too old for CIFuzz tests; or no C files were changed"
echo "run_cifuzz=false" >> $GITHUB_OUTPUT
fi
- name: Compute hash for config cache key
id: config_hash
Expand Down