Skip to content

Conversation

@vyevtyus
Copy link

@vyevtyus vyevtyus commented Sep 8, 2025

Description

This PR introduces workflow for Coverity scan on the main oneDAL branch. The scan is implemented for Linux platform only, but may be extended for Windows as well, if needed. Results uploading is configured into https://scan.coverity.com/projects/uxlfoundation-onedal (please, request access in advance). Taking into account the size of the project and limitations described in https://scan.coverity.com/faq#frequency, the maximum number of weekly builds for the project is 14 (2 per day), so it was decided to set up schedule (to scan once a day) instead of scanning after every push into the main branch, but it remains configurable. Also, there is a possibility to reuse this workflow, e.g. in case there are plans to include the scan into Nightly.

Patch file placed in .github/ reflects changes in Coverity configuration files, needed to reach 85% compilation units capturing (CCUs) level as a requirement of scan.coverity.com to perform analysis (there is a lack of DPCPP compiler support by Coverity analysis).

Checklist:

Completeness and readability

  • I have commented my code, particularly in hard-to-understand areas.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.

@napetrov
Copy link
Contributor

napetrov commented Sep 8, 2025

for patch -

  1. can we contribute it to coverity?
  2. if we would still have it - worth dropping it lower level ( .github/patches/coverity/ ?) and also putting explanatory .md file for why we have it and how to support it going forward?

@homksei homksei requested a review from syakov-intel September 8, 2025 15:33
Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

2 main things I would like clarified:

  • How are we going to go about testing changes in the coverity system (especially with new patches, verify functionality etc.)
  • How can we get an understanding of the issues in the patch such that it can be understood and analyzed my maintainers before merging, could we add some sort of guide what to look for here for reference?

mkdir cov-linux64-tool
tar -xzf cov-linux64-tool.tar.gz --strip 1 -C cov-linux64-tool
cd cov-linux64-tool/config
git apply --check ${GITHUB_WORKSPACE}/.github/fix.coverity-2024.12.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you talk about the maintainability of this patch? I expect this will need to be updated on a regular basis, and do we have a contingency plan for testing and verifying changes to the patch?

Copy link
Author

Choose a reason for hiding this comment

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

Taking into account low frequency of client versions update and almost zero difference between compiler configs between versions (according to our experience), I would say that we'll probably need to review the patch relevance once every 6-12 month. As for plan, currently there is no strategy on maintainability and it worth to be discussed. We may have patch application optional - in this case it's irrelevance won't broke the scan in case of tool client version update. Besides, the newer client version we use, the better Intel compiler support is implemented in it (theoretically).

vyevtyus and others added 5 commits September 12, 2025 14:44
Co-authored-by: Ian Faust <icfaust@gmail.com>
Co-authored-by: Ian Faust <icfaust@gmail.com>
Co-authored-by: Ian Faust <icfaust@gmail.com>
@vyevtyus
Copy link
Author

for patch -

  1. can we contribute it to coverity?
  2. if we would still have it - worth dropping it lower level ( .github/patches/coverity/ ?) and also putting explanatory .md file for why we have it and how to support it going forward?
  1. We've submitted several requests to vendor to have it implemented, but it takes significant time. Besides, the latest version for open source scan provided on https://scan.coverity.com/download?tab=cxx is relatively old and doesn't contain requested fixes.
  2. Added. Please, review.

schedule:
- cron: '0 21 * * *'
# allows to reuse this workflow
workflow_call:
Copy link
Contributor

Choose a reason for hiding this comment

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

Using workflow_call is the recommended way to trigger this CI step for validating updates to coverity? Still trying to figure out how we will validate new patches.

Copy link
Author

Choose a reason for hiding this comment

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

workflow_call is not a way to trigger the scan. It provides a possibility to include this scan as step into CI/Nightly if needed. If there are no such plans at the moment, we can remove this.

If someone needs or wants to test a new patch, it's enough to create a fork, make necessary changes to the patch, disable results uploading stage, and run a scan for that fork. After the build step completes, the percentage of captured compilation units, displayed at the end of the log, will indicate whether the new patch is good enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think such a pipeline could be added to this PR which checks for changes in the patch file and runs a specially triggered version of the current pipeline which doesn't upload (by additional logic checks in the current PR).

Copy link
Contributor

@icfaust icfaust left a comment

Choose a reason for hiding this comment

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

I'd like to have a guideline added to the markdown how to approve new patches and changes to this pipeline

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants