-
-
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
Refactor signing/assemble code in openjdk_build_pipeline.groovy to support Windows/docker builds #1117
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Stewart X Addison <[email protected]>
Signed-off-by: Stewart X Addison <[email protected]>
Signed-off-by: Stewart X Addison <[email protected]>
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 (Running at https://ci.adoptium.net/job/build-scripts-pr-tester/job/openjdk-build-pr-tester/1920/) |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
0f71881
to
457077d
Compare
run tests (Triggered https://ci.adoptium.net/job/build-scripts-pr-tester/job/openjdk-build-pr-tester/1922/ with refactored cleanWS post build) |
457077d
to
ebbc774
Compare
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
ebbc774
to
c1a357b
Compare
run tests (This one is not using the cached download so has a chance of being a valid test :-) https://ci.adoptium.net/job/build-scripts-pr-tester/job/openjdk-build-pr-tester/1923/ ) |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
c1a357b
to
4286580
Compare
run tests (enableSigner was deleted from some invocations of buildScripts - linter didn't seem to pick it up though 🤔 New run: https://ci.adoptium.net/job/build-scripts-pr-tester/job/openjdk-build-pr-tester/1924 ) |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
4286580
to
c7d1ce7
Compare
run tests (Running at https://ci.adoptium.net/job/build-scripts-pr-tester/job/openjdk-build-pr-tester/1926/ since some re-runs using the mechanism described in #1057 (comment) first to verify that it should start testing my code once it gets to that point) |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
9d749be
to
6c889b5
Compare
run tests |
PR TESTER RESULT ✅ All pipelines passed! ✅ |
6c889b5
to
81504fc
Compare
Signed-off-by: Stewart X Addison <[email protected]>
Signed-off-by: Stewart X Addison <[email protected]>
run tests |
Signed-off-by: Stewart X Addison <[email protected]>
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
Possibly for later PR - reduction in the number of files copied over to the signing job to about 155 in jdk21u from over 0500. I had this in initially and it has been tested as completing ok with the sign_verification job on windows.
EDIT: Re-instated on later version of the PR as it helped to avoid the |
Signed-off-by: Stewart X Addison <[email protected]>
Signed-off-by: Stewart X Addison <[email protected]>
This version looks ok on windows docker and non-docker cases for versions 11 through 21 and the zip files are producing the same set of files (other than dfferences in the redist dlls) as the main builds ;-) JDK8 passes with the latest push to "Disable internal sign phase on jdk8u" Once this is done we'll need to verify reproducibility, but if there are any issues there then they can likely be resolved in a future PR |
Signed-off-by: Stewart X Addison <[email protected]>
Signed-off-by: Stewart X Addison <[email protected]>
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
Only failure was jdk11u/linux/x64 which failed pulling the build docker image down onto a dynamically provisioned Azure machine:
Since jdk11u/Linux/aarch64 and Linux/x64 on other versions worked ok this should not be considered a blocker. |
Note to self on workflow in build()
|
FYI @andrew-m-leonard I'm not able to verify reproducibility between the 21.0.4+7-ga and a build in docker (Note: The docker builds are using build tools instead of the full MSVC we used for the GA) It gave me 129 differences, 99.61% reproducibility. I haven't dug into what might be different in the files yet however the files I'm getting out of the build seem noticably larger in the windbld#999 build (e.g.
The cygwin version is different (3.4.6 vs 3.5.4). The compiler version is identical (19.37.32822) There was no |
Signed-off-by: Stewart X Addison <[email protected]>
run tests |
PR TESTER RESULT ✅ All pipelines passed! ✅ |
@sxa Ah of course, the July release still had the --user-openjdk-build-root-directory C:/workspace/openjdk-build/workspace/build/openjdkbuild |
There are still some issues relating to the clean workspace options in the windows/docker case but otherwise this PR is doing what it needs to and we should be able to get it in after the release (for safety reasons) |
Testing at https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld/1048/ (outside docker) and https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld/1049/ (in a container). EDIT: That doesn't work as the alternate In reference to our call earlier, it's worth noting that:
I am also running https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld/1052/ which is a build of jdk21 head which is NOT marked as a release but includes the REPRODUCIBLE_COMPARE parameter so hopefully that will trigger the "old" comparison job. Also queued up another test of 21.0.5 with the devkit in place: https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld/1054/ which should eliminate all known differences. (Non-release so probably wont' compare - https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld/1056/ is with |
There's a lot in here, so apologies in advance to the reviewers who will need a bit of time to review this properly ;-) Noting that there are some
SXAEC:
references in here which are things that still need to be cleaned up but the overall architecture and flow is now functional as per the builds in https://ci.adoptium.net/job/build-scripts/job/jobs/job/jdk21u/job/windbld which are now getting through all the steps and running tests.Some things done in this PR:
batOrSh
function that runs commands under the windows batch scripts if we're on windows instead of always usingbash
as this was causing some instability (issue 3714 listed below)buildScriptsAssemble
andbuildScriptsEclipseSigner
sign
phase has been renamed assign tgz/zip
to avoid ambiguity. I'm tempted to changeassemble
toassemble & SBoM
(Noting that it also includes thedu -k
of the output frommake-adopt-build-farm.sh
which is slow - at least 10 minutes - on my current test machine)openjdk_build_pipeline:
eyecatchers added so we can more easily find the separate sections while looking at the build logs (curl -s
theconsoleText
link and grep it for_pipeline:
- similar to thebuild.sh:
lines from the temurin-build repo)envVars
has been extracted into the top levelbuild()
and passed into thebuildScripts()
andbuildScriptsAssemble()
functions that use itopenjdk_build_dir_arg
(obsoleted in Remove use of --user-openjdk-build-root-directory now JDK-8326685 backported #1084) commented out but I'm still tempted to just remove it now ...**/*
was unnecessary.There are still some issues with the workspace cleaning options which may need to be addressed, although that can be done in subsequent PRs. e.g.
rm -rf c:/workspace/openjdk-build/cyclonedx-lib c:/workspace/openjdk-build/security
andrm -rf ' + context.WORKSPACE + '/workspace/target/*
. Some additional work will be needed before the clean options will work. Ref: adoptium/infrastructure#3723Fixes adoptium/infrastructure#3709
Supercedes #1103 (as this includes those changes)
Fixes adoptium/infrastructure#3714
Note: There is some potential follow-on work that could be done to tidy up this groovy script overall in #1116
Existing windows pipeline
New pipeline: