Dump compiler_info.json #4715
Dump compiler_info.json #4715
Conversation
bruntib
force-pushed
the
dump_compiler_info
branch
from
71b8961 to
bff21ab
Compare
barnabasdomozi
left a comment
barnabasdomozi
left a comment
There was a problem hiding this comment.
I have left some questions when reviewing this patch.
Also note that analyzer tests are currently failing:
- File "/home/runner/work/codechecker/codechecker/build/CodeChecker/lib/python3/codechecker_analyzer/cli/analyze.py", line 1383, in main
- if args.dump_compiler_info_file:
- AttributeError: 'Namespace' object has no attribute 'dump_compiler_info_file'
| if args.dump_compiler_info_file: | ||
| compiler_info = Path(args.output_path) / 'compiler_info.json' | ||
| try: | ||
| os.rename( |
There was a problem hiding this comment.
From the code, it's not clear to me what this flag actually does. Renames an already existing file? Why?
Why not dump the file to output path args.dump_compiler_info_file in the first place?
The help description says:
Dump implicit gcc compiler info to a json file
that can be used for fine-tuning analysis later.
To me, it seems this file is already dumped regardless of this flag, just under a different name: compiler_info.json. Correct me if I'm wrong, but this part is not clear to me.
There was a problem hiding this comment.
The first idea was introducing this command:
CodeChecker analyze compile_commands.json --dump-compiler-info-file <filename>
This could have created the given file. However, CodeChecker analyze has a mandatory -o flag:
CodeChecker analyze compile_commands.json --dump-compiler-info-file <filename> -o reports
Intuitively this tells me that the compiler info file will be created into reports folder. However, reports/compiler_info.json is created anyway. I didn't find it elegant, that this file is created twice in the report folder: with the name compiler_info.json and the given <filename>.
This led me to the current solution which simply renames the compiler_info.json to the given filename.
Another solution could have been not dumping compiler_info.json in the log_parser module, but here in the main() function. Maybe we could refactor it in the future.
There was a problem hiding this comment.
Can't we do this instead?
CodeChecker analyze compile_commands.json --dump-compiler-info-file -o <filename>
after the -o flag the user could directly specify the compiler_info.json. Then we could avoid this unfortunate renaming trick.
bruntib
force-pushed
the
dump_compiler_info
branch
2 times, most recently
from
5595f30 to
09cf96b
Compare
dkrupp
left a comment
dkrupp
left a comment
There was a problem hiding this comment.
I think
CodeChecker analyze --dump-compiler-info-file -o compiler_info.json
would be a better invocation syntax.
| if args.dump_compiler_info_file: | ||
| compiler_info = Path(args.output_path) / 'compiler_info.json' | ||
| try: | ||
| os.rename( |
There was a problem hiding this comment.
Can't we do this instead?
CodeChecker analyze compile_commands.json --dump-compiler-info-file -o <filename>
after the -o flag the user could directly specify the compiler_info.json. Then we could avoid this unfortunate renaming trick.
|
Please, don't merge, but approve this PR. I'd like to squash the commits before merging. Thank you! |
bruntib
force-pushed
the
dump_compiler_info
branch
from
219fa3b to
6fa9743
Compare
A new flag is added to "CodeChecker analyze": --dump-compiler-info-file. This flag is given a filename as parameter. This filename will be created in the output folder. The file contains implicit include paths and some other gcc-specific data that can be used to finetune analysis later. When this flag is used, then analysis doesn't run. The compiler_info.json was generated in the output directory by default. Due to some bug, this file is empty in recent CodeChecker versions. The reason is that parsing compile_commands.json is done in parallel with a process pool. The dict object that collects this data must be handled by multiprocessing.SyncManager() so it can be used in a process pool.
bruntib
force-pushed
the
dump_compiler_info
branch
from
cfac124 to
c863409
Compare
3 participants
A new flag is added to "CodeChecker analyze": --dump-compiler-info-file. This flag is given a filename as parameter. This filename will be created in the output folder. The file contains implicit include paths and some other gcc-specific data that can be used to finetune analysis later. When this flag is used, then analysis doesn't run.
The compiler_info.json was generated in the output directory by default. Due to some bug, this file is empty in recent CodeChecker versions. The reason is that parsing compile_commands.json is done in parallel with a process pool. The dict object that collects this data must be handled by multiprocessing.SyncManager() so it can be used in a process pool.