-
-
Notifications
You must be signed in to change notification settings - Fork 71
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
Enable nightly build with reproducible comparison #714
Conversation
Thank you for creating a pull request!Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work). Code Quality and Contributing GuidelinesIf you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before. TestsGithub actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation. In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post |
Hi - I'd like to just understand the approach since there is no description associated with this PR. Is this adding an extra step into every nightly build that will rebuild it in parallel with running the normal testing, and verify that it comes out identical? And this is only enabling it on Linux/x64. Is it using the output of the SBOM to perform the second build or just re-running with the same parameters from jenkins? |
3224ffa
to
679cfd9
Compare
This is to add an extra step to trigger a **_reproduce_compare jenkins job, which will not block current pipeline job. I have created job https://ci.adoptium.net/job/jdk17u-linux-x64-temurin_reproduce_compare/ for testing, but need admins help to move it to folder build-scripts/jobs/jdk17u/. @sxa @andrew-m-leonard could you please help this? |
Thanks for the explanation. Since this job isn't being auto-generated I would suggest that it may be preferable to keep it outside the normal jdk17u folder to ensure it doesn't get accidentally deleted at any point (someone might think that everything in there can be regenerated automatically). |
bd01565
to
73a87bb
Compare
@sophia-guo Can you avoid force pushing on every fix please since it makes it impossible to see what's changed since a reviewer last looked at the code. |
Signed-off-by: Sophia Guo <[email protected]>
Did a rebase and some changes are accidentally gone.
@sxa I think with force push the changes since last review can still be available |
Resubmit the changes Signed-off-by: Sophia Guo <[email protected]>
Signed-off-by: Sophia Guo <[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.
just a couple of comments
Signed-off-by: Sophia Guo <[email protected]>
Some more linter and test fixes needed |
Yes you can get to it but it's a bit more difficult to find them, especially if there have been multiple force pushes. |
Signed-off-by: Sophia Guo <[email protected]>
Signed-off-by: Sophia Guo <[email protected]>
Signed-off-by: Sophia Guo <[email protected]>
docs/UsingOurScripts.md
Outdated
@@ -79,7 +79,7 @@ This file contains the default constants and paths used in the build scripts for | |||
// Test suite and inclusion details | |||
"testDetails" : { | |||
// Boolean denoting wether the reproducible compare is needed |
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.
// Boolean denoting wether the reproducible compare is needed | |
// Boolean denoting whether the reproducible compare is needed |
Note the test failed at https://github.com/adoptium/ci-jenkins-pipelines/pull/714/files#diff-fc15f6bbd3ef3cf595558447270454056407397071c7efbd13ca516fcf0a3a5bR66. Test is using master default.json, which has not been updated with this PR |
Signed-off-by: Sophia Guo <[email protected]>
Depends adoptium/temurin-build#3386 |
Signed-off-by: Sophia Guo <[email protected]>
10fdd38
to
cf01432
Compare
Signed-off-by: Sophia Guo <[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.
nice work @sophia-guo
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.
Just an update to https://raw.githubusercontent.com/adoptium/ci-jenkins-pipelines/master/pipelines/defaults.json required
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
Has been updated... |
Is the failing Groovy check a concern here? |
Shouldn't. See |
* Add detail error messages Signed-off-by: Sophia Guo <[email protected]> * Enable nightly reproducible comparison Resubmit the changes Signed-off-by: Sophia Guo <[email protected]> * Correct lint error Signed-off-by: Sophia Guo <[email protected]> * Add enableReproducibleCompare flag Signed-off-by: Sophia Guo <[email protected]> * Set reproducible job ENABLE_REPRODUCIBLE_COMPARE false Avoid recursive build Signed-off-by: Sophia Guo <[email protected]> * Update build timestamp Signed-off-by: Sophia Guo <[email protected]> * Format update Signed-off-by: Sophia Guo <[email protected]> * Update test case Signed-off-by: Sophia Guo <[email protected]> * Remove unnecessary file permission step Signed-off-by: Sophia Guo <[email protected]> * Typo Signed-off-by: Sophia Guo <[email protected]> * Fix isRelease and params correctly Signed-off-by: Sophia Guo <[email protected]> * Limit json file to SBOM specific and remove unnecessary code Signed-off-by: Sophia Guo <[email protected]> --------- Signed-off-by: Sophia Guo <[email protected]>
Related adoptium/temurin-build#3245
Signed-off-by: Sophia Guo [email protected]