PROPOSAL: Use "temurin" as an alternate VARIANT option from hotspot

[sxa]

Putting the feelers out for this one. Thoughts in here sprung out as part of the changes proposed in https://github.com/adoptium/temurin-build/pull/2670/files#diff-bf1a71b6b982bb100bd797f1aac5049b232b43398baf9494c8f2823c62c8aa82

Reasons for doing it:

• It would make it simpler for third parties use our scripts to build alternate variants. At the moment they would have to use the disable-adopt-branch-safety option if they don't have Adopt/Termurin-proprietory things like our marker files, which it would be nice not to need for end-users trying our scripts.
• We could default the hotspot VARIANT to build from the upstream skara repositories
• At the moment anyone who uses our scripts to build from our repositories will produce a binary that calls itself Temurin in the version output and other vendor properties. This is something we should discourage to avoid confusion
• If we have other Adoptium project members who wish to support our binaries we should make it easy to use our build scripts to build and ship with the minimal amount of adjusting of parameters or potential trademark infringements, so having a "default HotSpot" option without any of the temurin-specifics would assist with that - it would come out as totally vanilla openjdk unless explicitly overriden - and we could provide good guidance on the
• It would likely make building alternates like Loom, Panama with our scripts etc. a bit simpler

Reasons against doing it:

• It's an extra codepath that should be maintained, although we can reasonably easily run it periodically to check, potentially just using VagrantPlaybookCheck

[jerboaa]

A huge +1 from me. It would allow folks to use the scripts to produce vanilla OpenJDK builds fairly easily and that would be very beneficial. I could see myself help testing those code branches on a regular basis.

[karianna]

Are we conflating the VM variant with the vendor here? I'm pretty sure we had some old/long-running issues stating that we needed to add vendor and allow folks to differentiate on that.

[sxa]

A huge +1 from me. It would allow folks to use the scripts to produce vanilla OpenJDK builds fairly easily and that would be very beneficial. I could see myself help testing those code branches on a regular basis.

I remember you hitting some problems a few months ago and needed some of the additional options in such a situation :-)

[sxa]

Are we conflating the VM variant with the vendor here? I'm pretty sure we had some old/long-running issues stating that we needed to add vendor and allow folks to differentiate on that.

This is purely an issue of usability of the builds scripts. At the moment they only take a VARIANT at the top level, and if it's set to HotSpot it defaults to Eclipse/Temurin. We could certainly provide a way to make VENDOR something that's passed in, but I would be strongly in favour of getting rid of a "HotSpot == Temurin" default in our scripts like

temurin-build/sbin/build.sh

Line 256 in b376432

addConfigureArg "--with-company-name=" "Temurin"

and my gut feel is that using a separate VARIANT would likely be the cleanest way to do it. Bear in mind that all VARIANTs we currently support in the build scripts are also HotSpot-based other than Eclipse OpenJ9 so my proposal reflects treating Temurin in a consistent way to the others in a relatively simple way.

Alternative proposals always welcome, but let's make sure we keep it simple, straightforward, and easy for others to integrate any future variants.

[karianna]

I agree that Hotspot != Temurin as a default (Temrun should be passed in like any other vendor) but I'd prefer to stop overloading variant in this manner going forward, this seems like the time to make the split between variant and vendor (how much appetite with have for this before July PSU I'm not sure).

[sxa]

The other advantage of contunuing to use VARIANT is my second point - that it would be a simple way to allow the default for "HotSpot" to be the upstream skara instead of Temurin (assuming there was appetite for doing that)

The question becomes how much additional work it is to switch it to being passed in as a parameter (and it wouldn't just be limited to this repository) as opposed to listing it as a variant where the setting of "all relevant values" is internalised into the scripts. I'm conscious of the fact we're seriously short of people to to significant refactoring work to change how things work at the moment and I'd be in favour of not kick the can down the road on "fixing" the default.

[sxa]

As @jerboaa has just discovered, the presence of our --with-company-name patch also makes it hard for people to build from their own JDK8 repository as it doesn't have that option that the build scripts expect.

[karianna]

As @jerboaa has just discovered, the presence of our --with-company-name patch also makes it hard for people to build from their own JDK8 repository as it doesn't have that option that the build scripts expect.

@jerboaa Is this patch something that you think jdk8u would accept in time? I honestly can't remember if we tried.

[jerboaa]

@karianna @sxa As mentioned on slack this is a patch which shouldn't be needed at all since 8u212:

https://bugs.openjdk.java.net/browse/JDK-8193764
https://bugs.openjdk.java.net/browse/JDK-8189761

Use --with-vendor-name instead. That's the upstream configure option to use.

[jerboaa]

Except for this version output change, I guess. That's not possible in plain 8u:

https://github.com/adoptium/jdk8u/compare/master..dev#diff-ee823353863b571a03eb346d7e1975d5752e78ca4cfb0ae2ce57772e2811f31fR109

[jerboaa]

https://github.com/adoptium/jdk8u/compare/master..dev#diff-362e955b86dcc01922130656ff6ee4934c8d85795f6d13bf4378efef8e9a6da1R112-R115

These should be: --with-vendor-url and --with-vendor-bug-url

[sxa]

NOTE: Ref #2817 ensure that the -without-version-xxx parameters (and the equivalent for JDK8) are included in the docs once the separate HotSpot build is in place.

[sxa]

Going to add a list of PRs that have gone in this week as a result of these changes for historic reference in case anyone needs to implement something similar:

• pipe:@sxa - Initial ci-jenkins-pipeline changes
• bld:@sxa - Initial temurin-build changes
• pipe:@sxa - Output filename was still coming out as temurin - switched back for compatibility
• build:@sxa - Adjust auto-download of boot JDK to use hotspot in API and not temurin if VARIANT=temurin
• pipe:@sxa Change public visibility of pipeline jobs so that temurin ones are visible and not hotspot
• pipe:@sophia-guo - Use buildConfig.VARIANT instead of jdk_Version to set JDK_IMPL for test jobs
• installer:@sxa - windows installers did not understand "temurin" as JVM parameter (Mapped to hotspot)
• pipe:@sxa - Issue relating to smoke tests not being auto-generated until explicitly deleted (Default was JVM_IMPL=temurin)
• build:@sxa - release file in root of JDK8 tarball had JVM_VARIANT="Temurin" in it.
• pipe:@andrew-m-leonard - TRSS API call needs to continue to use hotspot instead of temurin
• api:@johnoliver - Allow JSON metadata of temurin in the Adoptium API (Nightlies weren't showing)
• bld:@gdams - Switch github workflows to use temurin
• pipe:@gdams - Switch PR tester to use temurin
• build:@steelhead31 - src archive should not have platform name

@sophia-guo @smlambert If we have any other fixes relating to this change to use temurin instead of hotspot in the build pipelines please add them into this list, or just mention this issue in the corresponding PR - thanks :-)

[sophia-guo]

@sxa any plan to remove "hotspot" in 11 and 17 for weekly run https://github.com/adoptium/ci-jenkins-pipelines/blob/master/pipelines/jobs/configurations/jdk11u.groovy#L22
https://github.com/adoptium/ci-jenkins-pipelines/blob/master/pipelines/jobs/configurations/jdk17u.groovy#L51 ?

[sxa]

@sxa any plan to remove "hotspot" in 11 and 17 for weekly run https://github.com/adoptium/ci-jenkins-pipelines/blob/master/pipelines/jobs/configurations/jdk11u.groovy#L22 https://github.com/adoptium/ci-jenkins-pipelines/blob/master/pipelines/jobs/configurations/jdk17u.groovy#L51 ?

The targetConfigurations doesn't have hotspot in it so it shouldn't matter, but I can create a PR to remove them.