Skip to content

Update fuzzer build in CI to use oss-fuzz tooling#4132

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
lgirdwood merged 3 commits into from
May 18, 2021
Merged

Update fuzzer build in CI to use oss-fuzz tooling #4132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0002d0
actions: scant fuzzer: refactor CI build
cujomalainey May 3, 2021
59b87f7
actions: rename fuzzer job
cujomalainey May 3, 2021
3e001cb
actions: consolidate yamlint
cujomalainey May 3, 2021
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
12 changes: 8 additions & 4 deletions .github/workflows/shallow_checkpatch.yml → .github/workflows/codestyle.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,7 +9,7 @@
# github.com also has a powerful web editor that can be used without
# committing.

name: checkpatch
name: codestyle

# yamllint disable-line rule:truthy
on: [pull_request]
Expand All@@ -31,9 +31,6 @@ jobs:
- uses: actions/checkout@v2
with: {fetch-depth: 2}

- name: yamllint ourselves
run: yamllint .github/workflows/shallow_checkpatch.yml

- name: install codespell
run: sudo apt-get -y install codespell && dpkg -L codespell | grep dict

Expand All@@ -51,3 +48,10 @@ jobs:
- name: checkpatch --strict
run: .github/workflows/checkpatch_list.sh ${CHK_CMD_OPTS}
--strict < PR_SHAs.txt
yamllint:

@marc-hbmarc-hbMay 3, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered moving this to a new yamllint.yml file? With hindsight I think using one big file for everything was a Github Action beginner mistake.

@cujomalaineycujomalaineyMay 4, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have nothing against it. i just figured it would make more sense to patch sanity check tools into the same workflow "yamllint, checkpatch, pep8, etc.." thoughts? I agree things like build tools or actual builds should be separate files

@marc-hbmarc-hbMay 4, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just figured it would make more sense to patch sanity check tools into the same workflow "yamllint, checkpatch, pep8, etc.." thoughts?

Why not, in that case maybe change the titles a bit because right now this is called "checkpatch" at every level: workflow, job and what not. Warning: github actions "headings" look slightly different in merged vs unmerged PRs, have a look at one example of each.

runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2

- name: run yamllint
run: yamllint --strict .github/workflows/*.yml
3 changes: 0 additions & 3 deletions .github/workflows/installer.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,9 +23,6 @@ jobs:
# From time to time this will catch a git tag and change SOF_VERSION
with: {fetch-depth: 50, submodules: recursive}

- name: yamllint ourselves
run: yamllint .github/workflows/installer.yml

- name: docker
run: docker pull thesofproject/sof && docker tag thesofproject/sof sof

Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/ipc_fuzzer.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
---

# For the actual fuzzer see tools/oss-fuzz/README.
# also see
# https://google.github.io/oss-fuzz/getting-started/continuous-integration/
#
# Build and run fuzzer for 5s just to check that it runs properly. If it
# consistently fails in under 5s you probably did something wrong

name: IPC fuzzer compile test

# 'workflow_dispatch' allows running this workflow manually from the
# 'Actions' tab

# yamllint disable-line rule:truthy
on: [pull_request, workflow_dispatch]
Comment thread
cujomalainey marked this conversation as resolved.
Outdated

jobs:
ipc-fuzzer-build:
runs-on: ubuntu-latest
steps:
- name: Build Fuzzers
uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master
with:
oss-fuzz-project-name: 'sound-open-firmware'

- name: Run Fuzzers
uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
Comment thread
cujomalainey marked this conversation as resolved.
Outdated
with:
oss-fuzz-project-name: 'sound-open-firmware'
language: c
fuzz-seconds: 5
Comment thread
cujomalainey marked this conversation as resolved.
Outdated
13 changes: 0 additions & 13 deletions .github/workflows/pull-request.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -139,16 +139,3 @@ jobs:
PLATFORM: ${{ matrix.platform }}
run: ./scripts/docker-qemu.sh
../sof.git/scripts/qemu-check.sh ${PLATFORM}

yamllint-gh:

@cujomalaineycujomalaineyMay 18, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marc-hb im pretty sure it didnt actually run because it is looking for this job, we need update the protected branches matches

@marc-hbmarc-hbMay 18, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why the summary did not update but for sure the check itself ran. Quoting https://github.com/thesofproject/sof/pull/4132/checks?check_run_id=2606149741:

Note: switching to 'refs/remotes/pull/4132/merge'.
HEAD is now at ed6ce7a Merge b5b59e7ef0261560d51b1e164785dae62bcda41c into f510435a253373c806c49fbc32d876287bcbe801

b5b59e7ef0261 is the last commit you pushed.

@cujomalaineycujomalaineyMay 18, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is the new yamllint job I added which is codestyle / yamllint which is also showing up below. It is stuck on yamllint /.github/workflows/ which you can see is the name of the above deleted job. I think its stuck because the protected branch requires it.

@marc-hbmarc-hbMay 18, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep looking but I don't see anything stuck sorry, I only see the status not updated in the summary.

( https://github.com/thesofproject/sof/runs/2607493012?check_suite_focus=true ran again fine and the status was not updated again)

@marc-hbmarc-hbMay 18, 2021•
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I get it now: missed the difference between the new yamllint check and the old yamllint check. Only the removed check is "stuck" which is maybe only because it's getting removed and does not exist anymore. I bet this will fix itself and just go away once this PR is merged.

@marc-hbmarc-hbMay 18, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I get it now: missed the difference between the new yamllint check and the old yamllint check. Only the removed check is "stuck" which is maybe only because it's getting removed and does not exist anymore. I bet this will fix itself and just go away once this PR is merged.

Looks like it "fixed itself", the old yamllint is now gone, see for instance #4211 and others. Let's assume github struggles a bit during the transition when renaming some Actions files.

@cujomalaineycujomalaineyMay 18, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet it disappeared when i removed the rule to require it. Basically protected branch logic stopped looking for something that no longer exists

name: yamllint /.github/workflows/
runs-on: ubuntu-20.04

steps:
- uses: actions/checkout@v2

- name: apt install yamllint
run: sudo apt-get -y install yamllint

- name: yamllint github actions
run: yamllint .github/workflows/pull-request.yml
43 changes: 0 additions & 43 deletions .github/workflows/scant_fuzzer.yml
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions .github/workflows/zephyr.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,9 +15,6 @@ jobs:
# From time to time this will catch a git tag and change SOF_VERSION
with: {fetch-depth: 10, submodules: recursive}

- name: yamllint ourselves
run: yamllint .github/workflows/zephyr.yml

- name: build
run: docker run -v "$(pwd)":/workdir
docker.io/zephyrprojectrtos/zephyr-build:v0.17.3
Expand Down