-
Notifications
You must be signed in to change notification settings - Fork 356
refactor(Repository): Add RepositoryProvenance attribute
#10575
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
base: main
Are you sure you want to change the base?
refactor(Repository): Add RepositoryProvenance attribute
#10575
Conversation
b608307 to
cdc634c
Compare
e01ca18 to
1107a05
Compare
|
Just a rebase onto |
plugins/reporters/ctrlx/src/funTest/kotlin/CtrlXAutomationReporterFunTest.kt
Outdated
Show resolved
Hide resolved
plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt
Outdated
Show resolved
Hide resolved
9ac6ba5 to
101b10c
Compare
|
Okay, according to the tests, I clearly missed something. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10575 +/- ##
============================================
- Coverage 57.57% 57.54% -0.03%
Complexity 1702 1702
============================================
Files 346 346
Lines 12829 12833 +4
Branches 1212 1212
============================================
- Hits 7386 7385 -1
- Misses 4978 4983 +5
Partials 465 465
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I think the general problem is that older ORT results that do not contain a |
9090be5 to
cc83150
Compare
Thanks for the hint, adding the custom Deserializer fixed a lot of tests. Quite some progress today. Working on fixing the other tests. Will get back to it tomorrow. |
6c12714 to
a799cee
Compare
|
@sschuberth @mnonnenmacher |
Sorry, I'll not be available tomorrow. |
Oh, my bad. I forgot. See you next week. |
This comment was marked as outdated.
This comment was marked as outdated.
f2bdb8c to
5333234
Compare
d12811f to
601b9ee
Compare
|
@pepper-jk, please rebase to get rid of a test failure. |
I think the failing test is unrelated. The Expat package got a security update on September 16, and Conan is delivering the new package now. |
601b9ee to
8615132
Compare
That's exactly the reason why you should do the rebase 😉 We fixed that already in |
I am already working on the rebase as we speak. I was just checking the tests first to be sure yesterdays changes did not mess anything up. |
8615132 to
07b78f2
Compare
|
Rebased on |
07b78f2 to
1230128
Compare
|
Rebased on current |
|
As discussed in the community meeting today, the This should solve the remaining two threads. So that is what I will be working on now. |
62104e4 to
512ec6b
Compare
In order to support non-VCS input for ORT, `KnownProvenance`s need to be supported as input for `anaylzer` and `scanner`. The `Repository` data structure appears to be the best place to facilitate this change. This change focusses on adding a `RepositoryProvenance` attribute to `Repository`, which allows to keep previous behavior. For now it also exposes the raw `VcsInfo` of the `provenance` as its `vcs` attribute. Signed-off-by: Jens Keim <[email protected]>
Remove `vcs` and `vcsProcessed` attributes from `Repository`, since the `RepositoryProvenance` `provenance` is now the main attribute. Add `RepositoryDeserializer` to support parsing of legacy `OrtResult` files containing the previous attributes `vcs` and `vcsProcessed`, as well as the new format. It also needs to handle the `nestedRepositories` and `config` attributes for both legacy and recent `OrtResult` files. Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
…blank Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
Since these files are just parsed as plain text instead of deserialized into ORT data structures, updating them to the syntax appears to be the only way to fix the tests from here on out. Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
`Repository.provenance.vcsInfo` would be even better, but KDoc appears to not be able to follow at this attribute depth. Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
Signed-off-by: Jens Keim <[email protected]>
| // Get the [vcs]'s revision. | ||
| // Fall back to [vcsProcessed], if [vcs] has empty revision. | ||
| val resolvedRevision = vcs.revision.ifEmpty { | ||
| vcsProcess.revision.ifEmpty { | ||
| HashAlgorithm.SHA1.emptyValue | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EvaluatedModelReporterFunTests are currently failing, since the Deserializer can not extract a resolvedRevison from the legacy Repository data structure, as the vcs_processed only references master. See reporter-test-input.yml, which is used as input in the tests.
I assume I would need to iterate over the other nodes in the file and get the resolved_revision from a provenance, e.g. inside the scanner at L542 or the list of provenances at L438.
Since there is a findParent method, I assume it would at least be possible to iterate over other nodes from inside a Deserializer, but I have not confirmed this yet.
@sschuberth @mnonnenmacher Before I continue on this hunch, could you way in on the issue?
baab4b2 to
c45f7ba
Compare
|
Rebased onto |
This draft includes the first changes for my personal POC for the
LocalProvenanceInput discussed in #8803.As it's scope is so small, it might be good first contribution towards that goal.
If not, feel free to let me know. All feedback is welcome.
The
Repositorydata structure seems the easiest place to allowProvenances asAnalyzerandScannerinput.This avoids refactoring the attributes of higher level data structures, such as
OrtResult.At the moment the implementation limits it to
RepositoryProvencethough, in order to keep its previous behavior.To that end, it also continues to expose the rawVcsInfoof theprovenanceas itsvcsattribute.Signed-off-by: Jens Keim [email protected]