Skip to content

Improve the logic to determine the main license #10349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sschuberth wants to merge 4 commits into
base: main
Choose a base branch
from
+19 −17
Draft

Improve the logic to determine the main license #10349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
468dee1
Revert "fix(reporter): Fix a performance issue with generating SPDX r…
sschuberth May 16, 2025
c25bc22
chore(model): Rename some variables for clarity
sschuberth May 16, 2025
ac6e185
perf(model): Filter down to relevant files before getting the main li…
sschuberth May 16, 2025
7290fa3
WIP
sschuberth May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions model/src/main/kotlin/licenses/ResolvedLicenseInfo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@
import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.LicenseSource
import org.ossreviewtoolkit.model.Provenance
import org.ossreviewtoolkit.model.RepositoryProvenance

Check warning on line 26 in model/src/main/kotlin/licenses/ResolvedLicenseInfo.kt

View workflow job for this annotation

GitHub Actions / qodana-scan

Unused import directive 

Unused import directive
import org.ossreviewtoolkit.model.config.CopyrightGarbage
import org.ossreviewtoolkit.model.config.LicenseFilePatterns
import org.ossreviewtoolkit.model.config.PathExclude
import org.ossreviewtoolkit.model.utils.PathLicenseMatcher
import org.ossreviewtoolkit.model.utils.vcsPath
import org.ossreviewtoolkit.utils.common.FileMatcher

Check warning on line 32 in model/src/main/kotlin/licenses/ResolvedLicenseInfo.kt

View workflow job for this annotation

GitHub Actions / qodana-scan

Unused import directive 

Unused import directive
Copy link
Member

@fviernau fviernau May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only maybe related: Are we actually missing some kind of distinct() call on relativeFilePaths in ...?

fun getApplicableLicenseFilesForDirectories(
        relativeFilePaths: Collection<String>,
        directories: Collection<String>
)

As I just noticed the comment, that the issue came from large amount of paths.

Check warning

Code scanning / detekt

Detects unused imports Warning

Unused import

Check warning

Code scanning / QDJVMC

Unused import directive Warning

Unused import directive
import org.ossreviewtoolkit.utils.ort.CopyrightStatementsProcessor
import org.ossreviewtoolkit.utils.spdx.SpdxExpression
import org.ossreviewtoolkit.utils.spdx.SpdxLicenseChoice
Expand Down Expand Up @@ -83,31 +85,32 @@
* in any of the configured [LicenseFilePatterns] matched against the root path of the package (or project).
*/
fun mainLicense(): SpdxExpression? {
val matcher = PathLicenseMatcher(LicenseFilePatterns.getInstance())
val licensePaths = flatMap { resolvedLicense ->
val licenseFilePatterns = LicenseFilePatterns.getInstance()
Copy link
Member

@fviernau fviernau May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit-message: If this fixes this serious performance issue, I believe this should be made more prominent.

Furthermore, could you add some details, why exactly previously it was so slow?

Copy link
Member Author

@sschuberth sschuberth May 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For these topics, I'd refer to the original fix in ebc6fe0.

val licenseMatcher = PathLicenseMatcher(licenseFilePatterns)

val declaredLicenses = filter(LicenseView.ONLY_DECLARED, filterSources = true)
val detectedLicenses = filter(LicenseView.ONLY_DETECTED, filterSources = true)

val licensePaths = detectedLicenses.flatMapTo(mutableSetOf()) { resolvedLicense ->
resolvedLicense.locations.map { it.location.path }
}

val applicablePathsCache = mutableMapOf<String, Map<String, Set<String>>>()
val detectedLicenses = filterTo(mutableSetOf()) { resolvedLicense ->
resolvedLicense.locations.any {
val rootPath = (it.provenance as? RepositoryProvenance)?.vcsInfo?.path.orEmpty()
val packageRoot = detectedLicenses.firstOrNull()?.locations?.firstOrNull()?.provenance.vcsPath

val applicableLicensePaths = applicablePathsCache.getOrPut(rootPath) {
matcher.getApplicableLicenseFilesForDirectories(
licensePaths,
listOf(rootPath)
)
}
val applicableLicensePaths = licenseMatcher.getApplicableLicenseFilesForDirectories(
licensePaths,
listOf(packageRoot)
)

val applicableLicenseFiles = applicableLicensePaths[rootPath].orEmpty()
val applicableLicenseFiles = applicableLicensePaths[packageRoot].orEmpty()

val relevantDetectedLicenses = detectedLicenses.filterTo(mutableSetOf()) { resolvedLicense ->
resolvedLicense.locations.any {
it.location.path in applicableLicenseFiles
}
}

val declaredLicenses = filter(LicenseView.ONLY_DECLARED)
val mainLicenses = (detectedLicenses + declaredLicenses.licenses)
val mainLicenses = (declaredLicenses.licenses + relevantDetectedLicenses)
.flatMap { it.originalExpressions }
.map { it.expression }

Expand Down
3 changes: 1 addition & 2 deletions model/src/test/kotlin/licenses/ResolvedLicenseInfoTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ import org.ossreviewtoolkit.utils.spdx.toSpdx
class ResolvedLicenseInfoTest : WordSpec({
"mainLicense()" should {
"return declared and detected licenses, but no concluded license" {
RESOLVED_LICENSE_INFO.mainLicense() shouldBe
"($APACHE OR $MIT) AND ($MIT OR $GPL) AND ($BSD OR $GPL)".toSpdx()
RESOLVED_LICENSE_INFO.mainLicense() shouldBe "($APACHE OR $MIT) AND ($MIT OR $GPL)".toSpdx()
}
}

Expand Down
Loading