Skip to content

[SKIP CI] sparse: temporarily ignore MTL address errors and add to daily tests #6722

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

Closed
marc-hb wants to merge 2 commits into from
Closed

[SKIP CI] sparse: temporarily ignore MTL address errors and add to daily tests #6722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e027b2
parse_sparse_output.sh: temporarily ignore MTL address space errors
marc-hb Dec 6, 2022
44813d1
.github/daily-tests.yml: add sparse-zephyr.yml
marc-hb Dec 6, 2022
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
2 changes: 2 additions & 0 deletions .github/workflows/daily-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ jobs:
uses: ./.github/workflows/pull-request.yml
run-zephyr-builds:
uses: ./.github/workflows/zephyr.yml
sparse-zephyr:
uses: ./.github/workflows/sparse-zephyr.yml
5 changes: 4 additions & 1 deletion scripts/parse_sparse_output.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ main()

sparse_errors+=(-e '[[:space:]]error:[[:space:]]')

sparse_errors+=(-e '[[:space:]]warning:[[:space:]].*different address space')
# TODO: fix mtl source code
if test 'mtl' != "$platform"; then
sparse_errors+=(-e '[[:space:]]warning:[[:space:]].*different address space')
fi

! grep -v 'alsatplg.*topology2.*skip' | grep -i "${sparse_errors[@]}"

Copy link
Collaborator

@lyakh lyakh Dec 6, 2022

Choose a reason for hiding this comment

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

this means, we will see complete MTL sparse output including "address space" warnings in CI, it just won't be a failure? Could you maybe remove "SKIP CI" to let us see what it will look like?

Copy link
Collaborator Author

@marc-hb marc-hb Dec 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

this means, we will see complete MTL sparse output including "address space" warnings in CI, it just won't be a failure?

Yes, see link in comment above and in the "Checks" box below.

Could you maybe remove "SKIP CI"

Only sof-ci/jenkins reads "SKIP CI", Github does not care. Again look at the "Details" box below.

Copy link
Collaborator

@lyakh lyakh Dec 7, 2022

Choose a reason for hiding this comment

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

Indeed, thanks! I'm not sure whether it's a good idea to green-wash our failures, but as long as we have the output, I personally can live with it

Copy link
Collaborator Author

@marc-hb marc-hb Dec 7, 2022

Choose a reason for hiding this comment

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

I don't like it either but it's been red for too long now. I also made sure it's very easy to revert.

Can you approve then?

Copy link
Member

@lgirdwood lgirdwood Dec 7, 2022

Choose a reason for hiding this comment

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

MTL errors will be fixed after v2.4 is tagged - lets fix instead of hiding. The other patches are fine in this PR.

Copy link
Collaborator Author

@marc-hb marc-hb Dec 8, 2022

Choose a reason for hiding this comment

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

@lgirdwood re-submitting the other patches is not an issue but does anyone have any idea how to fix the current warnings on MTL and a line of sight on a fix? Right now we are actively training users to ignore the results on MTL and we are blind to possibly WORSE MTL regressions.

Copy link
Member

@lgirdwood lgirdwood Dec 9, 2022

Choose a reason for hiding this comment

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

Will be fixed after v2.4 is tagged, IOW early next week.

Expand Down