-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Upgrade TemurinGenSBOM to use CycloneDX v9.0.5 spec 1.0.6 #3985
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
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.
LGTM
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
Signed-off-by: Andrew Leonard <[email protected]>
@@ -887,23 +887,30 @@ buildCyclonedxLib() { | |||
else | |||
ANTBUILDFILE="${CYCLONEDB_DIR}/build.xml" | |||
fi | |||
|
|||
# Do we have a local cache for the dependency jars? | |||
local localJarCacheOption="" |
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.
Can we use a default value for this since it's likely to be in a common location on all machines, and it would be good to use the cached version automatically if it is available instead of requiring it to use a build option?
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.
@sxa bearing in mind "users" can use this script, do you see it a reasonable default for users in general?
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.
also what should the default be, and i'd need to add platform specific differences here..?
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.
my assumption was we would specify the adoptium ci defaults in the ci-jenkins-pipelines job configs
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.
If we're not specifying a default location within the script itself, I advise that we add error checking for a blank value.
Example below.
if [ "$(uname)" = "Darwin" ]; then | ||
JarSha=$(shasum -a 256 "$JAR" | cut -d' ' -f1) | ||
else | ||
JarSha=$(sha256sum "$JAR" | cut -d' ' -f1) | ||
fi | ||
addSBOMFormulationComponentProperty "${javaHome}" "${classpath}" "${sbomJson}" "CycloneDX" "CycloneDX jar SHAs" "${JarName}" "${JarSha}" | ||
addSBOMFormulationComponentProperty "${javaHome}" "${classpath}" "${sbomJson}" "CycloneDX" "CycloneDX jar SHAs" "${JarName}.jar" "${JarSha}" |
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.
What's the reason for stripping off the .jar
extension a few lines above then adding it back in?
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.
@sxa because line 1230 index into the dependency_data.properties needs just the component name, so i'd need to strip it for that...
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.
LGTM
Fixes #3970