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

SpdxDocumentFile: Report issues from subprojects that cannot be mapped to a package ID. #8851

Merged

Conversation

MarcelBochtler
Copy link
Member

Please ensure that your pull request adheres to our contribution guidelines. Thank you!

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.63%. Comparing base (25f07db) to head (43c2fa4).
Report is 24 commits behind head on main.

Files Patch % Lines
...spdx/src/main/kotlin/utils/SpdxResolvedDocument.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8851      +/-   ##
============================================
+ Coverage     67.54%   67.63%   +0.08%     
- Complexity     1166     1167       +1     
============================================
  Files           244      244              
  Lines          7775     7780       +5     
  Branches        865      866       +1     
============================================
+ Hits           5252     5262      +10     
+ Misses         2167     2161       -6     
- Partials        356      357       +1     
Flag Coverage Δ
funTest-docker 67.36% <50.00%> (+0.58%) ⬆️
funTest-non-docker 33.90% <0.00%> (-0.02%) ⬇️
test 37.91% <50.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch from d5f09b6 to 7babf76 Compare July 8, 2024 14:00
@MarcelBochtler MarcelBochtler changed the title SpdxDocumentFile: Do not drop issues SpdxDocumentFile: Report issues from subprojects that cannot be mapped to a package ID. Jul 8, 2024
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch 4 times, most recently from 2a46c29 to 882eea0 Compare July 15, 2024 09:21
@MarcelBochtler MarcelBochtler marked this pull request as ready for review July 15, 2024 09:46
@MarcelBochtler MarcelBochtler requested a review from a team as a code owner July 15, 2024 09:46
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch 2 times, most recently from db036d3 to 6999b77 Compare July 15, 2024 09:52
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch 2 times, most recently from 762e91a to f6e27e9 Compare July 15, 2024 11:17
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch from f6e27e9 to 7b47460 Compare July 15, 2024 14:04
* Retrieve the issues from [issuesByReferenceId] that are not associated with [any package][packagesById]. These
* issues can be related to general issues within the SPDX document.
*/
fun getIssuesWithoutSpdxPackage() = issuesByReferenceId.mapNotNull { (id, issue) ->

Check warning

Code scanning / detekt

Format signature to be single when possible, multiple lines otherwise. Warning

Newline expected before expression body
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch from 7b47460 to 4ffac63 Compare July 15, 2024 14:08
sschuberth
sschuberth previously approved these changes Jul 15, 2024
@sschuberth sschuberth enabled auto-merge (rebase) July 15, 2024 14:19
@MarcelBochtler MarcelBochtler disabled auto-merge July 16, 2024 11:37
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch from 4ffac63 to 6ec315d Compare July 16, 2024 13:26
@MarcelBochtler MarcelBochtler enabled auto-merge (rebase) July 16, 2024 13:47
@@ -16,7 +16,7 @@ externalDocumentRefs:
spdxDocument: "./subproject/subproject-with-conan-reference.spdx.yml"
checksum:
algorithm: "SHA1"
checksumValue: "53840294281f7d1bf401b746162da11dcf9306eb"
checksumValue: "032a087b0da8f353c03671a72179b807b4ec7eb3"
Copy link
Member

@sschuberth sschuberth Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we have tests for SpdxResolvedDocument in place that verify the checksum, so please explain in the commit message how this could have been wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to explain it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks. Let me try to rephrase that for my understanding: The checksum was always wrong, but that fact did not result in an issue or test failure until the previous commit. Now that issues are properly propagated, it would result in a test failure of SpdxDocumentFileFunTest whose expected result files contain no issues. Correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

Add synthetic project and an associated test to verify that issues from
an external document ref are added to the result.

Signed-off-by: Marcel Bochtler <[email protected]>
Ensure that issues added in `getSpdxPackageForId()` are not dropped,
but rather added to the resulting `PackageReference`.

Signed-off-by: Marcel Bochtler <[email protected]>
Before, issues from external document refs, which could not be mapped to
a `packageId`, failed to be added as issue in the ORT result.
Add these missing transitive issues, by explicitly check for any issues
that are not related to known packages.

Signed-off-by: Marcel Bochtler <[email protected]>
As the issue is caused by an external document reference, it wasn't
reported as an issue. The previous commit fixes this issue.
Therefore, fix the checksum to keep the correct expected result for the
`SpdxDocumentFileFunTest`.

Signed-off-by: Marcel Bochtler <[email protected]>
@MarcelBochtler MarcelBochtler force-pushed the spdx-doc-dropped-issues branch from 6ec315d to 43c2fa4 Compare July 16, 2024 14:09
@MarcelBochtler MarcelBochtler merged commit 9117279 into oss-review-toolkit:main Jul 17, 2024
22 checks passed
@MarcelBochtler MarcelBochtler deleted the spdx-doc-dropped-issues branch July 17, 2024 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants