-
Notifications
You must be signed in to change notification settings - Fork 314
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
WorkingTree: Make clear that returned revisions are resolved #4142
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…esults ORT does not support source artifacts for projects, therefore we can safely assume that the provenance of a scan result for a project is always a `RepositoryProvenance`. Note that this assumption was already made further down in the same function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
The function erroneously compared the `revision` and the `resolvedRevision` of the provided `vcsInfo`. This was wrong, because it was instead supposed to check if the `revision` of a nested repository matches the `resolvedRevision` of the provided `vcsInfo`. While on it, also change the signature of the functino to take a `RepositoryProvenance` instead of a `VcsInfo` to make clear that it is expected to contain a `resolvedRevision`. This is also a preparation for an upcoming commit which will move the `resolvedRevision` property from `VcsInfo` to `RepositoryProvenance`. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
This is a preparation for a subsequent commit which will remove `resolvedRevision` from `VcsInfo` and move it to `RepositoryProvenance` instead. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
The `resolvedRevision` property was supposed to be only set by the downloader. This was not ideal, because this requirement could easily be overlooked when creating `VcsInfo`s in the analyzer. It also made it complex to match the provenance of a scan result against the `VcsInfo` of `Project`s and `Package`s. Moving the property to `RepositoryProvenance` makes it clear which revision is provided by the analyzer and which revision is provided by the downloader which simplifies the matching. The requirement for `resolvedRevision` previouly lead to problems, especially because there was also the the demand that package managers can signal that a revision was already resolved (see 48d2cbb, 32681e1, 835b197). Address this by adding the new property `isResolvedRevision` to `VcsInfo`. This was done in the same commit, because some logic related to the removed `resolvedRevision` would otherwise have to be removed in this commit only to recover it in the next commit. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
The `VcsInfo.isResolvedRevision` property can be used by the analyzer to signal that the provided revision was already resolved. Therefore do not try to guess a revision anymore if this property is set to true and fail the download instead if the revision cannot be downloaded. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
The previously introduced `VcsInfo.isResolvedRevision` property allows to signal that the revision was already resolved. This allows to recover the logic removed by 835b197 without causing issues with the matching algorithm used for stored scan results. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
Revisions returned by the working tree are already resolved, so change the implementation of `getInfo()` to set `isResolvedRevision` to true. This will affect the VCS properties of projects, because `PackageManager.processProjectVcs()` merges the VCS infromation captured from metadata with the VCS information of the working tree. As a result, the ORT downloader will not try to guess revisions for projects anymore and instead only use the (resolved) revisions provided by the analyzer. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@here.com>
Closed for now, will be continued later once it is clear how we want to model resolved revisions in the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP: This depends on #4132 to be merged first.
Revisions returned by the working tree are already resolved, so change
the implementation of
getInfo()
to setisResolvedRevision
to true.This will affect the VCS properties of projects, because
PackageManager.processProjectVcs()
merges the VCS infromation capturedfrom metadata with the VCS information of the working tree.
As a result, the ORT downloader will not try to guess revisions for
projects anymore and instead only use the (resolved) revisions provided
by the analyzer.