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

[context][fix] Use the property instead of the field for analyzer env #4084

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

Szelethus
Copy link
Collaborator

AnalyzerContext.__analyzer_env is lazy initialized via AnalyzerContext.analyzer_env. __parse_CC_ANALYZER_BIN became the first to use it, but erronously accessed the field directly -- which wasn't initialized at that time.

AnalyzerContext.__analyzer_env is lazy initialized via
AnalyzerContext.analyzer_env. __parse_CC_ANALYZER_BIN became the
first to use it, but erronously accessed the field directly -- which
wasn't initialized at that time.
@Szelethus Szelethus added this to the release 6.23.0-rc2 milestone Nov 15, 2023
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

Looks like it works fine based on my tests

So if CC_ANALYZERS_FROM_PATH is unset
the lookup order is
CC_ANALYZER_BIN then
package_layout.json then PATH

if CC_ANALYZERS_FROM_PATH is set
the lookup order is
CC_ANALYZER_BIN then PATH

Could you please describe this in https://github.com/Ericsson/codechecker/blob/master/docs/analyzer/user_guide.md#configuring-clang-version ?

Could you please add a few Test cases?
given:

unset CC_ANALYZERS_FROM_PATH
echo $CC_ANALYZER_BIN
gcc:/usr/bin/gcc-13

{
  "runtime": {
    "analyzers": {
      "clangsa": "/usr/bin/clang",
      "clang-tidy": "/usr/bin/clang-tidy",
      "cppcheck": "cppcheck",
      "gcc": "g++"
    },
    "clang-apply-replacements": "clang-apply-replacements"
  },
  "ld_lib_path_extra" : [],
  "path_env_extra" : []
}```

clangsa should be taken from /usr/bin/clang, the package config
gcc: gcc:/usr/bin/gcc-13 (from CC_ANALYZER_BIN)

2)

CC_ANALYZERS_FROM_PATH=1
echo $CC_ANALYZER_BIN 
gcc:/usr/bin/gcc-13
PATH=/opt/llvm/bin:$PATH

clangsa should be taken from the PATH: /opt/lllvm/bin
gcc:/usr/bin/gcc-13  (configured from CC_ANALYZER_BIN)


@Szelethus
Copy link
Collaborator Author

I agree, but that is a separate issue worthy of a separate PR.

Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

LGTM provided that you write a test case for this.

@dkrupp dkrupp merged commit 006db80 into Ericsson:master Nov 16, 2023
8 checks passed
@whisperity whisperity modified the milestones: release 6.23.0-rc2, release 6.23.0 Nov 17, 2023
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.

None yet

3 participants