Include skiplist file in the analysis info - #4798
Conversation
a266ce7 to
95942c7
Compare
barnabasdomozi
added
database 🗄️
8586d7c to
c1b9b9a
Compare
3661f49 to
39a5e15
Compare
dkrupp
left a comment
dkrupp
left a comment
There was a problem hiding this comment.
Please keep the original config file name.
Otherwise looks good.
fef4592 to
ee296ef
Compare
| .filter(AnalysisInfo.analyzer_command == cmd) \ | ||
| .all() | ||
|
|
||
| if analysis_info_rows: |
There was a problem hiding this comment.
Why has this "if" statement been removed? In the new version, a new analysis info is added. What happens if it already exists in the database?
There was a problem hiding this comment.
Previously, the if check was used to determine whether a given analyzer command had already been inserted into the "AnalysisInfo" table, and if so, no new row was created.
However, with the introduction of the AnalysisInfoFile table, the design now includes a mapping between an AnalysisInfo ID and the corresponding FileContent rows that should be displayed on the analysis info page. If we keep the old if check, this leads to incorrect behavior.
Consider a scenario where 2 runs use the same analyzer command (for example, CodeChecker analyze -i ./skipfile ...), but the skipfile content differs between them.
When storing the first run, a new row is inserted into the AnalysisInfo table (e.g., ID 1), and the associated file content is linked through the AnalysisInfoFile table. For the second run, the if check detects that the analyzer command already exists and reuses AnalysisInfo ID 1 instead of creating a new row, while still inserting a new entry into AnalysisInfoFile linked to that same ID. As a result, both runs share the same AnalysisInfo entry, and we will display both skipfile contents for both run 1 and run 2.
To avoid this issue, a new AnalysisInfo entry must be created for every stored run, even if the analyzer command itself is identical.
There was a problem hiding this comment.
I'm worried about growing the AnalysisInfo table. We have current issues with the size of AnalysisInfoChecker table. We should consider keeping the deduplication of AnalysisInfo and connecting the skipfiles to RunHistory table.
ee296ef to
8659f56
Compare
|
@bruntib Updated alembic schema file. |
|
Before this change, the |
e83c789 to
adbb090
Compare
061d749 to
71cfa86
Compare
This patch introduces a new table called AnalysisInfoFile, which is intended to store files related to analysis information. The actual file contents are stored separately in the FileContent table.
71cfa86 to
bb622b4
Compare
| content_hash: Optional[str] | ||
| ): | ||
| content_hash: Optional[str], | ||
| commit: bool |
There was a problem hiding this comment.
We could commit at the places where it's necessary after calling this function. This way an extra parameter wouldn't be needed.
f012021 to
0c105d6
Compare
Co-authored-by: bruntib <12861163+bruntib@users.noreply.github.com>
0c105d6 to
64aa517
Compare
|
@dkrupp Can you please review this PR again? |
The GUI now shows the original filename as requested.
3 participants
This patch introduces a new table called AnalysisInfoFile,
which is intended to store files related to analysis information.
The actual file contents are stored separately in the FileContent
table.
During CodeChecker analyze, we create a new directory called
conf/ in the report directory.
The entire conf/ directory will be added to the massStoreRun
ZIP file.
With this PR, only the skipfile is copied to the conf directory,
but this could be extended later.