Skip generating checksum files for SBOM artifacts in Solaris build #4326
Conversation
|
Thank you for creating a pull request! |
|
@karianna The linter is showing warnings for lines in |
|
Still you can replace modified |
|
@judovana Thank you for your patience. I've implemented the recommended The persistent lint errors are from pre existing lines in the sections I modified. To avoid changing unrelated logic, I'd appreciate any suggestions you might have. |
|
Probably best what you can do, is to open new PR, where - without any logic change, you would fix all the linter issues. Once it is merged, you can rebase this PR, and it will automagically pass. I know it is ungrateful, but best to not mix logical changes and refactoring, and still make all bots happy. Sorry for that :( Thanx a lot for PR! All the best, J. I guess the issues paeared, as there were no changes in this file, and in meantime linter got updated. So new issues popped up. AS there is quite a few of them, would be worthy to fix them for the future. |
| else | ||
| metadata_file="${FILE}.json" | ||
| fi | ||
| createMetadataFile "$metadata_file" "${TARGET_ARCH}" "$SCM_REF" metadata/buildSource.txt metadata/version.txt "$sha256" |
There was a problem hiding this comment.
This is going to make $sha256 in the metadata file an empty string I think, which may cause problems for the API which uses the metadata file. We may have to still generate the sha256 checksum, but not leave the .sha256.txt file around. I'll tag in @andrew-m-leonard to look over this since he's a bit more familiar with this than I am :-)
There was a problem hiding this comment.
@sxa Thank you for pointing this out! I’m still learning my way around this part of the codebase, so I really appreciate the guidance. That makes sense I wasn’t fully aware of the impact on the metadata flow and the API. I’ll wait for Andrew’s input and adjust the change accordingly.
There was a problem hiding this comment.
Yes, we should ensure the metadata file has the correct sha256 value in it, but do not generate a sbom.sha256.txt file.
Just remove the sbom.sha256.txt after we have the sha256 value I suspect.
+1 from me to that, although I'd also be ok with merging this without it first being rebased on a second PR if this hasn't made the linter any worse :-) I thought we'd fixed the linter errors in here somehow. Maybe the linter just got more picky ... EDIT: Yeah https://github.com/adoptium/temurin-build/pull/4254/files should have excluded this file. FYI @adamfarley |
It did exclude this file. That PR only included sbin/solaris/dotests.sh (plus a yml file and a comment elsewhere). |
| if [[ "$FILE" =~ .*sbom.*\.json ]]; then | ||
| metadata_file=${FILE%.*}-metadata.json | ||
| # Skip checksum generation for SBOM files | ||
| if [[ "${FILE}" == *sbom.json ]]; then |
There was a problem hiding this comment.
This isn't the correct pattern as the filenames don't end in sbom.json. It needs to match a file like this:
OpenJDK8U-sbom_sparcv9_solaris_hotspot_8u482b01-ea.json
(and the equivalent x64 one instead of sparcv9)
6 participants
This PR modifies the Solaris build-simple.sh script to skip generating .sha256.txt checksum files for SBOM JSON artifacts. Previously, the script generated checksums for all artifacts, including SBOMs, which are not intended to be published. Closes #4316
Changes:
Local Testing:
Created a test workspace with dummy artifacts (OpenJDK-fake.tar.gz and OpenJDK-sbom.json).
Verified that:
Note: Reason for opening a new PR
Due to Windows removing the executable flag during local edits, the previous commit did not preserve the executable bit, which caused CI/linter failures. This PR restores the executable permission and includes the SBOM checksum skip changes together.