Create "temurin" variant distinct from "hotspot"

[sxa]

The other half of adoptium/temurin-build#2818

Signed-off-by: Stewart X Addison sxa@redhat.com

[github-actions]

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 Guidelines

If you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before.

Tests

Github 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), I require an admin to post run tests on this PR.
If you are not an admin, please ask for one's attention in #infrastructure on Slack or ping one here.

[sophia-guo]

Does this mean hotspot variant is 'openjdk'? Test job is named by VARIANT. . What should we name test job for VARIANT temurin and VARIANT hotspot?

        switch (buildConfig.VARIANT) {
            case "openj9": variant = "j9"; break
            case "corretto": variant = "corretto"; break
            case "dragonwell": variant = "dragonwell"; break;
            case "bisheng": variant = "bisheng"; break;
            default: variant = "hs"
        }
        def jobName = "Test_openjdk${jobParams['JDK_VERSIONS']}_${variant}_${testType}_${jobParams['ARCH_OS_LIST']}"

[sxa]

I think this is ok as both temurin and hotspot would go into the variant = "hs" section which is correct. I do NOT plan to build VARIANT=hotspot in jenkins, so at the moment the hs test jobs would be exclusively for the temurin variant and when I was testing last week with this change the hs jobs got invoked correctly. We may wish to change that later of course but there should be no reason to change from hs just now :-)

[sophia-guo]

Will this change the value of System.getProperty('java.vm.name')? TKG is using the value to set impl

[sophia-guo]

Test jobs are using buildConfig.VARIANT directly to set impl parameter. jobParams.put('JDK_IMPL', buildConfig.VARIANT)

[sxa]

Will this change the value of System.getProperty('java.vm.name')? TKG is using the value to set impl

If it does it's a bug :-) I've only been testing the build process for now and making sure the correct vendor fields get passed in. vm.name should definitely be hotspot and I'll make sure that's still the case before this goes in.

[sxa]

Test jobs are using buildConfig.VARIANT directly to set impl parameter. jobParams.put('JDK_IMPL', buildConfig.VARIANT)

The jobs seemed to run through ok - possibly just by falling into "default" options. A quick solution to this would be to just override the value if it's temurin and use hotspot instead. How does that sound? We can look at doing a better split later (VARIANT probably shouldn't be used for this as has been discussed elsewhere but this is a tactical fix to try and avoid people from building their own thing called Temurin using our scripts :-) )

[sxa]

Will this change the value of System.getProperty('java.vm.name')? TKG is using the value to set impl

It's coming out as OpenJDK 64-Bit Server VM in Both cases. The differences in properties between hotspot and temurin with this change are as follows - the temurin version has the same properties as our existing GA on both versions as far as I can tell.

JDK17

21,23c21,24
<     java.vendor = Undefined Vendor
<     java.vendor.url = https://openjdk.java.net/
<     java.vendor.url.bug = https://bugreport.java.com/bugreport/
---
>     java.vendor = Eclipse Adoptium
>     java.vendor.url = https://adoptium.net/
>     java.vendor.url.bug = https://github.com/adoptium/adoptium-support/issues
>     java.vendor.version = Temurin-17.0.1+12-202201101740
32c33
<     java.vm.vendor = Undefined Vendor
---
>     java.vm.vendor = Eclipse Adoptium
55,56c56,57

JDK8

26,28c26,28
<     java.vendor = Oracle Corporation
<     java.vendor.url = http://java.oracle.com/
<     java.vendor.url.bug = http://bugreport.sun.com/bugreport/
---
>     java.vendor = Temurin
>     java.vendor.url = https://adoptium.net/
>     java.vendor.url.bug = https://github.com/adoptium/adoptium-support/issues
35c35
<     java.vm.vendor = Oracle Corporation
---
>     java.vm.vendor = Temurin
67,68c67,68

Properties for my hotspot/temurin builds and the latest adoptium GA levels are in this tarball: properties.tar.gz

[sxa]

Test jobs are using buildConfig.VARIANT directly to set impl parameter. jobParams.put('JDK_IMPL', buildConfig.VARIANT)

The jobs seemed to run through ok - possibly just by falling into "default" options. A quick solution to this would be to just override the value if it's temurin and use hotspot instead. How does that sound? We can look at doing a better split later (VARIANT probably shouldn't be used for this as has been discussed elsewhere but this is a tactical fix to try and avoid people from building their own thing called Temurin using our scripts :-) )

ping @sophia-guo - does this sound reasonable, or do you know if it would fall into a default option that does the right thing regardless?

[sxa]

Linter failures are equivalent to adoptium/temurin-build#2831 and unrelated to this PR, so should be resolved separately.

[sophia-guo]

override the value if it's temurin and use hotspot instead

@sxa that should be good.

[karianna]

I'm largely good with this as it's a good stepping stone to the variant vs vendor split.

[sxa]

I'm largely good with this as it's a good stepping stone to the variant vs vendor split.

Agreed. My feeling is that if we can at least prove we can make the split (Which we'll hopefully find out in the next couple of days when these go in!) we can have confidence that such a subsequent change will be relatively easy to do. I just didn't like the prospect of making too many changes all at once in here, particuarly when it's risky and may need to be backed out if something very obvious doesn't work.

[sxa]

override the value if it's temurin and use hotspot instead
@sxa that should be good.

Done in latest commit in this PR: a69df7a