Skip to content
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 how "vendor" configure arguments are generated #2073

Merged
merged 2 commits into from Sep 24, 2020

Conversation

austin0
Copy link
Contributor

@austin0 austin0 commented Sep 11, 2020

The intention of this PR is to clean up and centralise how the java.vendor.x properties are set, and to do this in a way that is easily extensible for any future implementations. It also enables a number of properties for jdk8u that were previously only enabled for 9+. These include:

  • Vendor URL
  • Vendor Bug URL
  • Vendor VM Bug Url

I also intend for this to be a solution to #1387. The request made in the issue is to set java.vendor.version to the the source of the OpenJDK repo, this change has already been made to the release file (#2049) and is planned for the the json metadata file so seems redundant. I have instead set it to the name of the variant being built (Hotspot, OpenJ9, SAP, Corretto, Dragonwell, etc). @brunoborges @lumpfish @sxa @gdams any comments?

Java properties after PR:

    java.runtime.name = OpenJDK Runtime Environment
    java.runtime.version = 15+34-202009101534
    java.specification.name = Java Platform API Specification
    java.specification.vendor = Oracle Corporation
    java.specification.version = 15
    java.vendor = AdoptOpenJDK
    java.vendor.url = https://adoptopenjdk.net/
    java.vendor.url.bug = https://github.com/AdoptOpenJDK/openjdk-support/issues
    java.vendor.version = Hotspot
    java.version = 15
    java.version.date = 2020-09-15

Signed-off-by: Austin Bailey Austin.Bailey@ibm.com

@austin0 austin0 added refactoring Issues that focus on moving around the codebase inside or between repos enhancement Issues that enhance the code or documentation of the repo in any way labels Sep 11, 2020
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superficially looks like it'll be ok to me, but i'll let others make final judgements on the changes :-)

@M-Davies
Copy link
Contributor

run tests

@lumpfish
Copy link
Contributor

What values do you get for an openj9 build?
Also, which releases are affected and in what way? If users are 'used to' the current values, what changes would they see?

@austin0
Copy link
Contributor Author

austin0 commented Sep 14, 2020

@lumpfish the only change from the existing code for jdk11u+ is that java.vendor.version will be the build variant (i.e. Hotspot, OpenJ9, etc) instead of the current "AdoptOpenJDK", for jdk8u for following default settings will change.
Before:

java.vendor = AdoptOpenJDK
java.vendor.url = http://java.oracle.com/
java.vendor.url.bug = http://bugreport.sun.com/bugreport/

After:

java.vendor = AdoptOpenJDK
java.vendor.url = https://adoptopenjdk.net/
java.vendor.url.bug = https://github.com/AdoptOpenJDK/openjdk-support/issues

Somebody upstream decided that --with-vendor-name should set both java.vendor and java.vm.vendor so OpenJ9 has a patch to manually change it (see eclipse-openj9/openj9#5041). For jdk8u OpenJ9 explicitly sets vendor variables in a way that overwrites anything set in the Adopt build scripts (vendor_version.h), so values for jdk8u will remain exactly the same.

@lumpfish
Copy link
Contributor

Thanks @austin0 , looks good.

@M-Davies
Copy link
Contributor

@austin0 I'm going to run tests again. Can you let me know if the results are what you expect?

@M-Davies
Copy link
Contributor

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

1 similar comment
@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

@karianna karianna added this to TODO in temurin-build via automation Sep 17, 2020
@karianna karianna added this to the September 2020 milestone Sep 17, 2020
@karianna karianna moved this from TODO to In Progress in temurin-build Sep 17, 2020
Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - but we should check the output carefully. Does PR Test Builder spit out Dragonwell or OpenJ9 builds?

@austin0
Copy link
Contributor Author

austin0 commented Sep 17, 2020

@karianna I've manually checked OpenJ9 and it correctly matches other binaries, I haven't found a way to check dragonwell but I can't conceive any reason for it to break. It seems the dragonwell section will likely change anyway after a decision on what Vendor/Variant/Distribution means.

@austin0 austin0 merged commit 0636821 into adoptium:master Sep 24, 2020
temurin-build automation moved this from In Progress to Done Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that enhance the code or documentation of the repo in any way refactoring Issues that focus on moving around the codebase inside or between repos
Projects
No open projects
temurin-build
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants