Skip to content
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.

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

Allow excluding a specific dependency #3537

Open
tsteenbe opened this issue Jan 26, 2021 · 9 comments
Open

Allow excluding a specific dependency #3537

tsteenbe opened this issue Jan 26, 2021 · 9 comments
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements

Comments

@tsteenbe
Copy link
Member

tsteenbe commented Jan 26, 2021

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."
@tsteenbe tsteenbe added analyzer About the analyzer tool enhancement Issues that are considered to be enhancements evaluator About the evaluator tool labels Jan 26, 2021
@nnobelis
Copy link
Member

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 #3508 even is this is not the same use case.

@sschuberth sschuberth removed the evaluator About the evaluator tool label Oct 19, 2022
@brueggenthies-ams
Copy link

@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
Copy link
Member

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

@fviernau
Copy link
Member

fviernau commented May 15, 2023

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
Copy link
Member

@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
Copy link

@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
Copy link
Member

sschuberth commented May 19, 2023

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

Then this is pretty much the use-case of #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.

sschuberth added a commit that referenced this issue Feb 2, 2024
Enable this by default for performance reasons of showcasing ORT. For
simplicity, only use a single parameter for all tools as there is
barely a need to configure this flag differently per tool. However, the
meaning of this parameter can slightly differ per tool, as not all tools
support e.g. path excludes yet, see [1] or also [2] in that context.

[1]: #5018
[2]: #3537

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
sschuberth added a commit that referenced this issue Feb 2, 2024
Enable this by default for performance reasons of showcasing ORT. For
simplicity, only use a single parameter for all tools as there is
barely a need to configure this flag differently per tool. However, the
meaning of this parameter can slightly differ per tool, as not all tools
support e.g. path excludes yet, see [1] or also [2] in that context.

[1]: #5018
[2]: #3537

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
sschuberth added a commit that referenced this issue Feb 2, 2024
Enable this by default for performance reasons of showcasing ORT. For
simplicity, only use a single parameter for all tools as there is
barely a need to configure this flag differently per tool. However, the
meaning of this parameter can slightly differ per tool, as not all tools
support e.g. path excludes yet, see [1] or also [2] in that context.

[1]: #5018
[2]: #3537

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
@cz-dev-ge
Copy link
Contributor

For example NuGet does not have scopes in the sense npm and python do. There is no such way do distinguish development and test dependencies from distributed production dependencies.

@sschuberth
Copy link
Member

For example NuGet does not have scopes in the sense npm and python do.

We already name "conventional namespaces" for NuGet, so maybe we could have "conventional scopes" as well?

There is no such way do distinguish development and test dependencies from distributed production dependencies.

From our Slack conversation, I though that we could use privateAssets="all" as a way to come up with such a "conventional scope"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer About the analyzer tool enhancement Issues that are considered to be enhancements
Projects
None yet
Development

No branches or pull requests

6 participants