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 empty revisions in VCS matchers of package configurations #4196

Closed
sschuberth opened this issue Jun 21, 2021 · 9 comments
Closed

Allow empty revisions in VCS matchers of package configurations #4196

sschuberth opened this issue Jun 21, 2021 · 9 comments
Labels
configuration About configuration topics enhancement Issues that are considered to be enhancements

Comments

@sschuberth
Copy link
Member

sschuberth commented Jun 21, 2021

In curations we already support (Ivy-style) ranges for versions as part of IDs. On the other hand, package configurations do currently not support version ranges.

While in a recent developer meeting HERE expressed concerns about that feature due to potential misuse, the same risk of misuse already stems from curations, and Bosch definitely needs the feature. So we should implement it IMO, and simply leave it to every party to make use of it or not. If required, there could even be simple CI checks for package configuration changes that prevent the use of version ranges.

@sschuberth sschuberth added configuration About configuration topics enhancement Issues that are considered to be enhancements labels Jun 21, 2021
@sschuberth
Copy link
Member Author

sschuberth commented Mar 25, 2022

@fviernau, we realized this issue seems to confuse two things:

  • the version as part of the id of a package configuration, and
  • the revision as part of a VCS matcher.

What we'd like to implement is not to support version ranges as part of the id, because that would be nonsense anyway as the VCS / source artifact matches would still point to a fixed revision / artifact URL.

Instead, the idea is to allow to leave the revision of a VCS matcher empty, so any revision is matched. The use case is SPDX files that define packages whose source code resides in the same repository as the project itself, or whose package directories point to Git submodules. Currently, whenever the project or the submodule is updated, also the revision in the VCS matcher of package configurations needs to be updated, which is inconvenient.

So, would you still be fine with optionally omitting the revision to match any revision? Would you require this possibility to be hidden behind a feature toggle?

@sschuberth sschuberth changed the title Support version ranges in package configurations Allow empty revisions in VCS matchers of package configurations Mar 25, 2022
@sschuberth
Copy link
Member Author

So, would you been fine with optionally omitting the revision to match any revision?

Alternatively, we could require to explicitly write the name of the symbolic revision (like "master" / "main"), which would probably get us around implementing a feature toggle: The toggle then is to write out the symbolic revision explicitly.

@sschuberth sschuberth changed the title Allow empty revisions in VCS matchers of package configurations Allow empty / symbolic revisions in VCS matchers of package configurations Mar 25, 2022
@fviernau
Copy link
Member

fviernau commented Mar 25, 2022

Instead, the idea is to allow to leave the revision of a VCS matcher empty, so any revision is matched. The use case is SPDX files that define packages whose source code resides in the same repository as the project itself, or whose package directories point to Git submodules. Currently, whenever the project or the submodule is updated, also the revision in the VCS matcher of package configurations needs to be updated, which is inconvenient.

Defining LicenseFindingsCurationss and PathExcludes is typically done via ort.yml, can you explain why you want to use package configurations for that purpose? ..Or rather, would it make sense to use ort.yml instead?

edit: I recall parts of a discussion quite a while ago in ORT developer meeting: it included the proposal of mine to implement treating a subtree of the source tree as corresponding to a (SPDX) package, including treating the respective ort.yml path excludes and license finding curations corresponding to the subtree as applicable to the package only. While the root of the subtree was according to any spdx-package.yml file. Note: I've implemented it only very rudimentary / partially for the notices, and IIRC there was consent that this approach should be followed not just in the notice (report) but also earlier stages.

@sschuberth
Copy link
Member Author

The package configurations I'm referring to do come from .ort.yml, i.e. we have enableRepositoryPackageConfigurations set to true.

@fviernau
Copy link
Member

fviernau commented Mar 25, 2022

The package configurations I'm referring to do come from .ort.yml, i.e. we have enableRepositoryPackageConfigurations set to true.

I didn't mean package configurations in ort.yml but I meant the properties excludes->paths and curations->license-findings in ort.yml.

See also https://github.com/oss-review-toolkit/ort/blame/main/docs/config-file-package-configuration-yml.md#L4-L7

@sschuberth
Copy link
Member Author

@fviernau, I guess we should have a meeting about this.

@sschuberth
Copy link
Member Author

sschuberth commented Apr 6, 2022

As just discussed with @mnonnenmacher matching symbolic revisions is not possible as ORT does not store them in the scan results currently.

@sschuberth sschuberth changed the title Allow empty / symbolic revisions in VCS matchers of package configurations Allow empty revisions in VCS matchers of package configurations Apr 6, 2022
sschuberth added a commit that referenced this issue Apr 6, 2022
Allow to omit a `VcsMatcher`'s `revision` to match any `resolvedRevision`
from a `RepositoryProvenance`. This is especially useful when working
with `SpdxDocumentFile`s that define packages in subdirectories of the
project's repository, as it avoids the `revision` to be updated whenever
commits to the project (not the package) are being made.

Resolves #4196.

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
sschuberth added a commit that referenced this issue Apr 6, 2022
Allow to omit a `VcsMatcher`'s `revision` to match any `resolvedRevision`
from a `RepositoryProvenance`. This is especially useful when working
with `SpdxDocumentFile`s that define packages in subdirectories of the
project's repository, as it avoids the `revision` to be updated whenever
commits to the project (not the package) are being made.

Resolves #4196.

Signed-off-by: Sebastian Schuberth <sebastian.schuberth@bosch.io>
@hanna-modica
Copy link
Contributor

Can it be documented in [1], that empty revisions are allowed and what the implications are?

[1] https://github.com/oss-review-toolkit/ort/blob/main/docs/config-file-package-configuration-yml.md#package-configuration-file-basics

@sschuberth
Copy link
Member Author

sschuberth commented Apr 8, 2022

Can it be documented in [1], that empty revisions are allowed and what the implications are?

It is already briefly documented there by saying "[...] with an optional revision".

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

No branches or pull requests

3 participants