Skip to content
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.

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

Add a clang problem matcher #43222

Closed

Conversation

jbytheway
Copy link
Contributor

@jbytheway jbytheway commented Aug 25, 2020

Summary

SUMMARY: Infrastructure "Add a clang problem matcher"

Purpose of change

Partially addresses #39904.

To make it easier for contributors and reviewers to understand problems reported by CI.

Describe the solution

Add a problem matcher and the toolkit command required to activate it.

This first matcher is intended to understand clang and clang-tidy errors. I suspect it might work for gcc too.

Describe alternatives you've considered

Maintaining inscrutability.

Testing

Needs to be tested live, so I'm opening this as a draft PR to that end.

Additional context

Hope to add more for other common failure situations, but this is a start to learn how problem matchers work.

@anothersimulacrum anothersimulacrum added Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tests Measurement, self-control, statistics, balancing. labels Aug 25, 2020
@jbytheway jbytheway force-pushed the clang-tidy_problem_matcher branch 2 times, most recently from 6a68e6b to d506191 Compare August 25, 2020 18:33
@jbytheway
Copy link
Contributor Author

Looks like it's working. You can see an annotation in the diff view here. If all goes well a second annotation should appear once the clang-tidy run finishes.

@jbytheway
Copy link
Contributor Author

OK, looks like the annotation is repeated for every time the header was included. That's mildly annoying, but I guess that's how things are.

@jbytheway
Copy link
Contributor Author

For reference, the issue with duplicate annotations has been discussed here and it doesn't look like there's a simple solution.

@jbytheway
Copy link
Contributor Author

Here's a screenshot in case the example at the link goes away when I force-push again:
image

Problem matchers are a feature of GitHub actions intended to make it
easier to quickly locate the interesting part of failed CI output.

This is a first attempt to write a problem matcher for CDDA.  This one
is intended to match error messages from clang and clang-tidy (and I
think it will work for gcc too).
Clang build analyzer can also produce clang errors but doesn't use the
standard build scripts.  Enable the problem matcher there right in the
workflow yaml.
@jbytheway jbytheway force-pushed the clang-tidy_problem_matcher branch from d506191 to 8134751 Compare August 25, 2020 20:35
@jbytheway jbytheway marked this pull request as ready for review August 26, 2020 01:17
@jbytheway
Copy link
Contributor Author

I've marked this ready for review now. If we're content to risk the duplicated annotations then it's safe to merge. I have some ideas of how to reduce duplication spam somewhat if it gets bad, but I won't do them on this PR unless I get more encouragement to.

Comment on lines +1 to +18
{
"problemMatcher": [
{
"owner": "cata-clang",
"pattern": [
{
"regexp": "^(.+):(\\d+):(\\d+):\\s(error|warning):\\s(.+)\\s\\[(.+)\\]$",
"file": 1,
"line": 2,
"column": 3,
"severity": 4,
"message": 5,
"code": 6
}
]
}
]
}

Choose a reason for hiding this comment

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

You might find this problem matcher just works:
https://github.com/marketplace/actions/gcc-problem-matcher

Otherwise you might want to publish the problem matcher JSON as an action on its own (like the GCC one) so others can benefit too.

Copy link
Member

Choose a reason for hiding this comment

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

I'm mildly in favour of using the existing action, let's go that way unless you have an objection jbtw.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. I'll close this and open another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tests Measurement, self-control, statistics, balancing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants