fix(gradle-plugin): Do not fail with NPE when dependency POMs are missing #8463
Conversation
brigaccess
force-pushed
the
main
branch
2 times, most recently
from
3110bc5 to
f8953a7
Compare
|
Fixed detekt issue |
There was a problem hiding this comment.
Some additional commit message nits: We prefer passive voice in commit messages, so no mention of I / my / we etc.
This commit fixes an issue I reported earlier in Slack conversation:
I'd simply drop this sentence to avoid the use of non-passive voice, and the mention of the Slack conversation is not super useful unless also a link would be added (but that could expire with our free plan of Slack).
In my particuar case
Just say "In this specific case...".
| if (modelBuildingResult == null) { | ||
| val message = "POM is missing for $id" | ||
| logger.warn(message) | ||
| warnings += message | ||
| } | ||
|
|
There was a problem hiding this comment.
Let's move this directly after line 201 to handle the null case where it may occur.
There was a problem hiding this comment.
Come to think of it, I suppose it should also check if the dependency ID is not in visitedDependencies to avoid having multiple warnings?
There was a problem hiding this comment.
Yeah, extending the check to if (modelBuildingResult == null && id !in visitedDependencies) sounds like a good idea.
There was a problem hiding this comment.
Thanks for your contribution!
plugins/package-managers/gradle-plugin/src/main/kotlin/OrtModelBuilder.kt
Outdated
Show resolved
Hide resolved
plugins/package-managers/gradle-plugin/src/main/kotlin/OrtModelBuilder.kt
Outdated
Show resolved
Hide resolved
…sing Sometimes, POM cannot be resolved by ORT for one reason or another. When this happens, the analysis finishes early, omitting dependencies that have POMs but were unlucky to be listed after the POMless one. The fix itself is simple: instead of requiring a non-null value in POMs map (through `getValue`), add a warning and leave the fields empty when the returned POM is null. For example, ORT fails scanning [1] and [2] without the patch due to Gradle IntelliJ Plugin [3] injecting dependencies without POMs. [1] https://github.com/JetBrains/Grammar-Kit [2] https://github.com/JetBrains/ideavim [3] https://github.com/JetBrains/intellij-platform-gradle-plugin Signed-off-by: Igor Brovtsin <[email protected]>
This commit fixes an issue I reported earlier in Slack conversation. Sometimes, POM cannot be resolved by ORT for one reason or another. When this happens, the analysis finishes early, omitting dependencies that have POMs but were unlucky to be listed after the POMless one.
The fix itself is simple: instead of requiring a non-null value in POMs map (through
getValue), add a warning and leave the fields empty when the returned POM is null.In my particuar case, ORT fails scanning [1] and [2] without the patch due to Gradle IntelliJ Plugin [3] injecting dependencies without POMs.
[1] https://github.com/JetBrains/Grammar-Kit
[2] https://github.com/JetBrains/ideavim
[3] https://github.com/JetBrains/intellij-platform-gradle-plugin