model: Introduce the property `Package.declaredLicensesOperator` by fviernau · Pull Request #12487 · oss-review-toolkit/ort

fviernau · 2026-09-18T10:38:46Z

While this PR is part of #11459, it makes sense indepenndently of it.
In #11459 this is a necessary preparation, for not computing the processedDeclaredLicense for uncurated packages anymore.

It adds Package.declaredLicensesOperator, which has been the missing information needed for recomputing the processed declared license from the raw one(s). It make things more straight forward and clear, and simplifies the fix done by #10961.

Part of #11459.

codecov · 2026-09-18T10:55:50Z

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.18%. Comparing base (44f0a33) to head (8e337ad).

Files with missing lines	Patch %	Lines
...kage-managers/maven/src/main/kotlin/tycho/Tycho.kt	0.00%	1 Missing ⚠️
...nagers/maven/src/main/kotlin/utils/MavenParsers.kt	0.00%	1 Missing ⚠️
...nagers/maven/src/main/kotlin/utils/MavenSupport.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #12487      +/-   ##
============================================
- Coverage     59.21%   59.18%   -0.03%     
  Complexity     1892     1892              
============================================
  Files           366      366              
  Lines         13839    13837       -2     
  Branches       1462     1462              
============================================
- Hits           8195     8190       -5     
- Misses         5115     5118       +3     
  Partials        529      529

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.

mnonnenmacher · 2026-09-18T11:21:42Z

I would appreciate thoughts / comments on this.

IIUC this fixes a bug where incorrect operators are used for some package managers if declared license mappings are applied, so to me it would make sense to have this independent of any other change.

sschuberth · 2026-09-18T15:23:45Z

Would this also help to fix #10721 in a better way than #10995?

sschuberth · 2026-09-18T15:25:57Z

    val declaredLicenses: Set<String>,

+    @JsonInclude(JsonInclude.Include.NON_DEFAULT)
+    val declaredLicensesOperator: SpdxOperator = SpdxOperator.AND,


As the operator in use should only vary per ecosystem / package manager, does it really make sense to store this on the package level?

I think it does.

Please explain why. Meaning: Not why it's programmatically easier to do so, but why it semantically makes sense. Specifically, it's unclear to me why for an ecosystem that by default uses OR, we should serialize the OR operator for every package.

Please explain why.

The package becomes self contained

The decision about the operator remains flexible, and remains in the package manager plugin

It would easily possible to allow curating the operator

No unnecessary limitations

Things are more obvious, regarding declaredLicenses, e.g. the operator is next to it.

Hmm. 1) and 5) are essentially the same thing ("self contained" vs. "next to it"). Likewise for 2) and 4), because "more flexibility" is just a different way to say "fewer limitations". And for 3), I cannot think of a case where this can occur for a single package: If the operator as defined by the package manager implementation is wrong, it's most certainly wrong for all created packages of that package manager.

I'm not saying that for me this concrete proposal is totally out of the question. But so far the arguments are not convincing me fully. But it also depends on how we weight the different aspects, e.g. redundancy / storage vs. explicitness / (potential) flexibility.

Mind sharing your counterarguments?

To repeat what I wrote above:

Specifically, it's unclear to me why for an ecosystem that by default uses OR, we should serialize the OR operator for every package.

To me, storing this per-package is a waste of memory and disk space (for the non-default case) without a clear benefit.

To me, the benefit is clear, and the amount of added memory and disk scace neglegible, so we are stuck now in discussing this.

I'm fine to be overridden. I just cannot (yet) wholeheartedly approve this on my own.

Esp. @mnonnenmacher and @MarcelBochtler, please also share your views, not only on the idea, but also the concrete implementation.

I'm fine to be overridden. I just cannot (yet) wholeheartedly approve this on my own.

yes, I know such situations from myself too, no problem.

Esp. @mnonnenmacher and @MarcelBochtler, please also share your views,

yeah, let's just decide this as a group.

fviernau · 2026-09-18T21:00:11Z

Would this also help to fix #10721 in a better way than #10995?

#10995 does not fix anything, it only adds a test.
I've looked at the test, and the test does not make sense - not in context of this PR and also not independently of that.

@sschuberth did you maybe mix-up the pull-id? e.g. maybe it was #10961?

I suppose it was 10961, so again

Would this also help to fix #10721 in a better way than #10995?

Yes, it not only helps but this PR already cleans up that code, and that is mentioned in the commit message, too.

sschuberth · 2026-09-19T08:00:24Z

@sschuberth did you maybe mix-up the pull-id? e.g. maybe it was #10961?

No, I was indeed meaning to refer to #10995 as its description says Fixes #10721., but that's actually a false statement because the PR really only adds a test, as you say.

I'll edit that PR's description to clarify.

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

Any package has two versions of `declaredLicensesProcessed`: 1. The one contained in `AnalyzerResult` as set by the package manager 2. The one in the curated package, which is recomputed from the raw declared licenses, when curations get applied, e.g. in `OrtResult.kt`. Add the operator corresponding to the (raw) declared licenses to `Package`, so that the semantics of these (raw) declared licenses is clear, and the package self-contained in that regard. Also the code for re-computing the `declaredLicenseProcessed` in `PackageCurationData` becomes more straight forward. Note: This allows for dropping computing the processed license for uncurated packages entirely, so that the processing happens only as part of curating an uncurated package. Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

fviernau · 2026-09-29T08:32:33Z

Notes from discussion in dev call:

A use case could be:
- AND is used by package manager, but readme says it's an OR
Add tests for usage of OR in declared license processor (maybe also in PackageCurationData)

sschuberth reviewed Sep 18, 2026

View reviewed changes

fviernau force-pushed the package-declared-licenses-operator-property branch 3 times, most recently from 6d9c986 to 04a227d Compare September 18, 2026 23:40

fviernau marked this pull request as ready for review September 18, 2026 23:45

fviernau requested a review from a team as a code owner September 18, 2026 23:45

fviernau requested a review from sschuberth September 18, 2026 23:45

fviernau force-pushed the package-declared-licenses-operator-property branch 5 times, most recently from a8c06c7 to 0947924 Compare September 21, 2026 12:01

fviernau added 3 commits September 22, 2026 08:35

chore(maven): Extract a constant for the declared licenses operator

c1013cb

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

docs(spdx): Highlight that the merge works only if operators match

8e337ad

Signed-off-by: Frank Viernau <frank.viernau@gmail.com>

fviernau force-pushed the package-declared-licenses-operator-property branch from 0947924 to 8e337ad Compare September 22, 2026 06:35

sschuberth mentioned this pull request Sep 30, 2026

Original license operator not preserved in one case of a declared_license_mapping being specified #11054

Open

fviernau changed the title ~~model: Introduce the property PackagedeclaredLicensesOperator~~ Oct 1, 2026

Navigation Menu

model: Introduce the property `Package.declaredLicensesOperator` - #12487

model: Introduce the property `Package.declaredLicensesOperator`#12487
fviernau wants to merge 3 commits into
mainfrom
package-declared-licenses-operator-property

fviernau commented Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading

mnonnenmacher commented Sep 18, 2026

sschuberth commented Sep 18, 2026

sschuberth Sep 18, 2026

fviernau Sep 18, 2026

sschuberth Sep 19, 2026 •

edited

Loading

fviernau Sep 19, 2026 •

edited

Loading

sschuberth Sep 19, 2026 •

edited

Loading

fviernau Sep 19, 2026

sschuberth Sep 19, 2026

fviernau Sep 21, 2026

sschuberth Sep 21, 2026

fviernau Sep 21, 2026

fviernau commented Sep 18, 2026 •

edited

Loading

sschuberth commented Sep 19, 2026 •

edited

Loading

fviernau commented Sep 29, 2026

This branch has not been deployed

Labels

3 participants

Conversation

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

Uh oh!

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

Uh oh!

Codecov Report

mnonnenmacher commented Sep 18, 2026

sschuberth commented Sep 18, 2026

sschuberth Sep 18, 2026

Choose a reason for hiding this comment

fviernau Sep 18, 2026

Choose a reason for hiding this comment

sschuberth Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

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

Uh oh!

Choose a reason for hiding this comment

sschuberth Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

fviernau Sep 19, 2026

Choose a reason for hiding this comment

sschuberth Sep 19, 2026

Choose a reason for hiding this comment

fviernau Sep 21, 2026

Choose a reason for hiding this comment

sschuberth Sep 21, 2026

Choose a reason for hiding this comment

fviernau Sep 21, 2026

Choose a reason for hiding this comment

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

Uh oh!

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

Uh oh!

fviernau commented Sep 29, 2026

This branch has not been deployed

Labels

3 participants

fviernau commented Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading

sschuberth Sep 19, 2026 •

edited

Loading

fviernau Sep 19, 2026 •

edited

Loading

sschuberth Sep 19, 2026 •

edited

Loading

fviernau commented Sep 18, 2026 •

edited

Loading

sschuberth commented Sep 19, 2026 •

edited

Loading