oss-review-toolkit / ort

A suite of tools to automate software compliance checks.
https://oss-review-toolkit.org
Apache License 2.0
1.56k stars 306 forks source link

Allow excluding a specific dependency #3537

Open tsteenbe opened 3 years ago

tsteenbe commented 3 years ago

Not all package managers have scopes such as Python where its requirements.txt is basically a flat list of dependencies. What if in https://github.com/heremaps/xyz-spaces-python/blob/master/requirements.txt the developer was to exclude geopandas as PROVIDED_BY e.g. be provided by end user or to be installed on the system.

Propose we introduce way for ORT users to exclude a specific package and was thinking of below:

---
excludes:
  paths:
  - pattern: "requirements_dev.txt"
    reason: "BUILD_TOOL_OF"
    comment: "Optional dependencies only used for development / testing."
  - pattern: "tests/**"
    reason: "TEST_OF"
    comment: "Directory only used for testing."
  dependencies:
  // Pattern matches the id of the package, ommitting the version number here so it will always match the package 
  - pattern: "PyPI::geopandas"
    reason: "PROVIDED_BY"
    comment: "Packages to be provided at runtime by user."
nnobelis commented 3 years ago

I think is a good idea, especially if the scanner afterwards does not scan the dependency at all if --skip-excluded is set. See also https://github.com/oss-review-toolkit/ort/pull/3508 even is this is not the same use case.

brueggenthies-ams commented 1 year ago

@nnobelis we are currently evaluating ORT, and ran into the same "issue". Would the current architecture support the extension of the excludes mechanic to also work with packages? If yes, i would look into this.

nnobelis commented 1 year ago

@sschuberth @mnonnenmacher @fviernau Could you please answer @brueggenthies-ams ?

fviernau commented 1 year ago

I believe ORT only reflects what is supported by the respective package manager. E.g. if the package managers have scopes for "test dependencies" then these dependencies really do not go into the released artifact. So, the exclusion mechanism exists on the package manager side, not in ORT, ORT only reflects that. In my view we should stick to that approach, for clarity and simplicity and correctness of the results.

For requirements.txt there is the option to have a requirements-*.txt to define a separate set of dependencies (which could be excluded. But also, it seems that using a setup.py with a install_requires would solve the outlined issue, see https://packaging.python.org/en/latest/discussions/install-requires-vs-requirements/ .

sschuberth commented 1 year ago

@nnobelis we are currently evaluating ORT, and ran into the same "issue".

@brueggenthies-ams do you have another, maybe more convincing, use-case than the mentioned example of a Python requirements.txt file?

brueggenthies-ams commented 1 year ago

@sschuberth we are trying to analyse an Android project, and noticed that the scanner will run over every dependency, including those by JetBrains or Google, e.g. the Kotlin standard library. We would like to exclude some dependencies as we trust them. This would drastically reduce our analysis duration.

Although the setup is a little different (Android Project uses Gradle), we also cannot exclude any files/directories, as the dependencies are all defined inside the build files (or sometimes even included implicitly).

sschuberth commented 1 year ago

We would like to exclude some dependencies as we trust them.

Then this is pretty much the use-case of https://github.com/oss-review-toolkit/ort/issues/5105, and really excluding such dependencies is not the way to go. It must still be documented that they are there, but esp. scanning could be skipped.