scanner: Add Nomos plugin #10631
scanner: Add Nomos plugin #10631
Conversation
|
Commit message title: According to conventional commits, the title should start with |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10631 +/- ##
=========================================
Coverage 57.34% 57.34%
+ Complexity 1670 1668 -2
=========================================
Files 344 344
Lines 12747 12747
Branches 1209 1209
=========================================
Hits 7310 7310
Misses 4972 4972
Partials 465 465
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@Prakash-Mishra-9ghz, please have a look at these issues and make all automated checks pass before we start with the actual code review. |
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
from
2a75433 to
ba0ea5a
Compare
Resolved all Detekt issues |
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
from
ba0ea5a to
6af59a7
Compare
|
@Prakash-Mishra-9ghz we have a policy to fix any issues identified during review / by automated checks in the actual commit that introduces the issue as long as the PR is still under review. As your PR originally only had a single commit, this means to just amend that original commit with any updates. So please squash the two commits that you have now into just one again. |
|
@sschuberth I added the following copyright header: Copyright (C) 2025 Prakash Mishra. The CI failed due to this not being in the NOTICE file. Should I add myself there, or just remove the header? |
You should add yourself to the NOTICE file (grouped by year, then sorted alphabetically) and keep the generic Copyright header in the source file. |
|
hi @sschuberth could you help me to figure out the funtest-docker, test(unbuntu and windows) fail. |
Those are temporary failures due to foojayio/discoapi#124, which had been fixed, but now is back again... we simply need to wait for that external service to recover. Just rebase your PR onto latest |
Will you do that rebase, @Prakash-Mishra-9ghz? |
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
from
6af59a7 to
0bb3460
Compare
|
@sschuberth Currently working on the Docker implementation for the plugin. |
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
3 times, most recently
from
41e4847 to
2a793fa
Compare
|
@sschuberth CI is failing due to an sbt-assembly version mismatch—could you confirm if it is related to PR? |
No, it should not be related, I've retriggered the test. |
| */ | ||
|
|
||
| plugins { | ||
| id("org.jetbrains.kotlin.plugin.serialization") version "1.8.22" |
There was a problem hiding this comment.
Please change the syntax to make use of Gradle's version catalog alias like here:
There was a problem hiding this comment.
While at it, also add our conventional the grouping comments here, so it looks exactly like:
plugins {
// Apply precompiled plugins.
id("ort-plugin-conventions")
// Apply third-party plugins.
alias(libs.plugins.kotlinSerialization)
}| api(projects.scanner) | ||
|
|
||
| implementation(projects.utils.commonUtils) | ||
|
|
There was a problem hiding this comment.
No separation for grouping needed here, so just remove this blank line.
|
|
||
| ksp(projects.scanner) | ||
|
|
||
| implementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.8.1") |
There was a problem hiding this comment.
Again, align with code that makes use of the version catalog, so that we do not hard-code versions here:
ort/plugins/scanners/scancode/build.gradle.kts
Lines 40 to 41 in 40fa386
| override fun command(workingDir: File?): String { | ||
| return listOfNotNull(workingDir, "nomossa").joinToString(File.separator) | ||
| } |
There was a problem hiding this comment.
This is basically a one-liner that should be a function expression with =, similar to
ort/plugins/scanners/askalono/src/main/kotlin/Askalono.kt
Lines 48 to 49 in 97d1c08
| } | ||
|
|
||
| override fun transformVersion(output: String): String { | ||
| // Example output: nomossasa build version: 4.5.1.1 r(ff4fa7) |
There was a problem hiding this comment.
Nit: Maybe put the "nomossasa build version: 4.5.1.1 r(ff4fa7)" part to its own line, similar to
ort/plugins/scanners/askalono/src/main/kotlin/Askalono.kt
Lines 52 to 53 in 97d1c08
to avoid potential confusion where the example begins / ends due to the multiple colons in the line-
| internal fun NomossaResult.toScanSummary(startTime: Instant, endTime: Instant): ScanSummary { | ||
| val licenseFindings = results.flatMap { fileResult -> | ||
| fileResult.licenses.map { rawLicense -> | ||
|
|
There was a problem hiding this comment.
Nit: Omit this superfluous blank line.
| }.toSortedSet( | ||
| compareBy( | ||
| { it.license.toString() }, | ||
| { it.location.path }, | ||
| { it.location.startLine } | ||
| ) | ||
| ) |
There was a problem hiding this comment.
We shouldn't do any sorting here for the in-memory representation. Sorting is only meaningful for deserialization, and there we do it centrally. So just drop this code.
| } | ||
|
|
||
| LicenseFinding( | ||
| license = safeLicense, // use raw string |
There was a problem hiding this comment.
I think the comment is misleading, as the "raw string" has been converted to a "safe string". I'd just drop the comment.
| startLine = 1, | ||
| endLine = 1 |
There was a problem hiding this comment.
So, does Nomossa not provide the line range where a license matches was found at all? I fear that makes it quite unattractive as a scanner. In any case, if the line is unknown, the TextLocation.UNKNOWN_LINE constant should be used.
There was a problem hiding this comment.
Nomossa does provide line numbers. Some updates are needed in the implementation — will take care of that soon.
| val safeLicense = if (rawLicense.matches(Regex("^[A-Za-z0-9.\\-+]+$"))) { | ||
| rawLicense | ||
| } else { | ||
| "LicenseRef-Nomossa-${rawLicense.replace(Regex("[^A-Za-z0-9.+-]"), "-")}" // License not found | ||
| } |
There was a problem hiding this comment.
I guess instead of Regex-based checking we could just try to parse the license, similar to here:
ort/plugins/scanners/scanoss/src/main/kotlin/ScanOssResultParser.kt
Lines 101 to 107 in 3f4b2f6
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
from
2a793fa to
e5fb51c
Compare
Signed-off-by: Prakash Mishra <prakashmishra9921@gmail.com> chore(scanner): Update display name, fix detekt issue, and add copyright Signed-off-by: Prakash Mishra <prakashmishra9921@gmail.com>
Prakash-Mishra-9ghz
force-pushed
the
nomos
branch
from
e5fb51c to
a8ccf27
Compare
|
Please see my comment over here: This implementation lacks functional tests that make use of the scanner to prove that the implementation is working. |
This pull request introduces a new external scanner plugin for integrating FOSSology's Nomos license scanner into the OSS Review Toolkit (ORT).
🔹 What's implemented
ExternalScanner.ScanResultmodel.🔹 File breakdown
Nomossa.kt: Main scanner class implementing theScannerinterface.NomossaConfig.kt: Loads configuration for invoking Nomos CLI.NomossaResultParser.kt: Parses Nomos output to extract license info.NomossaResultExtension.kt: Converts parsed data intoScanResult.build.gradle.kts: Sets up plugin dependencies and build configuration.This plugin allows users to plug FOSSology’s Nomos agent into ORT workflows to enhance license detection coverage and flexibility.