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

Environment initialization for binaries #4337

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Conversation

dkrupp
Copy link
Member

@dkrupp dkrupp commented Sep 5, 2024

CodeChecker calls various binaries during analysis: clang, clang-tidy,
clang-extdef-mapping etc.
If the binary is delivered within the CodeChecker package,
LD_LIBRARY_PATH needs to be extended with it.

Otherwise the environment of the caller shell should be used
for excuting binaries.

@dkrupp dkrupp force-pushed the env_fix_more branch 2 times, most recently from de11c90 to cbd8936 Compare September 10, 2024 11:49
@dkrupp dkrupp changed the title Adding environment dependent handling of the anlayzer version detection Environment initialization for binaries Sep 10, 2024
@dkrupp dkrupp force-pushed the env_fix_more branch 2 times, most recently from 1d1c218 to 9257e1c Compare September 10, 2024 12:52
CodeChecker calls various binaries during analysis: clang, clang-tidy,
clang-extdef-mapping etc.
If the binary is delivered within the CodeChecker package,
LD_LIBRARY_PATH needs to be extended with it.

Otherwise the environment of the caller shell should be used
for excuting binaries.
Don't print missing analyzer warning unless the analyzer list
is explicitly provided
@bruntib
Copy link
Contributor

bruntib commented Sep 12, 2024

Great improvement, I like the idea of not trusting the callers to provide the environment to these functions.
Some further locations should apply this approach:

ctu_autodetection.py:78
  invoke_binary_checked
  
clangtidy/analyzer.py:145
  get_warnings
  
cppcheck/analyzer.py:243
  get_analyzer_checkers
  
gcc/analyzer.py:89
  get_analyzer_checkers

codechecker_analyzer/cmd/analyzers.py:123,126
  main

@dkrupp
Copy link
Member Author

dkrupp commented Sep 12, 2024

Thanks for the review. I added the environment initialization to the additional places.

@bruntib bruntib merged commit 321492e into Ericsson:master Sep 13, 2024
8 checks passed
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.

2 participants