Add support for buildpack.toml files #4031
Conversation
NucleonGodX
force-pushed
the
issue3477
branch
from
266b903 to
cd1ec55
Compare
NucleonGodX
changed the title
…pack package Signed-off-by: NucleonGodX <racerpro41@gmail.com>
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
NucleonGodX
force-pushed
the
issue3477
branch
from
418fb5d to
afaba83
Compare
There was a problem hiding this comment.
Thanks++ @NucleonGodX for the PR!
This is a great start.
See my comments for your consideration.
tests/packagedcode/data/buildpack/paketo-buildpacks/java-memory-assistant/buildpack.toml
Show resolved
Hide resolved
tests/packagedcode/data/buildpack/paketo-buildpacks/dotnet-execute/buildpack.toml
Show resolved
Hide resolved
tests/packagedcode/data/buildpack/paketo-buildpacks/opentelemetry/buildpack.toml
Show resolved
Hide resolved
| keywords = ["java", "apm", "trace", "opentelemetry"] | ||
| name = "Paketo Buildpack for OpenTelemetry" | ||
| sbom-formats = ["application/vnd.cyclonedx+json", "application/vnd.syft+json"] | ||
| version = "{{.version}}" |
There was a problem hiding this comment.
This is not useful to parse and store directly as version, check the spec for how we could deduce this
tests/packagedcode/data/buildpack/paketo-buildpacks/opentelemetry/buildpack.toml
Show resolved
Hide resolved
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
NucleonGodX
force-pushed
the
issue3477
branch
from
85b2ccd to
5399fdd
Compare
|
Thanks @AyanSinhaMahapatra for your suggestions, I've implemented them. |
Signed-off-by: NucleonGodX <racerpro41@gmail.com>
There was a problem hiding this comment.
@NucleonGodX thanks++ for your changes, see comments for some more improvements, and this should be ready to merge after.
| "type": "generic", | ||
| "namespace": null, | ||
| "name": "java-memory-assistant", | ||
| "version": "{{.version}}", |
There was a problem hiding this comment.
| "version": "{{.version}}", | |
| "version": null, |
We should not keep a version value if we cannot extract this properly from somewhere in the manifest or elsewhere.
| "is_optional": false, | ||
| "is_pinned": false, | ||
| "is_direct": true, | ||
| "resolved_package": {}, |
There was a problem hiding this comment.
There is a bit more information here about dependencies that we can store, since the dependencies are also resolved. This would be a PackageData mapping, with extra information about the dependencies, including the purl fields, license info and others. Please check the spec to see what other info can be specified here, and add tests accordingly with richer data as applicable.
| id = "java-memory-assistant" | ||
| name = "Java Memory Assistant Agent" | ||
| purl = "pkg:generic/sap-java-memory-assistant@0.5.0?arch=amd64" | ||
| sha256 = "9c5ffb4bdeec5ed6b4f1d734469500754a857d1452c3d253d89e2315addb04c5" |
There was a problem hiding this comment.
We need to preserve all these checksum, license, urls info in resolved_package field for the dependency. See comment below in the test expectation file too
| uri = "https://github.com/paketo-buildpacks/java-memory-assistant/blob/main/LICENSE" | ||
|
|
||
| [metadata] | ||
| include-files = ["LICENSE", "NOTICE", "README.md", "linux/amd64/bin/build", "linux/amd64/bin/detect", "linux/amd64/bin/main", "linux/amd64/bin/helper", "linux/arm64/bin/build", "linux/arm64/bin/detect", "linux/arm64/bin/main", "linux/arm64/bin/helper", "buildpack.toml"] |
There was a problem hiding this comment.
We should also have these in the extra_data as this can be useful to assemble packages potentially.
| result_packages = list(buildpack.BuildpackHandler.parse(test_file)) | ||
| expected_packages = [ | ||
| models.PackageData( | ||
| type=buildpack.BuildpackHandler.default_package_type, |
There was a problem hiding this comment.
YOu need to have these in test expectation files, this would be very hard to add/regenerate on changes otherwise. See the check_packages_data function used elsewhere like https://github.com/aboutcode-org/scancode-toolkit/blob/develop/tests/packagedcode/test_cargo.py#L60
There was a problem hiding this comment.
You don't need to add two tests for these, just checking package data in each manifest case once is fine, and add one full scan (with run_scan_click like you've done above) overall for each type of buildpack manifests is enough.
| package_data = dict( | ||
| datasource_id=cls.datasource_id, | ||
| type=cls.default_package_type, | ||
| name=name, |
There was a problem hiding this comment.
You are extracting namespace above but not including it in PackageData here.
| homepage_url=buildpack.get("homepage"), | ||
| keywords=buildpack.get("keywords", []), | ||
| extracted_license_statement=None, | ||
| dependencies=[], |
There was a problem hiding this comment.
You don't need to add dependencies and extra_data like this, as these are initialized by default. Remove these two lines.
|
Hey @AyanSinhaMahapatra, thanks for your suggestions, I will apply them in a few days. |
Fixes #3477
Tasks
Run tests locally to check for errors.
Signed-off-by: NucleonGodX manitsingh018@gmail.com