Skip to content

Update CI and clang-tidy for BAM support#35

Merged
mvassilev merged 1 commit into
compiler-research:developfrom
AdityaPandeyCN:workflow-patch
Mar 19, 2026
Merged

Update CI and clang-tidy for BAM support#35
mvassilev merged 1 commit into
compiler-research:developfrom
AdityaPandeyCN:workflow-patch

Conversation

@AdityaPandeyCN
Copy link
Copy Markdown

  • Add libhts-dev across CI jobs
  • Move ROOT install into clang-tidy container via install_commands
  • Disable false-positive checks for C interop (casting-through-void, reinterpret-cast, branch-clone, header-guard)
  • Fix coverage excludes

@AdityaPandeyCN
Copy link
Copy Markdown
Author

The reason to move the ROOT install into the clang-tidy container was because I suspect the old way installed ROOT on the host at /opt/root/, but the container can't see /opt/root/ as only the workspace directory is volume-mounted. So cmake failed silently, compile_commands.json was never generated, causing the headers not identified warnings.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.03%. Comparing base (3a3873a) to head (e10090c).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #35      +/-   ##
===========================================
+ Coverage    55.85%   57.03%   +1.17%     
===========================================
  Files           16       16              
  Lines         1425     1394      -31     
  Branches       752      625     -127     
===========================================
- Hits           796      795       -1     
  Misses         521      521              
+ Partials       108       78      -30     
Flag Coverage Δ
unittests 57.03% <ø> (+1.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 3 files with indirect coverage changes
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread .github/workflows/ci.yml Outdated
steps:
- uses: actions/checkout@v5

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you undo all whitespace changes ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

Comment thread .clang-tidy
-readability-function-cognitive-complexity,
-readability-implicit-bool-conversion,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-pro-type-reinterpret-cast,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The commit message does not explain why we suppress these checks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added more detail.

Add libhts-dev across CI jobs. Install ROOT inside clang-tidy Docker
container since host /opt/ is not mounted in the review container.
Fix coverage excludes and duplicate codecov token.

Suppress clang-tidy checks incompatible with htslib C interop:
- bugprone-branch-clone: false positive on switch cases with
  different signedness casts (int8_t vs uint8_t)
- bugprone-casting-through-void: htslib bam_aux_append requires
  casting typed pointers to const uint8_t* through const void*
- cppcoreguidelines-pro-type-reinterpret-cast: unavoidable when
  converting between htslib byte buffers and typed pointers
- llvm-header-guard: generates guard macros from CI container
  absolute path (/github/workspace/...) instead of project-relative

Signed-off-by: AdityaPandeyCN <adityapand3y666@gmail.com>
@AdityaPandeyCN
Copy link
Copy Markdown
Author

@vgvassilev Please have a look.

@mvassilev mvassilev merged commit 9460d22 into compiler-research:develop Mar 19, 2026
2 checks passed
@vgvassilev
Copy link
Copy Markdown

How casting-through-void, reinterpret-cast, branch-clone are false positives?

@AdityaPandeyCN
Copy link
Copy Markdown
Author

For casting-through-void , see this (#20 (comment)) was the warning but the bam_aux_append function expects const uint8_t * (https://github.com/samtools/htslib/blob/develop/htslib/sam.h#L1784) and I did static_cast through void* which caused this warning. I cannot think of any other way without this.

@vgvassilev
Copy link
Copy Markdown

For casting-through-void , see this (#20 (comment)) was the warning but the bam_aux_append function expects const uint8_t * (https://github.com/samtools/htslib/blob/develop/htslib/sam.h#L1784) and I did static_cast through void* which caused this warning. I cannot think of any other way without this.

That was a valid issue in the code. You should have fixed your code. In case of bugs we do not suppress the checks but can add a NOLINT comment for the relevant occurrence.

@AdityaPandeyCN
Copy link
Copy Markdown
Author

Apologies for this, have reverted back the suppression

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants