Merge distributed rule metadata files - #260
Conversation
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
Please note that the structure of metadata.json, specifically version 2, is specified here:
https://github.com/Ericsson/codechecker/blob/master/docs/report_directory.md#metadata-structure
It might be wise to drop this somewhere in the new metadata_merge.py file.
| # Fail if the plist file version is different | ||
| assert json1["version"] == json2["version"] |
There was a problem hiding this comment.
I'll one up you: Fail if the version isn't 2.
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
It'd be nice to follow the official documents order of items.
Also, we could have a very nice unit test on this. And currently we have none :)
| json1_root = json1["tools"][0] | ||
| json2_root = json2["tools"][0] |
There was a problem hiding this comment.
*_root sounds like a poor name. How about *_first_tool or something like that?
| # Merge time; we assume here both json files describe jobs in | ||
| # the same analysis invocation, implying that the analysis start | ||
| # time is the lowest timestamp, and the end is the highest. | ||
| # Note: caching will break this assumption |
There was a problem hiding this comment.
That sounds like a bigger deal than this Note implies.
There was a problem hiding this comment.
I extended the Note, not sure what else could I do here.
| assert json1_root["name"] == json2_root["name"] | ||
| # same CodeChecker version | ||
| assert json1_root["version"] == json2_root["version"] | ||
| # We assume that the list of enabled checkers haven't changed between runs. |
There was a problem hiding this comment.
What if we have several, differently configured codechecker_test targets, on which a main test target depends? Would each of those targets do their metadata merge individually?
Can we (should we) assert on this?
There was a problem hiding this comment.
The merging happens on a per-codechecker_test target basis, to mimic/follow the behavior of the non per-file codechecker_test rule.
I have written functions to merge analyzer sections of metadata files too.
Is this assert still relevant in this case?
We absolutely do not need the individual metadata files in the data folder, the merged one is enough
2 participants
Why:
We don't need multiple metadata files, just one.
What:
Addresses:
Fixes: #45
Depends on:
#79