ort icon indicating copy to clipboard operation
ort copied to clipboard

Review skipExcludes Analyzer option

Open tsteenbe opened this issue 6 months ago • 4 comments

In the ORT community meeting of May 8th, 2025 there was some confusion of how skipExcludes works for the Analyzer as a question to https://github.com/oss-review-toolkit/ort/commit/13960e9cfcc03c606dd837c50d661abf942d42f1 / PR https://github.com/oss-review-toolkit/ort/pull/10212. Propose we review the code in question and as needed make it clearer with code comments how the code works and improve user document to state for which package managers this Analyzer option is supported.

tsteenbe avatar May 08 '25 08:05 tsteenbe

In particular, this logic should be reviewed:

https://github.com/oss-review-toolkit/ort/blob/bca951028d4b90692111d2311a6b3e589dd5090c/analyzer/src/main/kotlin/PackageManager.kt#L194-L201

sschuberth avatar May 08 '25 09:05 sschuberth

To me, this reads as if this option works very different compared to options of the same name in other tools, e.g. ScannerConfiguration.skipExcluded:

Instead of configuring whether analysis should be skipped, it configures whether excludes defined in the RepositoryConfiguration should be taken into account or not. Or am I misreading things, @oss-review-toolkit/kotlin-devs?

sschuberth avatar May 08 '25 09:05 sschuberth

From my understanding, the feature works in this way:

  • An Excludes object is used by the Analyzer to control which parts of the project should be analyzed. Based on this, Analyzer may for instance filter out definition files matched by path excludes.
  • If the skipExcluded flag is true, this Excludes object is initialized from the exclusion rules defined in the repository configuration. So, everything marked as to be excluded there is going to be skipped.
  • If the skipExcluded flag is false, an empty Excludes object is used. Then Analyzer will do a full analysis, and later pipeline steps can ignore parts of the result based on the exclusion rules from the repository configuration.

I assume, this is actually what this feature is supposed to do, right?

oheger-bosch avatar May 08 '25 10:05 oheger-bosch

I think the behavior is indeed correct, but the naming / docs are partly misleading. For example the comment at

https://github.com/oss-review-toolkit/ort/blob/c07420d429b40a418e20d46d19284a5b34606c93/model/src/main/kotlin/config/RepositoryConfiguration.kt#L41-L46

says "Note that excluded parts will still be analyzed" which is not true e.g. for path excludes and skipExcluded enabled.

In general, path and scope excludes are handled quite differently, so that I'm thinking about having dedicated classes for each, actually. Path excludes are handled globally by findManagedFiles(), see

https://github.com/oss-review-toolkit/ort/blob/bca951028d4b90692111d2311a6b3e589dd5090c/analyzer/src/main/kotlin/PackageManager.kt#L108-L111

whereas scope excludes are sometimes handled directly in the package manager implementation, but are also handled by some (?) OrtResult helper functions / properties.

sschuberth avatar May 08 '25 11:05 sschuberth