Misc updates after trying to run clang-tidy #3

Vexthil · 2021-01-02T00:22:04Z

@rasjani
I finally managed to get some time to look into this after your comment on Ericsson/codechecker#555. This looks to be a very useful tool so thank you for creating it, but I came across a few issues in my use case that I have fixed for me in this pull request. These I imagine are not all going to be valid in the current format to be accepted into your branch but I wanted to make you aware of the issues I came across.

Python 3.6 has added a new error "attempted relative import with no known parent package" and it seemed to trip up your import commands. I don't know if that is because the package is not setup quite correct or I have not set my version up locally correct but making these explicit global imports instead fixed these. I assume there is a better solution for this but I don't know much about the python packaging system myself
The compile_commands.json data it seems has two possible formats(https://clang.llvm.org/docs/JSONCompilationDatabase.html). Interestingly fastbuild(which is what my project uses to generate this) uses the "arguments" format rather than the "command" format and so I updated it to use that format. This will need to be fixed to work with both formats as this change will break yours but I expect it to not be too complicated to read the two input formats into a common format in the tool
I had to rework a few bits as these seem to differentiate between a string[] and a dictionary so depending on how the arguments/commands change is structured these parts may be able to be reverted. I do feel sticking to a dictionary is a better structure though as the arg[2:] line in convert_includes may not find the correct data
includes_as_system was defaulting to if none were given it would make everything a system include. I have inverted this and so nothing is a system include by default but this may be an undesirable change depending on how you see this argument used. It is an additive argument and so I felt defaulting to none made more sense
Your details had been left in the config dumper location calculation. I ended up removing this as I was fine with it just writing into the working directory but I expect this may be better off reworking this to provide a more custom location if desired. The default I would say should be the working directory though

As I said above these changes are not in a format that can currently be accepted into your branch. If you want to take these as a starting point and refactor/improve as you need to solve the issues I came across then that would be great to hear about. If you want any testing done on my side or with different data then do ask and I can update as things change.

…th no known parent package"

When dumping the config this is now written to the current working directory instead of Janis specific folder

Updated the ignore file to make sure the processcdb.ini is not added into git

A couple of issues were found when trying to make clang tidy run. * The compiliation database was trying to use "command" instead of "arguments". According to the docs either are possible to this probably should be updated to support both instead of the quick fix I did here * to_dict can get given an empty string. Handle that error and return correctly * If there are no arg_additions specificed in the config then this needs to handle the error gracefully

xml files are the output from the process and so should not be submitted. This name is custom though so this may not be totally correct in all cases

* If no path_matcher is given then it would default to everything using the path matcher. Now it defaults to nothing using it instead as the setting is additive * It used to try to find the include path by using arg[2:] but will only work when the arguments are a single string. As the -I and the include_path are seperate arguments this needed to be rewritten to combine these into one argument. Sadly lambdas in python cannot handle out parameters or multiple return values so this needed to be unrolled a bit

Fixed a few spelling mistakes in the readme and added a note that the tools will all be defined in the config file

This was used to limit the compile_commands to specific git changelists. While something like this may be needed in the future this will need to be rewritten for perforce. The --commit arguments still exist but I have commented out the implementation for now until we know what is going to be needed for this fully. The main reason to remove this is to remove the dependencies and allow us to clean up our python packages

Fixed compile error

Add an initial batch script to run this tool for or usages. The paths are currently hard coded but when we move this into the full depot then we can abstract these correctly

If you put a path ../clang-tidy.exe into the binary field then it was not getting found and now it will correctly. Only clang-tody has been tested though

default_args in clang-tidy were getting prepended onto the standard set of arguments but this meant that filter_arguments culls the first one as it has an args[1:] in it. Secondly these were getting put after the -- line and so now these are "extra" instead and passed directly to clang-tidy instead of as extra arguments that are passed to the compiler

Made the clang tidy processor work with cl.exe and clang-cl.exe as this is what is available on windows

Vexthil added 13 commits January 1, 2021 14:32

Remove all . from import paths to solve "attempted relative import wi…

544ff1f

…th no known parent package"

Removed user config folder

03e7d60

When dumping the config this is now written to the current working directory instead of Janis specific folder

Ignore *.ini

38f73e4

Updated the ignore file to make sure the processcdb.ini is not added into git

Ignore *.xml files

c398bd8

xml files are the output from the process and so should not be submitted. This name is custom though so this may not be totally correct in all cases

Update README.md

b6d43ae

Fixed a few spelling mistakes in the readme and added a note that the tools will all be defined in the config file

Commented out check too

26860f9

Fixed compile error

Add run bat

7cf212e

Add an initial batch script to run this tool for or usages. The paths are currently hard coded but when we move this into the full depot then we can abstract these correctly

Make tool binary paths absolute

28e4b26

If you put a path ../clang-tidy.exe into the binary field then it was not getting found and now it will correctly. Only clang-tody has been tested though

Made clang-cl.exe set the driver mode correctly

d047acc

Made the clang tidy processor work with cl.exe and clang-cl.exe as this is what is available on windows

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Misc updates after trying to run clang-tidy #3

Misc updates after trying to run clang-tidy #3

Vexthil commented Jan 2, 2021

Misc updates after trying to run clang-tidy #3

Are you sure you want to change the base?

Misc updates after trying to run clang-tidy #3

Conversation

Vexthil commented Jan 2, 2021