Recognize --target=<value>/-target=<value> equals-sign form (#1158) - #5001
Recognize --target=<value>/-target=<value> equals-sign form (#1158)#5001mmido6039 wants to merge 3 commits into
Conversation
Support parsing of the equals-sign form of the --target option (e.g. --target=aarch64-linux-gnu) in addition to the existing space-separated form. The compilation target is now correctly stored and the original --target=<value> flag is preserved in analyzer options, ensuring the analyzer receives the intended target architecture. Add a regression test covering the equals-sign syntax and update the test to satisfy pylint.
c12f112 to
0ee5586
Compare
bruntib
left a comment
There was a problem hiding this comment.
This is a good catch and good bugfix. However, the original ticket (#1158) is about something else. Maybe I'm wrong, but I interpret the ticket in a way that there are two compilation actions: one with -target=<target1> and another with -target=<target2>. I understand like CodeChecker wouldn't analyze them separately. However, this is not the case, so let's accept this PR as a fix for this ticket.
| if flag_iterator.item.startswith(prefix): | ||
| details['compilation_target'] = \ | ||
| flag_iterator.item[len(prefix):] | ||
| details['analyzer_options'].append(flag_iterator.item) |
There was a problem hiding this comment.
In the first branch where -target is separated from its parameter, the flag is not added to analyzer_options. What is the motivation of adding it here? Either both or none of the branches should have this logic. I vote for not adding it to analyzer_options, because this was the original behavior.
Fixes #1158
Problem
CodeChecker did not differentiate compile database entries with
different Clang
-targetswitches, as described in the issue.Root cause
__get_target()in log_parser.py only matched the space-separatedform of the target flag ('-target ' / '--target ').
The equals-sign form ('--target=' / '-target='), a
very common cross-compilation syntax, was not recognized at all and
fell through unhandled. As a result, the explicitly requested target
was silently dropped, and the compiler's own native default target
was used instead (verified directly:
clang++ --target=aarch64-linux-gnuended up analyzed as
x86_64-pc-linux-gnuon the test machine).Fix
Extended
__get_target()to also recognize and strip the'--target='/'-target=' prefix form, in addition to the existing
space-separated form.
Testing
Added a regression test (test_target_equals_sign_form) with a new
fixture covering both the equals-sign and space-separated forms for
the same source file, asserting the exact resolved target for each -
the existing similarly-named test only asserted
len(target) > 0,which would not have caught this regression. All tests in
test_log_parser.py pass; pycodestyle clean.
Known follow-up (out of scope for this PR)
The compiler-info caching key (used to cache implicit include paths/
standard/target per compiler+language+flags) also does not account
for an explicit target, so two different
--target=values on thesame compiler binary could still share cached compiler_includes.
Fixing that touches the persisted compiler_info.json cache format
and felt like a separate, riskier change - happy to follow up in a
second PR if useful.