Uh oh!
There was an error while loading. Please reload this page.
Update fuzzer build in CI to use oss-fuzz tooling - #4132
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
marc-hb
left a comment
There was a problem hiding this comment.
We need the yamllint warning fixed unfortunately.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
lgirdwood
commented
May 5, 2021
Lets get this in rc2. |
gkbldcig
commented
May 10, 2021
Can one of the admins verify this patch? |
lgirdwood
commented
May 10, 2021
please test this |
cujomalainey
commented
May 10, 2021
if CI passes it works, will fixup issues this week, last week got last minute sheriff rotation that overtook my time |
lgirdwood
commented
May 12, 2021
hehe, sorry - I was speaking to the CI here as it was refusing to runt tests for some reason. |
cujomalainey
commented
May 14, 2021
Again, apologies for the delay, was blocked internally |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
marc-hb
left a comment
There was a problem hiding this comment.
One last yamllint warning
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
cujomalainey
commented
May 18, 2021
@lgirdwood looks like we will need to update the protected branches rules after this as some of the jobs might not match |
marc-hb
left a comment
There was a problem hiding this comment.
And now yamllint seems stuck in the summary, it cursed this PR!
A closer look in the "checks" tab shows it has actually run and passed, only the status is missing: https://github.com/thesofproject/sof/pull/4132/checks?check_run_id=2606149741
There was a problem hiding this comment.
@marc-hb im pretty sure it didnt actually run because it is looking for this job, we need update the protected branches matches
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I bet it disappeared when i removed the rule to require it. Basically protected branch logic stopped looking for something that no longer exists
Use oss-fuzz infra to make sure we don't break the fuzzer Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
rename scant to ipc as this is the ipc fuzzer Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
Lets not trust that everyone remembers to lint themselves or that they fetch the repo in order to execute their job. Also rename workflow file to be a bit more generic Signed-off-by: Curtis Malainey <cujomalainey@chromium.org>
cujomalainey
commented
May 18, 2021
Fixed a comment (had a double word) |
lgirdwood
commented
May 18, 2021
@cujomalainey ok, I'll merge now, and everything looks ok atm. What changes would we need to make ? |
cujomalainey
commented
May 18, 2021
@lgirdwood i fixed settings. Its under settings-> branches -> protected branches rules |
lgirdwood
commented
May 18, 2021
@cujomalainey is there anyway we can build fuzzer locally ? I've moved some code an now it doesn't build from #4202 |
cujomalainey
commented
May 18, 2021
To build the firmware locally follow this you need the oss-fuzz repo and run the commands in the repo while passing a path pointing to the SOF repo |
There was a problem hiding this comment.
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.
That was fuzzer v1 which started as a GSoC project. If found a number of bugs but was not scale-able unfortunately. Therefore I rewrote to work with oss-fuzz which is running continuous automatic fuzzing. Also note there are docs on sof-docs. |
marc-hb
commented
May 18, 2021
Should v1 be deleted or kept with a README? |
cujomalainey
commented
May 19, 2021
@ranj063 thoughts? Technically oss covers all ipc commands including topology |
ranj063
commented
May 19, 2021
@cujomalainey@marc-hb yes, Im fine with deleting or deprecating v1. |
As discussed in thesofproject#4132 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
As discussed in #4132 Signed-off-by: Marc Herbert <marc.herbert@intel.com>

Do full fuzzer build as part of the CI
Also consolidate all yamlint actions into a single job