gh-94759: Collect C-level coverage using llvm-cov

[mdboom]

This adds a Github Action to collect C-level coverage and publish it to a Github Pages repository.

You can see an example coverage output.

There are two admin things that would need to happen for this to work:

• Create a new repository in the python organization for the results, e.g. cpython-c-coverage
• Create an organization-scoped token to publish to this repository and use it from this action

• Issue: Publish C-level coverage of the CPython test suite #94759

[gpshead]

This will be great to have! Publishing to github pages is interesting and is as good a place to start as any.

It is unfortunate that GitHub doesn't support rendering coverage metadata on top of their existing source tree view and within PR code views.

[gpshead]

Does every run clobber the previous run's report? What about CI runs on PRs and CI runs from release branches? Can this be configured to commit the coverage results into a branch in the repo matching the reponame+branchname? Or do branches not render in gh-pages? in which case it'd need to be subdirectories in the repo.

[mdboom]

Currently, this clobbers the previous run. That is easily changed with a flag, but then we could run into Github repository size limits (each result is around 100MB of HTML). It's probably possible to keep the last N commits, but the tool I'm using to publish doesn't support that directly.

This is currently configured to just run on the main branch once a day. We could do multiple branches that publish to subdirectories if you think there's a good use case for that. (Github pages only publishes a single branch, but subdirectories would work).

[tiran]

If we would get some funding to run a persistent VM with public hostname from ... let's say Azure, then it would be trivial to create a buildbot worker and serve the LCOV results from HTTP server. They are static HTTP and JS files on the file system after all. Just saying :)

[gpshead]

I don't mind main branch only and once a day. Anyone working on coverage is presumably doing their own local coverage runs while creating PRs.

Regarding a buildbot configured to host the results, while I could simply set one up it'd probably make more sense for mdboom to do that and be an admin given who's driving this work. :)

[mdboom]

Yeah, I'd be happy to admin that if it comes to it. I think it's fine to go with the Github Action here as an MVP, and if the amount of history or frequency of runs isn't good enough, we can revisit migrating to our own VM down the road.

[arhadthedev]

What is the benefit to run the suit daily? If the report is used as a guide for additions into the tests, a weekly run seems to be enough.

[brettcannon]

What is the benefit to run the suite daily? If the report is used as a guide for additions into the tests, a weekly run seems to be enough.

If it's daily then it's up-to-date more than the weekly. More up-to-date is better than less up-to-date 😉. There's also the benefit that people will have a better idea of what to work on in terms of coverage than not as someone may put in a lot of work for something that's already been fixed earlier in the week.

Not to say that a weekly coverage run isn't good as well, but daily is just better if we can make it happen.

[tiran]

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

[gpshead]

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

Which one produces more useful output? (I really don't care which we use even though everything I do at work is clang/llvm based - I just want line and branch coverage data)

[brettcannon]

@tiran the idea of using clang comes from faster-cpython/ideas#426 where I believe branch coverage was considered the big win.

Maybe coverage-report should get updated to use LLVM? Or make it configurable for either clang or gcc?

[tiran]

GCC and LCOV can do branch coverge, too. I'll have a branch ready by tomorrow.

I would welcome to have LLVM coverage support in Makefile, too.

[mdboom]

Is there any particular reasons why you are using clang and custom instructions for LCOV instead of make coverage-report? The latter uses gcc and lcov tool.

It was an attempt to get branch coverage, not realizing lcov could do it. I still think llvm-cov displays branches within subexpressions in a more readable way, however. I don't know if using gcov for collection + llvm-cov for output would give us the best of both worlds. (Using gcc for the collection would be preferable, probably, since that's what we test with on Linux).

[mdboom]

I don't know if using gcov for collection + llvm-cov for output would give us the best of both worlds.

It doesn't look like this is possible. There is an llvm-cov gcov command that takes the gcc coverage data as input and outputs human-readable text file reports. But I can't find a way to use gcc coverage data and output the nice llvm-cov html reports.

@brettcannon wrote:

Maybe coverage-report should get updated to use LLVM? Or make it configurable for either clang or gcc?

@tiran wrote:

I would welcome to have LLVM coverage support in Makefile, too.

I think that's a good idea, but a fair chunk of fiddly work. The Makefile doesn't have any direct support for building with clang on Linux. It works if you set CC, obviously, but you have to do that prior to re-running configure. So I don't think it's possible to just have make coverage-report-llvm work (since the Makefile can't call configure). We could, probably, make a task that errors out if clang isn't being used.

[brettcannon]

It works if you set CC, obviously, but you have to do that prior to re-running configure

Can you have the coverage-report target do the right thing based on what CC was set to?

[mdboom]

I just refactored this so there is a new Makefile target coverage-report-llvm that uses llvm-cov to generate a coverage report -- and then the Github Action just uses that.

Can you have the coverage-report target do the right thing based on what CC was set to?

I hadn't thought of this -- it's probably doable. Given that you have to completely reconfigure to switch between each "mode" that probably makes sense.

[mdboom]

I've updated this to automatically choose lcov or llvm-cov depending on the compiler in use.

[mdboom]

@erlend-aasland wrote:

Also: please don't force push :) The GitHub UI don't play well with force-pushes. Using git merge --no-ff main makes for a better review experience.

I tried this and unfortunately it created a bunch of co-authored commits for everything that had happened in the meantime and then requested review from ~20 codeowners. I went back to rebasing/force pushing for a cleaner review of the actually relevant commits. Going to read the relevant parts of the devguide for next time...

[mdboom]

Are there any additional comments on this?

[encukou]

Create a new repository in the python organization for the results, e.g. cpython-c-coverage

There's a process for this now.
I don't think this repo needs to start under a personal GitHub account -- moving the config and creating a new token would be more trouble than it's worth. That leaves asking the SC as the next step, see here for example: python/steering-council#236

Adding LLVM support to coverage-report would work better as a separate PR, but I won't block on that.