-
-
Notifications
You must be signed in to change notification settings - Fork 243
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
Updates to better handle RISC-V cross envs and alternate OpenJDK repos #2670
Conversation
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.
Nitpicks + one linter failure.
CONFIGURE_ARGS_FOR_ANY_PLATFORM="${CONFIGURE_ARGS_FOR_ANY_PLATFORM} --with-build-jdk=$BUILDJDK --disable-ddr" | ||
if [ -d /usr/local/openssl102 ]; then |
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.
Seems brittle on specific version?
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.
Can you explain that comment please?
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.
It's checking for a very specific version of openssl102 - is there a way to future proof that?
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.
This should be a temporary tactical fix - it's a workaround for the fact that the new --withopenssl=fetched
added a few weeks ago doesn't currently work on RISC-V, so the future proofing will be for the OpenJ9 team to make the fetched
option work.
…sitories Signed-off-by: Stewart X Addison <sxa@redhat.com>
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.
LGTM now.
Thanks. I'm still doing a few more tests and will merge later this week |
Signed-off-by: Stewart X Addison <sxa@redhat.com>
Has been tested with the two RISC-V builds (Thanks @andrew-m-leonard) we have and has not made things any worse, but will enable this to be used in more environments, so taking out of draft. |
A few changes here - mostly in the interests of usability of our scripts:
NATIVE_API_ARCH
variable)BUILD_CC
andBUILD_CXX
for Bisheng if building natively on RISC-V/usr/bin
to be usedRISCV_SYSROOT
to be overriden but still use/opt/fedora28_riscv_root
*(what Adopt uses) as the default.I would imagine this is all pretty non-controversial, although I'm "open to offers" on the last item here. See this Slack thread for the original discussion - I've chosen here to re-use the existing flag to handle all "I'm building from somewhere funky" situations, but it could be an alternate parameter e.g. an "Override the tag to be this value explicitly" (can be done separately in a future PR of course)
I do also wonder whether we should split out
temurin
as a separate variant to HotSpot now - the more I think about it in various conversations (particuarly if other people are using our scripts to build HotSpot codebases) the more I think it would keep things simple ... Raised proposal in #2671THIS IS IN DRAFT AS I WANT TO DO SOME FURTHER CHECKS, BUT REVIEWS/COMMENTS ARE VERY WELCOME IN THE MEANTIME.
Signed-off-by: Stewart X Addison sxa@redhat.com