cli: Allow to configure an additional declared license mapping file by fviernau · Pull Request #12544 · oss-review-toolkit/ort

fviernau · 2026-09-30T13:35:47Z

See individual commits.

Part of #11459.

codecov · 2026-09-30T14:31:19Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.21%. Comparing base (ecd6689) to head (0678889).

Files with missing lines	Patch %	Lines
model/src/main/kotlin/config/OrtConfiguration.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #12544      +/-   ##
============================================
- Coverage     59.21%   59.21%   -0.01%     
  Complexity     1894     1894              
============================================
  Files           366      366              
  Lines         13839    13840       +1     
  Branches       1462     1462              
============================================
  Hits           8195     8195              
- Misses         5114     5115       +1     
  Partials        530      530

Flag	Coverage Δ
funTest-external-tools	`16.13% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fviernau · 2026-10-01T07:46:04Z

+
+private fun configureDeclaredLicenseMapper(ortConfig: OrtConfiguration) {
+    val customMappingFile = ortConfig.declaredLicenseMappingFilePath?.let { File(it) }
+        ?: ortConfigDirectory.resolve(ORT_DECLARED_LICENSE_MAPPING_FILENAME).takeIf { it.isFile }


I wonder if this code location is ok to read user config files and fail early.

Fail early now looks like this:

ort config Could not read the declared license mapping from '/home/user/declared-license-xxmapping.yml': java.io.FileNotFoundException: /home/user/declared-license-xxmapping.yml (Datei oder Verzeichnis nicht gefunden) at java.base/java.io.FileInputStream.open0(Native Method) at java.base/java.io.FileInputStream.open(FileInputStream.java:185) at java.base/java.io.FileInputStream.<init>(FileInputStream.java:139) at kotlin.io.FilesKt__FileReadWriteKt.readText(FileReadWrite.kt:135) at kotlin.io.FilesKt__FileReadWriteKt.readText$default(FileReadWrite.kt:135) at org.ossreviewtoolkit.utils.spdxexpression.SpdxDeclaredLicenseMapper$Companion.readMapping(SpdxDeclaredLicenseMapper.kt:52) at org.ossreviewtoolkit.cli.OrtMainKt.readDeclaredLicenseMapping(OrtMain.kt:257) at org.ossreviewtoolkit.cli.OrtMainKt.configureDeclaredLicenseMapper(OrtMain.kt:237) at org.ossreviewtoolkit.cli.OrtMainKt.access$configureDeclaredLicenseMapper(OrtMain.kt:1) at org.ossreviewtoolkit.cli.OrtMain.run(OrtMain.kt:174) at com.github.ajalt.clikt.core.CoreCliktCommandKt.parse(CoreCliktCommand.kt:107) at com.github.ajalt.clikt.core.CoreCliktCommandKt.main(CoreCliktCommand.kt:78) at com.github.ajalt.clikt.core.CoreCliktCommandKt.main(CoreCliktCommand.kt:90) at org.ossreviewtoolkit.cli.OrtMainKt.main(OrtMain.kt:95)

To me that is ok.

fviernau · 2026-10-01T07:47:59Z

+     * A file containing an additional declared license mapping, which is applied in addition to the built-in one. This
+     * mapping can override entries of the built-in mapping.
+     */
+    val declaredLicenseMappingFilePath: String? = null,


Alternative name: customDeclaredLicenseMappingFilePath

Also, any thoughts about File vs. FilePath ?

I would prefer declaredLicenseMappingFile.

mnonnenmacher · 2026-10-01T10:07:42Z

 */
 internal inline fun <reified T : SpdxExpression> readLicenseMappingResource(name: String): Map<String, T> {
-    val resource = checkNotNull(SpdxDeclaredLicenseMapper::class.java.getResource(name))
+    val resource = checkNotNull(object {}.javaClass.getResource(name))


Commit message is missing a rationale.

mnonnenmacher · 2026-10-01T10:08:50Z

 * Parse a YAML string which contains a top-level sequence of key-value pairs.
 */
-internal fun String.parseYamlKeyValueLines(): Map<String, String> =
+private fun String.parseYamlKeyValueLines(): Map<String, String> =


Commit message is missing a rationale.

mnonnenmacher · 2026-10-01T10:13:17Z

 import com.github.ajalt.clikt.completion.completionOption
 import com.github.ajalt.clikt.core.CliktCommand
 import com.github.ajalt.clikt.core.Context
+import com.github.ajalt.clikt.core.PrintMessage


Commit message nit: "to reduce"

As configureDeclaredLicenseMapper() throws in case a non-existing file
is configured, it also throws in the test case named
"EnvironmentVariableFilter is correctly initialized". Move that setup of
the mapper after the one for the environment filter, to make that test
pass.

In my opinion it's wrong that the test cases uses the reference file, it should use a dedicated config instead. Production code shouldn't have to compensate for a poorly written test. I have no problem with moving the mapper setup, but I would prefer to fix the test.

I have no problem with moving the mapper setup, but I would prefer to fix the test.

I will make a follow-up PR to fix the test, then.

Here it is: #12550.

(If you are ok, we can still merge this (12544) PR first, to not delay it further)

mnonnenmacher · 2026-10-01T10:22:39Z

+/**
+ * The name of the ORT declared license mapping file.
+ */
+const val ORT_DECLARED_LICENSE_MAPPING_FILENAME = "declared-license.mapping.yml"


Why not "declared-license-mapping.yml" with a dash before "mapping"?

mnonnenmacher · 2026-10-01T10:32:52Z

 }
+
+private fun configureDeclaredLicenseMapper(ortConfig: OrtConfiguration) {
+    val customMappingFile = ortConfig.declaredLicenseMappingFilePath?.let { File(it) }


If the file is not absolute, should we maybe resolve it relative to the config dir, to have some defined behavior for relative paths?

I've taken a look at analog configuration for CustomLicenseTextsProvider and LicenseTextCurationProvider.
In these two cases, we treat the path as absolute / do not resolve it relative to the config dir.

I think, at least for consistency, this should remain as-is in this regard.

Prior to b964c70 the function has been located in `SpdxDeclaredLicenseMapper.kt` and has only been used there. But as b964c70 intends to enable use by multiple callers from different classes, using `SpdxDeclaredLicenseMapper::class` seems arbitrary and confusing. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

There is no need for `internal` visibility. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

Prepare for re-use in an upcoming change. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

Improve readability and prepare for enhancing the logic. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

Allow the user to define additonal entries or to override built-in entries via a custom `declared-license-mapping.yml`. This is useful in case an entry cannot be contributed upstream, or to reduce the feedback cycle of seeing the effect of newly added entries. As `configureDeclaredLicenseMapper()` throws in case a non-existing file is configured, it also throws in the test case named "EnvironmentVariableFilter is correctly initialized". Move that setup of the mapper after the one for the environment filter, to make that test pass. Note: A following change will make the simple license mapping configurable in an analog way. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

fviernau requested a review from a team as a code owner September 30, 2026 13:35

fviernau force-pushed the feat-custom-declared-license-mapping branch from b3be124 to 0296573 Compare September 30, 2026 13:36

github-advanced-security Bot found potential problems Sep 30, 2026

View reviewed changes

Comment thread cli/src/main/kotlin/OrtMain.kt Fixed

fviernau force-pushed the feat-custom-declared-license-mapping branch from 0296573 to 519a8fd Compare September 30, 2026 14:17

github-advanced-security Bot found potential problems Sep 30, 2026

View reviewed changes

Comment thread cli/src/main/kotlin/OrtMain.kt Fixed

fviernau force-pushed the feat-custom-declared-license-mapping branch 2 times, most recently from de0f88b to 9cf78e5 Compare October 1, 2026 07:42

fviernau commented Oct 1, 2026

View reviewed changes

fviernau changed the title ~~cli: Allow to inject a custom declared license mapping~~ Oct 1, 2026

fviernau changed the title ~~cli: Allow to configure a custom declared license mapping file~~ Oct 1, 2026

fviernau force-pushed the feat-custom-declared-license-mapping branch 2 times, most recently from cec06b5 to 6d768c3 Compare October 1, 2026 09:16

mnonnenmacher requested changes Oct 1, 2026

View reviewed changes

mnonnenmacher reviewed Oct 1, 2026

View reviewed changes

fviernau added 5 commits October 1, 2026 12:34

chore(spdx-expression): Make parseYamlKeyValueLines() private

fc92edb

There is no need for `internal` visibility. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

refactor(spdx-expression): Factor out parseLicenseMapping()

22c1fef

Prepare for re-use in an upcoming change. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

refactor(main): Factor out functions for configuring the mappers

d5d648f

Improve readability and prepare for enhancing the logic. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

fviernau force-pushed the feat-custom-declared-license-mapping branch from 6d768c3 to 0678889 Compare October 1, 2026 10:42

fviernau requested a review from mnonnenmacher October 1, 2026 10:49

Navigation Menu

cli: Allow to configure an additional declared license mapping file - #12544

cli: Allow to configure an additional declared license mapping file#12544
fviernau wants to merge 5 commits into
mainfrom
feat-custom-declared-license-mapping

fviernau commented Sep 30, 2026

Uh oh!

Uh oh!

codecov Bot commented Sep 30, 2026 •

edited

Loading

fviernau Oct 1, 2026 •

edited

Loading

fviernau Oct 1, 2026

mnonnenmacher Oct 1, 2026

fviernau Oct 1, 2026

mnonnenmacher Oct 1, 2026

mnonnenmacher Oct 1, 2026

mnonnenmacher Oct 1, 2026

mnonnenmacher Oct 1, 2026

fviernau Oct 1, 2026

fviernau Oct 1, 2026 •

edited

Loading

mnonnenmacher Oct 1, 2026

mnonnenmacher Oct 1, 2026

fviernau Oct 1, 2026

This branch has not been deployed

Labels

3 participants

Conversation

fviernau commented Sep 30, 2026

Uh oh!

Uh oh!

codecov Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

fviernau Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fviernau Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This branch has not been deployed

Labels

3 participants

codecov Bot commented Sep 30, 2026 •

edited

Loading

fviernau Oct 1, 2026 •

edited

Loading

fviernau Oct 1, 2026 •

edited

Loading