codechecker icon indicating copy to clipboard operation
codechecker copied to clipboard

Configurability of analyzer versions

Open cservakt opened this issue 2 years ago • 3 comments

The modification can be used to validate the version of each analyzer. The user can specify all desired analyzers after the --analyzers flag, as was possible before. Also, the exact version number can be typed for each analyzer, separated by an '==' char. For example: 'CodeChecker analyze compile_commands.json -o reports --analyzers clangsa==14.0.0 cppcheck==2.7'. If the version number was not the same as in the current environment, error message would be logged and the system exit would trigger. The user can enumerate the analyzers with or without versions, for example: 'CodeChecker analyze compile_commands.json -o reports --analyzers clangsa=14.0.0 cppcheck'.

cservakt avatar Aug 28 '23 13:08 cservakt

I'm a big fan of hard errors in favour of warnings. CodeChecker has a lot of output, and these warnings are hard to miss. Also, if I explicitly request an analyzer on a specific version, and the specified version is not found, I might prefer to fix that before sinking countless hours into an analysis.

This idea might clash with the philosophy we have with clang-tidy not having a single checker enabled (in which case we actually prefer a warning to a hard error).

Also, there migth be several versions of the binary available from config files or PATH, but we we don't try to look for them, is that correct?

I think the warning message is sufficient in this case. We print similar message when other analyzer problems occur, for example, when the binary of one analyzer is not found, but the others are.

cservakt avatar Aug 30 '23 09:08 cservakt

I don't intend to die on this hill, but I'm not yet convinced that a soft warning is the way to go. Breaking existing functionality is never a step to be taken lightly, but this is a new feature, we actually can be quite strict.

I foresee users setting an explicit version of the analyzer trying to achieve a reproducable analysis across different machines. I don't really see the scenario where the desired behaviour would be to just (almost) silently disable it and go on.

Also, you haven't commented on the other idea, checking whether the analyzer with the requested version is available from the path. I suspect we only get hold of a single binary and look no further. This could be especially cool for people that need to have multiple compiler versions on hand (which is rather common among C++ developers).

Szelethus avatar Aug 31 '23 14:08 Szelethus

I don't intend to die on this hill, but I'm not yet convinced that a soft warning is the way to go. Breaking existing functionality is never a step to be taken lightly, but this is a new feature, we actually can be quite strict.

I foresee users setting an explicit version of the analyzer trying to achieve a reproducable analysis across different machines. I don't really see the scenario where the desired behaviour would be to just (almost) silently disable it and go on.

Also, you haven't commented on the other idea, checking whether the analyzer with the requested version is available from the path. I suspect we only get hold of a single binary and look no further. This could be especially cool for people that need to have multiple compiler versions on hand (which is rather common among C++ developers).

Now, we give error message if an analyzer version is incorrect.

In the future, it would be great not only to validate the version number, but also to allow the user to select it.

cservakt avatar Oct 06 '23 14:10 cservakt