OrtResult: Apply package curations on-the-fly #6391
Merged
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
fviernau
force-pushed
the
package-curations-on-the-fly
branch
5 times, most recently
from
c3cd639 to
b756c3a
Compare
fviernau
requested review from
a team,
MarcelBochtler and
oheger-bosch
as code owners
fviernau
changed the title
fviernau
force-pushed
the
package-curations-on-the-fly
branch
3 times, most recently
from
0b8d9b3 to
2df16c4
Compare
fviernau
force-pushed
the
package-curations-on-the-fly
branch
from
2df16c4 to
3f92142
Compare
mnonnenmacher
previously requested changes
Jan 30, 2023
analyzer/src/funTest/assets/projects/synthetic/gradle-all-dependencies-expected-result.yml
Outdated
Show resolved
Hide resolved
Simplify an upcoming change. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Adhere to Law of Demeter and thereby prepare for changing the representation of package in the analyzer result. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau
force-pushed
the
package-curations-on-the-fly
branch
2 times, most recently
from
23a0b2d to
1a777ea
Compare
The VCS info `OrtResult.repository.vcsProcessed` must always be normalized. In case it is not, ORT runs into an exception [1] when calling `OrtResult.getDefinitionFilePathRelativeToAnalyzerRoot()`. This in fact happens for this test asset. Change the VCS URL to the corresponding normalized one to fix the problem. Also update the URLs in VCS processed of each respective project for consistency, even though not necessary to fix the mentioned exception. [1] https://github.com/oss-review-toolkit/ort/blob/a0d9181b2c7d2cf537611ec476fb016bb3a95704/model/src/main/kotlin/OrtResult.kt#L246 Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The asset contains placeholder strings of the form `<REPLACE_...>`. These placeholder are unused, because they never get actually replaced. Replace the placeholder with real world values, choosing an arbitrary ort revision as revision. Previously, using this asset lead to crashes as soon as the VCS info gets parsed into a URL which is now fixed. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
De-serialize and serialize again some ORT files to fix formatting and to eliminate use of legacy formats. Note: These have been missing in [1]. [1] 0b29706 Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The `OrtResult` does not store the uncurated packages as part of the analyzer result, but only the curated packages along with the applied package curation data. This tightly couples the curations with the analyzer without need, because the analyzer does not need (to consume) any curations at all. Also, computing the respective uncurated package from each curated package is not always possible due to missing data [1]. So, curations currently cannot properly be (re-applied) without re-running the analyzer [2]. Furthermore, the current representation stores package curation data redundantly in case the curation applies to multiple packages. Given that, it makes sense to store the curations separately from the uncurated package. So, utilize the new toplevel `resolvedConfiguration` to store the package curations and change the analyzer result to contain uncurated instead of curated packages. Note that this partially implements [1] and [2]. Adjusting the logic which turns curated into uncurated packages, e.g. `toUncuratedPackage()`, is left for a future change to limit the size of this change. Apart from that [3] can be implemented by relatively easily without redundantly encoding the provider (for each curation data). [1] #5637 [2] #6188 [3] #5668 Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Signed-off-by: Frank Viernau <frank_viernau@epam.com>
Curated packages are not de-serialize anymore, as they have been dropped from `OrtResult` by a preceding change. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The backwards compatibility for de-serialization has been broken by the recent separation of packages and package curations in the `OrtResult`. So, now is a good time to clean-up now unused backwards compatibility code. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The file is used only in `cli` tests. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
The file is used only by `model` tests. Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau
force-pushed
the
package-curations-on-the-fly
branch
from
1a777ea to
de35c27
Compare
oheger-bosch
approved these changes
Jan 31, 2023
fviernau
added a commit
to oss-review-toolkit/orthw-shell
that referenced
this pull request
Feb 2, 2023
A recent ORT file format change does not have backwards compatibility for deserialization, see [1]. So, replace the contents of this file with a re-scan done with ORT revision 28d7426ee4855c7c27c10c8a36e3a602f68c9e87. [1] oss-review-toolkit/ort#6391 Signed-off-by: Frank Viernau <frank_viernau@epam.com>
fviernau
added a commit
to oss-review-toolkit/orthw-shell
that referenced
this pull request
Feb 2, 2023
A recent ORT file format change does not have backwards compatibility for deserialization, see [1]. So, replace the contents of this file with a re-scan done with ORT revision 28d7426ee4855c7c27c10c8a36e3a602f68c9e87. [1] oss-review-toolkit/ort#6391 Signed-off-by: Frank Viernau <frank_viernau@epam.com>
sschuberth
reviewed
Feb 2, 2023
| @@ -377,7 +377,7 @@ analyzer: | |||
| path: "" | |||
| homepage_url: "" | |||
| scopes: [] | |||
| - id: "Unmanaged:manifest.xml:manifest:<REPLACE_REVISION>" | |||
| - id: "Unmanaged:manifest.xml:manifest:31588aa8f8555474e1c3c66a359ec99e4cd4b1fa" | |||
There was a problem hiding this comment.
Is it intended that this gets expanded as part of this commit? Similar below.
There was a problem hiding this comment.
No it wasn't. Thanks for highlighting this!
I've addressed it here: #6429
| @@ -499,10 +499,10 @@ packages: | |||
| declared_licenses: | |||
| - 12 | |||
| declared_licenses_processed: | |||
| spdx_expression: "EPL-1.0 OR MPL-2.0" | |||
| spdx_expression: "MPL-2.0 OR EPL-1.0" | |||
There was a problem hiding this comment.
Why are SPDX expression terms now not sorted alphabetically anymore?
There was a problem hiding this comment.
Hm, good question I hadn't notice that it's now in reversed order.
The update was done programmatically. Have to investigate that...
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.
This PR changes the format of the ORT file to store package curations separately from packages, so that the analyzer result holds the uncurated instead of the curated packages. A couple of follow-ups, like adjusting the logic related to
toUncuratedPackage()is left for a following PR in order to not make this one too large.See the individual commits for the details.
Breaking change: The change is breaking because the ORT file format changes. Deserializing the previous format is not supported, while in all other aspects this change is intended to be non-functional.