hasIssues property is set to false although there are issues in a Maven project

[sschuberth]

We have a case where in the analysis of a Maven project issues occur (ProjectBuildingException) and these issues also show up in the analyzer result, but still has_issues is set to false. This might be a regression from the refactoring to use the new dependency graph format.

[sschuberth]

@mnonnenmacher and @fviernau, as I'm currently working on #4096, I was wondering what to do with this bug. We could:

• Fix the bug and keep the semantics.
• Simply drop the hasIssues property (as we had issues with it before).
• Change the semantics (probably renaming it to hasSevereIssues) to only take issues into account that count as "severe" according to the user configuration.

Any opinions?

[mnonnenmacher]

I like the idea with hasSevereIssues. How can users configure what counts as severe?

[sschuberth]

How can users configure what counts as severe?

I would have said by another ort.conf entry, like severeIssueThreshold, which defaults to Severity.WARNING or Severity.HINT depending on whether we'd want to define it inclusive or exclusive. I would have said it should be inclusive, meaning we should use >= (not >) to compare against it in order to determine what counts as severe.

[sschuberth]

FYI, just rewriting the file with resolved scope like so fixes has_issues to be correct:

val result = OrtMain().readOrtResult(File("analyzer-result.yml")) // Result with wrong `has_issues` in dependency graph format.
OrtMain().writeOrtResult(result, listOf(File("analyzer-result-rewritten.yml")), "analyzer") // Result with correct `has_issues` with resolved scopes.

[mnonnenmacher]

How can users configure what counts as severe?

I would have said by another ort.conf entry, like severeIssueThreshold, which defaults to Severity.WARNING or Severity.HINT depending on whether we'd want to define it inclusive or exclusive. I would have said it should be inclusive, meaning we should use >= (not >) to compare against it in order to determine what counts as severe.

Ah ok, I was surprised because it sounded like it's already possible. I agree that the setting should be inclusive.

[mnonnenmacher]

FYI, just rewriting the file with resolved scope like so fixes has_issues to be correct:

val result = OrtMain().readOrtResult(File("analyzer-result.yml")) // Result with wrong `has_issues` in dependency graph format.
OrtMain().writeOrtResult(result, listOf(File("analyzer-result-rewritten.yml")), "analyzer") // Result with correct `has_issues` with resolved scopes.

Yes, that's because readOrtResult is resolving the dependency trees, but I think we need a solution that doesn't require that. Based on the other changes you are doing I assume you are going to remove the property in favor of the exit code?
Anyway we need to fix the collectIssues() function, I will assign this issue to me because it's a good chance to get more familiar with the dependency graph format.

[sschuberth]

Based on the other changes you are doing I assume you are going to remove the property in favor of the exit code?

Not necessarily. #4108 was done in parallel to this discussion, with a main focus on aligning the exit code behavior. I'd be still fine with introducing hasSevereIssues and making use of it to determine the exit code. But if the only use of the hasSevereIssues flag would be the exit code determination, I'd also be fine with my second bullet item from above and drop that flag.

So that boils down to the question: Do we (or does someone we know) use hasIssues / will use hasSevereIssues for anything else than exit code calculation? Is keeping that convenience flag still required, or is it fine to eventually call collectIssues()?

[mnonnenmacher]

So that boils down to the question: Do we (or does someone we know) use hasIssues / will use hasSevereIssues for anything else than exit code calculation? Is keeping that convenience flag still required, or is it fine to eventually call collectIssues()?

I think the main reason to keep the property would be third-party code that processes the ORT results, because parsing the whole file is not feasible. Another reason would be performance, because accessing the property multiple times wouldn't call collectIssues(), but since we are not using the property anywhere in the ORT code that's not really relevant.