Skip to content

Detect analyzers not just in /bin for monolithic rule#233

Merged
furtib merged 5 commits into
Ericsson:mainfrom
furtib:monolithic-user-path
Jun 12, 2026
Merged

Detect analyzers not just in /bin for monolithic rule#233
furtib merged 5 commits into
Ericsson:mainfrom
furtib:monolithic-user-path

Conversation

@furtib

@furtib furtib commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Why:
When running our tests, e.g. with micromamba, analyzers are not getting found due to PATH not being passed to the monolithic rule.
This went unnoticed so far, since /bin was being added to the path in the Python script.

What:

  • Mimicking the way the codechecker binary is found, I find clang and clang-tidy too.
  • Ran ormatter on codechecker_script.py

Addresses:
Fixes: #234

@furtib furtib requested a review from Szelethus June 8, 2026 11:22
@furtib furtib self-assigned this Jun 8, 2026
@Szelethus

Copy link
Copy Markdown
Contributor

Whoa. I just verified that all this time while using micromamba, we never actually used pretty much anything from the dev.yaml file -- despite the file specifying clang 21, the metadata files indicate that it found the system clang (which is on 18.1.3) instead:

$ cat $(find -L bazel-bin -type f -name "metadata.json") | jq | grep "analyzer_statistics" -A8
          "analyzer_statistics": {
            "failed": 0,
            "failed_sources": [],
            "successful": 1,
            "successful_sources": [
              "/path/to/rules_codechecker/test/unit/config/zero_div.cc"
            ],
            "version": "18.1.3"
          }

So this is far more severe than a simple error.

@furtib furtib force-pushed the monolithic-user-path branch from 844aa16 to bc5b514 Compare June 8, 2026 13:08
@furtib furtib force-pushed the monolithic-user-path branch from bc5b514 to 841b252 Compare June 8, 2026 13:08
Comment thread src/codechecker.bzl Outdated
@furtib furtib requested a review from Szelethus June 8, 2026 14:16

@Szelethus Szelethus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
@nettle I think this is a necessary evil we should consider living with. WDYT?

Comment thread src/codechecker.bzl Outdated
Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>
nettle
nettle previously requested changes Jun 10, 2026

@nettle nettle left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is definitely not a fix.
And I'm not convinced that we should merge this workaround.
I suggest to look at line: "{codechecker_bin}": CODECHECKER_BIN_PATH,
I guess there is how the problem should be fixed or at least worked around.

@furtib furtib requested review from Szelethus and nettle June 11, 2026 12:04
@furtib furtib force-pushed the monolithic-user-path branch from d8f021e to 5be6409 Compare June 11, 2026 12:06
@furtib furtib force-pushed the monolithic-user-path branch from 5be6409 to 6c79405 Compare June 11, 2026 12:10

@Szelethus Szelethus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cheers @nettle for the input, this ended up looking much more ideal of a workaround.

I tested it out, analyzer version is correctly showing what micromamba set up!

bazel clean --expunge
source .ci/micromamba/init.sh
bazel test ...
cat $(find -L bazel-bin -type f -name "metadata.json") | jq | grep "\"version\""

@furtib furtib dismissed nettle’s stale review June 12, 2026 13:04

Have made the requested changes.

@furtib furtib merged commit 311cfe9 into Ericsson:main Jun 12, 2026
15 checks passed
@furtib furtib deleted the monolithic-user-path branch June 12, 2026 13:04
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.

Monolithic rule uses analyzers only from /bin

3 participants