-
-
Notifications
You must be signed in to change notification settings - Fork 72
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 code to support publishing ea builds of JDK21 #740
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 |
run tests |
PR TESTER RESULT ✅ All pipelines passed! ✅ |
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.
Would be good to extract this into a publishEABinaries function - whitespace also seems a little out on this PR
Signed-off-by: Stewart X Addison <sxa@redhat.com>
Bear in mind that the current implementation is currently consistent with the way we use the single function to ship nightly and release binaries, so this is extending that to do EAs as well. I can do the adjustments to support splitting it out but it'll have to wait til next week (changes to these pipelines are a pain to test :-( ) |
(it's been a while since I wrote this but I'll look at reviewing the comments some time soon!) |
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
bf4ea8e
to
12e79d4
Compare
run tests |
Signed-off-by: Stewart X Addison <sxa@redhat.com>
12e79d4
to
544c05f
Compare
run tests |
Signed-off-by: Stewart X Addison <sxa@redhat.com>
caf47d4
to
6278420
Compare
PR check job is failing due to the windows ones being hard coded to use
So these can be ignored if they are the only failures. Somewhat annoyingly they come out as "green" in the BlueOcean view although that view does show the problem underneath. |
Hitting some issues undelated to this PR (Disk space exhausted on some, but it's not clear where since all the machines I've checked look ok) but I don't think there are any fundamental issues here. |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
Sigh ... failing PR tests:
... and others .... Ultimately something seems to have ran out of space. Maybe C3jenkins although it is showing 65Gb free just now. I'll try again and keep an eye on it ... |
run tests |
PR TESTER RESULT ✅ All pipelines passed! ✅ |
a8f06b7
to
4bd9341
Compare
Signed-off-by: Stewart X Addison <sxa@redhat.com>
4bd9341
to
4106ddc
Compare
Woohoo PR tester passed :-)
@karianna Are you ok with the whitespace now? And are you suggesting splitting the whole function in two so ending up with two separate places where we'd invoke the refactor_release_tool job? I'm inclined to prefer it inline so you can see the difference in logic side by side more clearly but open to other options ... @andrew-m-leonard are you ok with this now - I think I've got all your feedback addressed and am using |
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
PR TESTER RESULT ✅ All pipelines passed! ✅ |
@sxa I think so, my only one doubt is line 794 setting of TIMESTAMP, whether it should check scmReference is not null, otherwise I think publishName maybe blank (if not a tag driven build), and publish will probably fail saying TIMESTAMP not specified...? although that maybe what we want if we want to prevent publishing non-tag nightly builds? |
Good shout - I'd changed it for the TAG after your trailer content but not TIMESTAMP. Fixed. |
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
@sxa pipeline runs seem stuck, not sure how to restart them... |
They've been misbehaving a bit in the last few runs, but an earlier one that had fundamentally the same basic functionality worked so I'm not too concerned: #740 (comment) I'll try and initiate another one ... But bear in mind that since the PR tester won't test the publication those are almost irrelevant for this PR unless there's a syntax problem. |
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
Looks good I think jdk8 pipeline failed on Alpine due to the usual gpg timeout thingy... |
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
* Fix weekly check logic * Add login to be able to auto-publish JDK22 ea builds * Fixes following linting/review * Fixup whitespace Signed-off-by: Stewart X Addison <sxa@redhat.com> * Adjust for JDK21 and 22 Signed-off-by: Stewart X Addison <sxa@redhat.com> * Adjust tag logic to handle publish of JDK21 weekly builds Signed-off-by: Stewart X Addison <sxa@redhat.com> * Updates following review Signed-off-by: Stewart X Addison <sxa@redhat.com> * Update build_base_file.groovy --------- Signed-off-by: Stewart X Addison <sxa@redhat.com>
Final part (probably) of the experiment in adoptium/temurin-build#3355 (comment)
I have no idea if this is the best place to achieve this, but it should work ... Happy to adjust approach later if desired.
Tested with jdk22 then adjusted back to jdk21 before creating the PR
Note this is based on #738 so that needs to be merged first.