-
Notifications
You must be signed in to change notification settings - Fork 316
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
Add an initial BitBake analyzer implementation #8642
base: main
Are you sure you want to change the base?
Add an initial BitBake analyzer implementation #8642
Conversation
066b93e
to
1bc9cdd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8642 +/- ##
=========================================
Coverage 67.92% 67.92%
Complexity 1005 1005
=========================================
Files 244 244
Lines 7772 7772
Branches 876 876
=========================================
Hits 5279 5279
Misses 2110 2110
Partials 383 383
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8fc4c26
to
bd94fe3
Compare
[do-convert]: https://github.com/doubleopen-project/do-convert | ||
[SBOM]: https://docs.yoctoproject.org/dev/dev-manual/sbom.html | ||
[SPDX]: https://spdx.dev/ | ||
[SPDX document file analyzer]: https://oss-review-toolkit.org/ort/docs/tools/analyzer |
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.
commit: Why did you choose to use the analyzer for mapping the SPDX file?
(I don't know that analyzer inside out, but I recall that it was implemented with some specific use case in mind which is beyond the SDPX spec)
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.
Why did you choose to use the analyzer for mapping the SPDX file?
That was the easiest way to make use of SpdxDocumentCache
/ SpdxResolvedDocument
and related resolution logic right now.
I recall that it was implemented with some specific use case in mind which is beyond the SDPX spec
Yes, indeed we might run into issues here when fully implementing document resolution. Ideally, any such issues could be fixed in SpdxDocumentFile
itself (e.g. by generalizing code or making some behavior configurable). But if that does not work out, we should consider making direct use of the "low-level" spdx-utils
in BitBake
instead.
} | ||
|
||
result.analyzer?.result shouldNotBeNull { | ||
projects shouldHaveSize 90 |
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.
I think it would be valuable to make the fun test consistent with the ones for other package managers,
to use an expected output file which contains the expected ORT model. This would make it more obvious what the package manager outputs and protect from unintended breaking changes when changing the SPDX analyzer.
} | ||
} | ||
|
||
if (!scriptFile.delete()) logger.warn { "Unable to delete the temporary '$scriptFile' file." } |
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.
Consider putting the '$scriptFile' to the very end of the sentence.
} | ||
|
||
override fun resolveDependencies(definitionFile: File, labels: Map<String, String>): List<ProjectAnalyzerResult> = | ||
throw NotImplementedError("This function is not supported for $managerName.") |
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.
Please add a code comment explaining why.
throw NotImplementedError("This function is not supported for $managerName.") | ||
|
||
private fun getDeployDir(workingDir: File, target: String): File { | ||
val bitbakeEnv = runBitBake(workingDir, "-e", target) |
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.
Should this be bitBakeEnv
?
Alternative:
val lines = runBitBake(workingDir, "-e", target).stdout.lineSequence()
return lines.mapNotNull { it.withoutPrefix("DEPLOY_DIR=") }.first().let { File(it.removeSurrounding("\"")) }
private fun createSpdx(workingDir: File, target: String) = | ||
runBitBake(workingDir, "-r", spdxConfFile.absolutePath, "-c", "create_spdx", target) | ||
|
||
private fun File.findSpdxFiles() = resolve("spdx").walk().filter { it.isFile && it.name.endsWith(".spdx.json") } |
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.
Could be moved to file level.
website/docs/tools/analyzer.md
Outdated
@@ -14,6 +14,7 @@ Currently, the following package managers (grouped by the programming language t | |||
|
|||
* C / C++ | |||
* [Bazel](https://bazel.build/) (**experimental**) (limitations: see [open tasks](https://github.com/oss-review-toolkit/ort/issues/264)) | |||
* [BitBake](https://docs.yoctoproject.org/bitbake/) (**experimental**) (limitations: slow performance, resolution of SPDX external document refs) |
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.
Do the SPDX files exported by BitBake use external document refs?
|
||
"Analyzing recipes from Poky" should { | ||
val projectDir = tempdir() | ||
val pokyVcsInfo = VcsInfo(VcsType.GIT, "https://git.yoctoproject.org/poky", "kirkstone-4.0.17") |
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.
Not sure how long the clone takes, but it seems the repository is rather large.
Not just for speed, but also for minimizing the test, what do you think about crafting a more minimal project under the assets
directory? This would enable adding further (corner) cases easily, on future increments of this implementation.
|
||
logger.info { "Found ${spdxFiles.size} SPDX file(s) in '$commonDeployDir'." } | ||
|
||
return spdxManager.resolveDependencies(spdxFiles, labels) |
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.
Without mapping the result from the manager, I suspect that Identifiers
(e.g. type), purl, cpe might not match what is desired. Maybe also the concluded licenses should explicitly be nullified.
a8372f8
to
bf83c5b
Compare
See [1]. This prepares for BitBake support in the analyzer. [1]: https://docs.yoctoproject.org/bitbake/ Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
Add the beginnings of an analyzer for BitBake [1], see [2] for context. It basically works by making the build inherit from the "create-spdx" class [3] to create SPDX 2.2 files and post-processing them via ORT's SPDX document file analyzer. This initial implementations has several known limitations still. First of all, as the "create-spdx" class cannot be used without building, and builds are not cached, so the analyzer is very slow. Secondly, SPDX external documents refs cannot be resolved yet. This requires some post-processing of the SPDX document files before passing them on, notably by adjusting the `SPDX_NAMESPACE_PREFIX` variable [4]. [1]: https://docs.yoctoproject.org/bitbake/ [2]: oss-review-toolkit#722 [3]: https://docs.yoctoproject.org/dev/dev-manual/sbom.html [4]: https://docs.yoctoproject.org/ref-manual/variables.html#term-SPDX_NAMESPACE_PREFIX Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
bf83c5b
to
7771f2e
Compare
@@ -421,6 +421,17 @@ | |||
COPY --from=bazelbuild /opt/bazel /opt/bazel | |||
COPY --from=bazelbuild /opt/go/bin/buildozer /opt/go/bin/buildozer | |||
|
|||
#------------------------------------------------------------------------ | |||
# BITBAKE | |||
FROM base as bitbakebuild |
Check warning
Code scanning / Scorecard
Pinned-Dependencies Medium
Click Remediation section below to solve this issue
Please have a look at the individual commit messages for the details.